Hadoop Common
  1. Hadoop Common
  2. HADOOP-8071

Avoid an extra packet in client code when nagling is disabled

    Details

    • Type: Improvement Improvement
    • Status: Resolved
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 0.23.1
    • Fix Version/s: 0.23.2
    • Component/s: ipc
    • Labels:
      None
    • Hadoop Flags:
      Reviewed
    • Target Version/s:

      Description

      Currently, if you disable TCP_NODELAY in the IPC client, you get an extra packet for each call which contains the call's length. This is unnecessary. Instead, we can just reserve the 4 bytes in the buffer up front, then go back to fill it in before pushing the call to the wire.

        Issue Links

          Activity

          Hide
          Todd Lipcon added a comment -

          This seems to only be happening with larger RPC response sizes.
          I verified this patch by running wireshark along with the following command:

           /usr/lib/jvm/java-6-sun/bin/java -cp /home/todd/git/hadoop-common/hadoop-dist/target/hadoop-0.24.0-SNAPSHOT/share/hadoop/common/lib/*:target/classes:target/test-classes org.apache.hadoop.ipc.RPCCallBenchmark -Dipc.server.tcpnodelay=true -Dipc.client.tcpnodelay=true -c 1 -s 1 -t 1 -m 8300  -e protobuf
          

          Without the patch, I saw the 4-byte length prefix packets followed by actual data packets, whereas with the patch, they were collapsed.

          When I ran with a smaller message size (10 bytes instead of 8300 bytes) the behavior didn't reproduce - probably something to do with buffering somewhere in the IPC code.

          Show
          Todd Lipcon added a comment - This seems to only be happening with larger RPC response sizes. I verified this patch by running wireshark along with the following command: /usr/lib/jvm/java-6-sun/bin/java -cp /home/todd/git/hadoop-common/hadoop-dist/target/hadoop-0.24.0-SNAPSHOT/share/hadoop/common/lib/*:target/classes:target/test-classes org.apache.hadoop.ipc.RPCCallBenchmark -Dipc.server.tcpnodelay= true -Dipc.client.tcpnodelay= true -c 1 -s 1 -t 1 -m 8300 -e protobuf Without the patch, I saw the 4-byte length prefix packets followed by actual data packets, whereas with the patch, they were collapsed. When I ran with a smaller message size (10 bytes instead of 8300 bytes) the behavior didn't reproduce - probably something to do with buffering somewhere in the IPC code.
          Hide
          Hadoop QA added a comment -

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

          +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 eclipse:eclipse. The patch built with eclipse:eclipse.

          +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 unit tests in .

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

          Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/593//testReport/
          Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/593//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/12514451/hadoop-8071.txt against trunk revision . +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 eclipse:eclipse. The patch built with eclipse:eclipse. +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 unit tests in . +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/593//testReport/ Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/593//console This message is automatically generated.
          Hide
          Aaron T. Myers added a comment -

          +1, the patch looks good to me. Great find.

          Show
          Aaron T. Myers added a comment - +1, the patch looks good to me. Great find.
          Hide
          Todd Lipcon added a comment -

          Committed to 0.23.2 (since it's low risk) and trunk.

          Show
          Todd Lipcon added a comment - Committed to 0.23.2 (since it's low risk) and trunk.
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Hdfs-trunk-Commit #1800 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Commit/1800/)
          HADOOP-8071. Avoid an extra packet in client code when nagling is disabled. Contributed by Todd Lipcon. (Revision 1244189)

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

          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Client.java
          Show
          Hudson added a comment - Integrated in Hadoop-Hdfs-trunk-Commit #1800 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Commit/1800/ ) HADOOP-8071 . Avoid an extra packet in client code when nagling is disabled. Contributed by Todd Lipcon. (Revision 1244189) Result = SUCCESS todd : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1244189 Files : /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Client.java
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Common-trunk-Commit #1726 (See https://builds.apache.org/job/Hadoop-Common-trunk-Commit/1726/)
          HADOOP-8071. Avoid an extra packet in client code when nagling is disabled. Contributed by Todd Lipcon. (Revision 1244189)

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

          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Client.java
          Show
          Hudson added a comment - Integrated in Hadoop-Common-trunk-Commit #1726 (See https://builds.apache.org/job/Hadoop-Common-trunk-Commit/1726/ ) HADOOP-8071 . Avoid an extra packet in client code when nagling is disabled. Contributed by Todd Lipcon. (Revision 1244189) Result = SUCCESS todd : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1244189 Files : /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Client.java
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Common-0.23-Commit #550 (See https://builds.apache.org/job/Hadoop-Common-0.23-Commit/550/)
          HADOOP-8071. Avoid an extra packet in client code when nagling is disabled. Contributed by Todd Lipcon. (Revision 1244190)

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

          • /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/CHANGES.txt
          • /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Client.java
          Show
          Hudson added a comment - Integrated in Hadoop-Common-0.23-Commit #550 (See https://builds.apache.org/job/Hadoop-Common-0.23-Commit/550/ ) HADOOP-8071 . Avoid an extra packet in client code when nagling is disabled. Contributed by Todd Lipcon. (Revision 1244190) Result = FAILURE todd : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1244190 Files : /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/CHANGES.txt /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Client.java
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Hdfs-0.23-Commit #538 (See https://builds.apache.org/job/Hadoop-Hdfs-0.23-Commit/538/)
          HADOOP-8071. Avoid an extra packet in client code when nagling is disabled. Contributed by Todd Lipcon. (Revision 1244190)

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

          • /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/CHANGES.txt
          • /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Client.java
          Show
          Hudson added a comment - Integrated in Hadoop-Hdfs-0.23-Commit #538 (See https://builds.apache.org/job/Hadoop-Hdfs-0.23-Commit/538/ ) HADOOP-8071 . Avoid an extra packet in client code when nagling is disabled. Contributed by Todd Lipcon. (Revision 1244190) Result = SUCCESS todd : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1244190 Files : /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/CHANGES.txt /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Client.java
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Mapreduce-0.23-Commit #554 (See https://builds.apache.org/job/Hadoop-Mapreduce-0.23-Commit/554/)
          HADOOP-8071. Avoid an extra packet in client code when nagling is disabled. Contributed by Todd Lipcon. (Revision 1244190)

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

          • /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/CHANGES.txt
          • /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Client.java
          Show
          Hudson added a comment - Integrated in Hadoop-Mapreduce-0.23-Commit #554 (See https://builds.apache.org/job/Hadoop-Mapreduce-0.23-Commit/554/ ) HADOOP-8071 . Avoid an extra packet in client code when nagling is disabled. Contributed by Todd Lipcon. (Revision 1244190) Result = ABORTED todd : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1244190 Files : /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/CHANGES.txt /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Client.java
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Mapreduce-trunk-Commit #1738 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Commit/1738/)
          HADOOP-8071. Avoid an extra packet in client code when nagling is disabled. Contributed by Todd Lipcon. (Revision 1244189)

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

          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Client.java
          Show
          Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk-Commit #1738 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Commit/1738/ ) HADOOP-8071 . Avoid an extra packet in client code when nagling is disabled. Contributed by Todd Lipcon. (Revision 1244189) Result = ABORTED todd : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1244189 Files : /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Client.java
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Hdfs-trunk #956 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/956/)
          HADOOP-8071. Avoid an extra packet in client code when nagling is disabled. Contributed by Todd Lipcon. (Revision 1244189)

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

          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Client.java
          Show
          Hudson added a comment - Integrated in Hadoop-Hdfs-trunk #956 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/956/ ) HADOOP-8071 . Avoid an extra packet in client code when nagling is disabled. Contributed by Todd Lipcon. (Revision 1244189) Result = FAILURE todd : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1244189 Files : /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Client.java
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Hdfs-0.23-Build #169 (See https://builds.apache.org/job/Hadoop-Hdfs-0.23-Build/169/)
          HADOOP-8071. Avoid an extra packet in client code when nagling is disabled. Contributed by Todd Lipcon. (Revision 1244190)

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

          • /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/CHANGES.txt
          • /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Client.java
          Show
          Hudson added a comment - Integrated in Hadoop-Hdfs-0.23-Build #169 (See https://builds.apache.org/job/Hadoop-Hdfs-0.23-Build/169/ ) HADOOP-8071 . Avoid an extra packet in client code when nagling is disabled. Contributed by Todd Lipcon. (Revision 1244190) Result = FAILURE todd : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1244190 Files : /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/CHANGES.txt /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Client.java
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Mapreduce-0.23-Build #197 (See https://builds.apache.org/job/Hadoop-Mapreduce-0.23-Build/197/)
          HADOOP-8071. Avoid an extra packet in client code when nagling is disabled. Contributed by Todd Lipcon. (Revision 1244190)

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

          • /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/CHANGES.txt
          • /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Client.java
          Show
          Hudson added a comment - Integrated in Hadoop-Mapreduce-0.23-Build #197 (See https://builds.apache.org/job/Hadoop-Mapreduce-0.23-Build/197/ ) HADOOP-8071 . Avoid an extra packet in client code when nagling is disabled. Contributed by Todd Lipcon. (Revision 1244190) Result = FAILURE todd : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1244190 Files : /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/CHANGES.txt /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Client.java
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Mapreduce-trunk #991 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/991/)
          HADOOP-8071. Avoid an extra packet in client code when nagling is disabled. Contributed by Todd Lipcon. (Revision 1244189)

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

          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Client.java
          Show
          Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk #991 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/991/ ) HADOOP-8071 . Avoid an extra packet in client code when nagling is disabled. Contributed by Todd Lipcon. (Revision 1244189) Result = SUCCESS todd : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1244189 Files : /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Client.java

            People

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

              Dates

              • Created:
                Updated:
                Resolved:

                Development