Hadoop Common
  1. Hadoop Common
  2. HADOOP-7346

Send back nicer error to clients using outdated IPC version

    Details

    • Type: Improvement Improvement
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.22.0
    • Fix Version/s: 0.22.0
    • Component/s: ipc
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      When an older Hadoop version tries to contact a newer Hadoop version across an IPC protocol version bump, the client currently just gets a non-useful error message like "EOFException".

      Instead, the IPC server code can speak just enough of prior IPC protocols to send back a "fatal" message indicating the version mismatch.

      1. hadoop-7346.txt
        13 kB
        Todd Lipcon
      2. hadoop-7346.txt
        14 kB
        Todd Lipcon
      3. hadoop-7346.txt
        3 kB
        Todd Lipcon

        Issue Links

          Activity

          Hide
          Todd Lipcon added a comment -

          I manually tested this patch using an 0.18.3 client, 0.20.0 client, and CDH3 client (which has the same IPC stack as 0.20.20x series)

          I also looped an old client for a few minutes hitting the new server, then took a jstack on the new server, to make sure there weren't any thread leaks.

          Unfortunately it's not easy to automatically test it since we can't load old versions of the IPC clients within the context of a unit test.

          Show
          Todd Lipcon added a comment - I manually tested this patch using an 0.18.3 client, 0.20.0 client, and CDH3 client (which has the same IPC stack as 0.20.20x series) I also looped an old client for a few minutes hitting the new server, then took a jstack on the new server, to make sure there weren't any thread leaks. Unfortunately it's not easy to automatically test it since we can't load old versions of the IPC clients within the context of a unit test.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12481025/hadoop-7346.txt
          against trunk revision 1129905.

          +1 @author. The patch does not contain any @author tags.

          -1 tests included. The patch doesn't appear to include any new or modified tests.
          Please justify why no new tests are needed for this patch.
          Also please list what manual steps were performed to verify this patch.

          +1 javadoc. The javadoc tool did not generate any warning messages.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          +1 core tests. The patch passed core unit tests.

          +1 system test framework. The patch passed system test framework compile.

          Test results: https://builds.apache.org/hudson/job/PreCommit-HADOOP-Build/550//testReport/
          Findbugs warnings: https://builds.apache.org/hudson/job/PreCommit-HADOOP-Build/550//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: https://builds.apache.org/hudson/job/PreCommit-HADOOP-Build/550//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12481025/hadoop-7346.txt against trunk revision 1129905. +1 @author. The patch does not contain any @author tags. -1 tests included. The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed core unit tests. +1 system test framework. The patch passed system test framework compile. Test results: https://builds.apache.org/hudson/job/PreCommit-HADOOP-Build/550//testReport/ Findbugs warnings: https://builds.apache.org/hudson/job/PreCommit-HADOOP-Build/550//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/hudson/job/PreCommit-HADOOP-Build/550//console This message is automatically generated.
          Hide
          Konstantin Boudnik added a comment -

          +1 patch looks good.

          Do you think it is possible to test this new behavior with perhaps mocks?

          Show
          Konstantin Boudnik added a comment - +1 patch looks good. Do you think it is possible to test this new behavior with perhaps mocks?
          Hide
          Todd Lipcon added a comment -

          I couldn't think of any way to do it with mocks, so I did brute force and wrote some tests which just replay RPC traces that I captured by hand from 0.18.3, branch-0.20, and 0.21.0. They then verify that the response matches my captured response (which I verified generated a reasonable exception on the client)

          Show
          Todd Lipcon added a comment - I couldn't think of any way to do it with mocks, so I did brute force and wrote some tests which just replay RPC traces that I captured by hand from 0.18.3, branch-0.20, and 0.21.0. They then verify that the response matches my captured response (which I verified generated a reasonable exception on the client)
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12481179/hadoop-7346.txt
          against trunk revision 1129989.

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 3 new or modified tests.

          +1 javadoc. The javadoc tool did not generate any warning messages.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          +1 core tests. The patch passed core unit tests.

          +1 system test framework. The patch passed system test framework compile.

          Test results: https://builds.apache.org/hudson/job/PreCommit-HADOOP-Build/558//testReport/
          Findbugs warnings: https://builds.apache.org/hudson/job/PreCommit-HADOOP-Build/558//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: https://builds.apache.org/hudson/job/PreCommit-HADOOP-Build/558//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - +1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12481179/hadoop-7346.txt against trunk revision 1129989. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed core unit tests. +1 system test framework. The patch passed system test framework compile. Test results: https://builds.apache.org/hudson/job/PreCommit-HADOOP-Build/558//testReport/ Findbugs warnings: https://builds.apache.org/hudson/job/PreCommit-HADOOP-Build/558//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/hudson/job/PreCommit-HADOOP-Build/558//console This message is automatically generated.
          Hide
          Konstantin Boudnik added a comment -

          Creative Thanks.
          There's a couple of white-space changes in the latest patch.
          +1 otherwise.

          Show
          Konstantin Boudnik added a comment - Creative Thanks. There's a couple of white-space changes in the latest patch. +1 otherwise.
          Hide
          Todd Lipcon added a comment -

          Rebased against trunk and removed spurious whitespace change. I'll commit based on Cos's +1 if this passes Hudson (no code change since previous patch)

          Show
          Todd Lipcon added a comment - Rebased against trunk and removed spurious whitespace change. I'll commit based on Cos's +1 if this passes Hudson (no code change since previous patch)
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12481406/hadoop-7346.txt
          against trunk revision 1130833.

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 3 new or modified tests.

          +1 javadoc. The javadoc tool did not generate any warning messages.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          +1 core tests. The patch passed core unit tests.

          +1 system test framework. The patch passed system test framework compile.

          Test results: https://builds.apache.org/hudson/job/PreCommit-HADOOP-Build/573//testReport/
          Findbugs warnings: https://builds.apache.org/hudson/job/PreCommit-HADOOP-Build/573//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: https://builds.apache.org/hudson/job/PreCommit-HADOOP-Build/573//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - +1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12481406/hadoop-7346.txt against trunk revision 1130833. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed core unit tests. +1 system test framework. The patch passed system test framework compile. Test results: https://builds.apache.org/hudson/job/PreCommit-HADOOP-Build/573//testReport/ Findbugs warnings: https://builds.apache.org/hudson/job/PreCommit-HADOOP-Build/573//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/hudson/job/PreCommit-HADOOP-Build/573//console This message is automatically generated.
          Hide
          Todd Lipcon added a comment -

          I'd like to commit this to 0.22 branch, since it really helps with version transitions, and I think 0.22 should see more uptake than 0.21 did.

          It applies cleanly, the only issue is that it requires guava as a dependency in the tests. Anyone have an issue with my adding it to ivy? The other option is to not include the tests in the commit (or reimplement the two methods we need). Adding guava seems easiest and will also make it easier to backport other patches from trunk which might use it, so that' my preference.

          Show
          Todd Lipcon added a comment - I'd like to commit this to 0.22 branch, since it really helps with version transitions, and I think 0.22 should see more uptake than 0.21 did. It applies cleanly, the only issue is that it requires guava as a dependency in the tests. Anyone have an issue with my adding it to ivy? The other option is to not include the tests in the commit (or reimplement the two methods we need). Adding guava seems easiest and will also make it easier to backport other patches from trunk which might use it, so that' my preference.
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Common-trunk-Commit #637 (See https://builds.apache.org/hudson/job/Hadoop-Common-trunk-Commit/637/)
          HADOOP-7346. Send back nicer error message to clients using outdated IPC version. Contributed by Todd Lipcon.

          todd : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1131254
          Files :

          • /hadoop/common/trunk/CHANGES.txt
          • /hadoop/common/trunk/src/java/org/apache/hadoop/ipc/Server.java
          • /hadoop/common/trunk/src/test/core/org/apache/hadoop/ipc/TestIPC.java
          Show
          Hudson added a comment - Integrated in Hadoop-Common-trunk-Commit #637 (See https://builds.apache.org/hudson/job/Hadoop-Common-trunk-Commit/637/ ) HADOOP-7346 . Send back nicer error message to clients using outdated IPC version. Contributed by Todd Lipcon. todd : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1131254 Files : /hadoop/common/trunk/CHANGES.txt /hadoop/common/trunk/src/java/org/apache/hadoop/ipc/Server.java /hadoop/common/trunk/src/test/core/org/apache/hadoop/ipc/TestIPC.java
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Common-trunk #709 (See https://builds.apache.org/hudson/job/Hadoop-Common-trunk/709/)
          HADOOP-7346. Send back nicer error message to clients using outdated IPC version. Contributed by Todd Lipcon.

          todd : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1131254
          Files :

          • /hadoop/common/trunk/CHANGES.txt
          • /hadoop/common/trunk/src/java/org/apache/hadoop/ipc/Server.java
          • /hadoop/common/trunk/src/test/core/org/apache/hadoop/ipc/TestIPC.java
          Show
          Hudson added a comment - Integrated in Hadoop-Common-trunk #709 (See https://builds.apache.org/hudson/job/Hadoop-Common-trunk/709/ ) HADOOP-7346 . Send back nicer error message to clients using outdated IPC version. Contributed by Todd Lipcon. todd : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1131254 Files : /hadoop/common/trunk/CHANGES.txt /hadoop/common/trunk/src/java/org/apache/hadoop/ipc/Server.java /hadoop/common/trunk/src/test/core/org/apache/hadoop/ipc/TestIPC.java
          Hide
          Todd Lipcon added a comment -

          I checked with Nigel, he said he was cool with adding guava as a dependency in 0.22. So I've committed this to 22 as well.

          Show
          Todd Lipcon added a comment - I checked with Nigel, he said he was cool with adding guava as a dependency in 0.22. So I've committed this to 22 as well.
          Hide
          Todd Lipcon added a comment -

          Thanks for the review, Cos!

          Show
          Todd Lipcon added a comment - Thanks for the review, Cos!
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Common-22-branch #62 (See https://builds.apache.org/hudson/job/Hadoop-Common-22-branch/62/)
          HADOOP-7346. Send back nicer error message to clients using outdated IPC version. Contributed by Todd Lipcon.

          todd : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1132528
          Files :

          • /hadoop/common/branches/branch-0.22/src/java/org/apache/hadoop/ipc/Server.java
          • /hadoop/common/branches/branch-0.22/ivy/libraries.properties
          • /hadoop/common/branches/branch-0.22/src/test/core/org/apache/hadoop/ipc/TestIPC.java
          • /hadoop/common/branches/branch-0.22/CHANGES.txt
          • /hadoop/common/branches/branch-0.22/ivy.xml
          Show
          Hudson added a comment - Integrated in Hadoop-Common-22-branch #62 (See https://builds.apache.org/hudson/job/Hadoop-Common-22-branch/62/ ) HADOOP-7346 . Send back nicer error message to clients using outdated IPC version. Contributed by Todd Lipcon. todd : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1132528 Files : /hadoop/common/branches/branch-0.22/src/java/org/apache/hadoop/ipc/Server.java /hadoop/common/branches/branch-0.22/ivy/libraries.properties /hadoop/common/branches/branch-0.22/src/test/core/org/apache/hadoop/ipc/TestIPC.java /hadoop/common/branches/branch-0.22/CHANGES.txt /hadoop/common/branches/branch-0.22/ivy.xml

            People

            • Assignee:
              Todd Lipcon
              Reporter:
              Todd Lipcon
            • Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development