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
        3 kB
        Todd Lipcon
      2. hadoop-7346.txt
        14 kB
        Todd Lipcon
      3. hadoop-7346.txt
        13 kB
        Todd Lipcon

        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:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development