Uploaded image for project: 'Hadoop HDFS'
  1. Hadoop HDFS
  2. HDFS-9700

DFSClient and DFSOutputStream should set TCP_NODELAY on sockets for DataTransferProtocol

    Details

    • Type: Improvement
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 2.7.1, 2.6.3
    • Fix Version/s: 2.8.0, 3.0.0-alpha1
    • Component/s: hdfs-client
    • Labels:
      None

      Description

      In DFSClient.connectToDN() and DFSOutputStream.createSocketForPipeline(), we never call setTcpNoDelay() on the constructed socket before sending. In both cases, we should respect the value of ipc.client.tcpnodelay in the configuration.

      While this applies whether security is enabled or not, it seems to have a bigger impact on latency when security is enabled.

      1. HDFS-9700_branch-2.7.patch
        3 kB
        Gary Helmling
      2. HDFS-9700_branch-2.7-v2.patch
        4 kB
        Gary Helmling
      3. HDFS-9700.002.patch
        5 kB
        Gary Helmling
      4. HDFS-9700.003.patch
        5 kB
        Gary Helmling
      5. HDFS-9700.004.patch
        5 kB
        Gary Helmling
      6. HDFS-9700-branch-2.7.002.patch
        4 kB
        Gary Helmling
      7. HDFS-9700-branch-2.7.003.patch
        4 kB
        Gary Helmling
      8. HDFS-9700-v1.patch
        3 kB
        Gary Helmling
      9. HDFS-9700-v2.patch
        5 kB
        Gary Helmling

        Issue Links

          Activity

          Hide
          ghelmling Gary Helmling added a comment -

          The attached patch is against branch-2.7. For an HBase deployment on secure Hadoop, this reliably lowers our P95 write latencies from 40ms+ to ~2ms.

          I'm still working out how/if the same changes apply to trunk.

          Show
          ghelmling Gary Helmling added a comment - The attached patch is against branch-2.7. For an HBase deployment on secure Hadoop, this reliably lowers our P95 write latencies from 40ms+ to ~2ms. I'm still working out how/if the same changes apply to trunk.
          Hide
          ghelmling Gary Helmling added a comment -

          Attaching a patch for the same changes against trunk.

          Show
          ghelmling Gary Helmling added a comment - Attaching a patch for the same changes against trunk.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 0s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          -1 test4tests 0m 0s 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 mvninstall 7m 47s trunk passed
          +1 compile 0m 31s trunk passed with JDK v1.8.0_66
          +1 compile 0m 28s trunk passed with JDK v1.7.0_91
          +1 checkstyle 0m 12s trunk passed
          +1 mvnsite 0m 34s trunk passed
          +1 mvneclipse 0m 12s trunk passed
          +1 findbugs 1m 51s trunk passed
          +1 javadoc 0m 21s trunk passed with JDK v1.8.0_66
          +1 javadoc 0m 24s trunk passed with JDK v1.7.0_91
          +1 mvninstall 0m 30s the patch passed
          +1 compile 0m 24s the patch passed with JDK v1.8.0_66
          +1 javac 0m 24s the patch passed
          +1 compile 0m 26s the patch passed with JDK v1.7.0_91
          +1 javac 0m 26s the patch passed
          +1 checkstyle 0m 12s the patch passed
          +1 mvnsite 0m 32s the patch passed
          +1 mvneclipse 0m 9s the patch passed
          +1 whitespace 0m 0s Patch has no whitespace issues.
          +1 findbugs 2m 3s the patch passed
          +1 javadoc 0m 17s the patch passed with JDK v1.8.0_66
          +1 javadoc 0m 21s the patch passed with JDK v1.7.0_91
          +1 unit 0m 49s hadoop-hdfs-client in the patch passed with JDK v1.8.0_66.
          +1 unit 0m 54s hadoop-hdfs-client in the patch passed with JDK v1.7.0_91.
          +1 asflicense 0m 19s Patch does not generate ASF License warnings.
          21m 33s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:0ca8df7
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12784241/HDFS-9700-v1.patch
          JIRA Issue HDFS-9700
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 4f1abf5f155b 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / d62b4a4
          Default Java 1.7.0_91
          Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_66 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_91
          findbugs v3.0.0
          JDK v1.7.0_91 Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/14237/testReport/
          modules C: hadoop-hdfs-project/hadoop-hdfs-client U: hadoop-hdfs-project/hadoop-hdfs-client
          Max memory used 76MB
          Powered by Apache Yetus 0.2.0-SNAPSHOT http://yetus.apache.org
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/14237/console

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 0s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. -1 test4tests 0m 0s 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 mvninstall 7m 47s trunk passed +1 compile 0m 31s trunk passed with JDK v1.8.0_66 +1 compile 0m 28s trunk passed with JDK v1.7.0_91 +1 checkstyle 0m 12s trunk passed +1 mvnsite 0m 34s trunk passed +1 mvneclipse 0m 12s trunk passed +1 findbugs 1m 51s trunk passed +1 javadoc 0m 21s trunk passed with JDK v1.8.0_66 +1 javadoc 0m 24s trunk passed with JDK v1.7.0_91 +1 mvninstall 0m 30s the patch passed +1 compile 0m 24s the patch passed with JDK v1.8.0_66 +1 javac 0m 24s the patch passed +1 compile 0m 26s the patch passed with JDK v1.7.0_91 +1 javac 0m 26s the patch passed +1 checkstyle 0m 12s the patch passed +1 mvnsite 0m 32s the patch passed +1 mvneclipse 0m 9s the patch passed +1 whitespace 0m 0s Patch has no whitespace issues. +1 findbugs 2m 3s the patch passed +1 javadoc 0m 17s the patch passed with JDK v1.8.0_66 +1 javadoc 0m 21s the patch passed with JDK v1.7.0_91 +1 unit 0m 49s hadoop-hdfs-client in the patch passed with JDK v1.8.0_66. +1 unit 0m 54s hadoop-hdfs-client in the patch passed with JDK v1.7.0_91. +1 asflicense 0m 19s Patch does not generate ASF License warnings. 21m 33s Subsystem Report/Notes Docker Image:yetus/hadoop:0ca8df7 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12784241/HDFS-9700-v1.patch JIRA Issue HDFS-9700 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 4f1abf5f155b 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / d62b4a4 Default Java 1.7.0_91 Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_66 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_91 findbugs v3.0.0 JDK v1.7.0_91 Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/14237/testReport/ modules C: hadoop-hdfs-project/hadoop-hdfs-client U: hadoop-hdfs-project/hadoop-hdfs-client Max memory used 76MB Powered by Apache Yetus 0.2.0-SNAPSHOT http://yetus.apache.org Console output https://builds.apache.org/job/PreCommit-HDFS-Build/14237/console This message was automatically generated.
          Hide
          iwasakims Masatake Iwasaki added a comment -

          CommonConfigurationKeysPublic.IPC_CLIENT_TCPNODELAY_KEY is configuration key for Hadoop IPC but the sockets refered here are for data transfer protocol (which does not use common IPC framework). Should we use another key or always set TCP_NODELAY as DFSUtilClient#peerFromSocket do, if it is crucial?

          Show
          iwasakims Masatake Iwasaki added a comment - CommonConfigurationKeysPublic.IPC_CLIENT_TCPNODELAY_KEY is configuration key for Hadoop IPC but the sockets refered here are for data transfer protocol (which does not use common IPC framework). Should we use another key or always set TCP_NODELAY as DFSUtilClient#peerFromSocket do, if it is crucial?
          Hide
          ghelmling Gary Helmling added a comment -

          Should we use another key or always set TCP_NODELAY as DFSUtilClient#peerFromSocket do, if it is crucial?

          Sure, I'm happy to just always call setTcpNoDelay(true) on the sockets created, similar to what is done in peerFromSocket. Should I do that for both cases, or should the one in DFSClient#connectToDN still be governed by the client config key?

          Show
          ghelmling Gary Helmling added a comment - Should we use another key or always set TCP_NODELAY as DFSUtilClient#peerFromSocket do, if it is crucial? Sure, I'm happy to just always call setTcpNoDelay(true) on the sockets created, similar to what is done in peerFromSocket . Should I do that for both cases, or should the one in DFSClient#connectToDN still be governed by the client config key?
          Hide
          iwasakims Masatake Iwasaki added a comment -

          Both of DFSClient#connectToDN and DataStreamer#createSocketForPipeline should not use CommonConfigurationKeysPublic.IPC_CLIENT_TCPNODELAY_KEY.

          Though I prefer always setting TCP_NODELAY, we should run some benchmark like TestDFSIo to see the effect before change the default behavior.

          Adding configuration key such as HdfsClientConfigKeys.DFS_CLIENT_SOCKET_TCP_NODELAY might be conservative option to retain existing behaviour and change the default value later. (You can see HDFS-8829 and HDFS-9259 as example for the fix.)

          Show
          iwasakims Masatake Iwasaki added a comment - Both of DFSClient#connectToDN and DataStreamer#createSocketForPipeline should not use CommonConfigurationKeysPublic.IPC_CLIENT_TCPNODELAY_KEY . Though I prefer always setting TCP_NODELAY, we should run some benchmark like TestDFSIo to see the effect before change the default behavior. Adding configuration key such as HdfsClientConfigKeys.DFS_CLIENT_SOCKET_TCP_NODELAY might be conservative option to retain existing behaviour and change the default value later. (You can see HDFS-8829 and HDFS-9259 as example for the fix.)
          Hide
          cmccabe Colin P. McCabe added a comment -

          Good find, Gary Helmling.

          Masatake Iwasaki wrote: Adding configuration key such as HdfsClientConfigKeys.DFS_CLIENT_SOCKET_TCP_NODELAY might be conservative option to retain existing behaviour and change the default value later. (You can see HDFS-8829 and HDFS-9259 as example for the fix.)

          Yeah, it makes sense to have a separate configuration key controlling whether TCP_NODELAY is set on DataTransferProtocol.

          I think we should change the default to be that TCP_NODELAY is "on" for both DataTransferProtocol and Hadoop RPC. We already try to avoid sending small messages over DataTransferProtocol, so Nagle's algorithm doesn't add a lot (and may significantly degrade the performance of things like hflush and hsync).

          Show
          cmccabe Colin P. McCabe added a comment - Good find, Gary Helmling . Masatake Iwasaki wrote: Adding configuration key such as HdfsClientConfigKeys.DFS_CLIENT_SOCKET_TCP_NODELAY might be conservative option to retain existing behaviour and change the default value later. (You can see HDFS-8829 and HDFS-9259 as example for the fix.) Yeah, it makes sense to have a separate configuration key controlling whether TCP_NODELAY is set on DataTransferProtocol . I think we should change the default to be that TCP_NODELAY is "on" for both DataTransferProtocol and Hadoop RPC. We already try to avoid sending small messages over DataTransferProtocol, so Nagle's algorithm doesn't add a lot (and may significantly degrade the performance of things like hflush and hsync).
          Hide
          ghelmling Gary Helmling added a comment -

          Here are updated patches for trunk and branch-2.7. These change the setting to use a new config key: "dfs.client.socket.tcpnodelay". This defaults to true, since HADOOP-8069 has already update the default for ipc.(client|server).tcpnodelay to true.

          I'll see if I can do some basic benchmarks with TestDFSIO with and without the setting. I don't expect this to make things worse, but neither do I expect much improvement in a throughput test like TestDFSIO.

          Where we do see a difference from this is in the P95 write latencies for our HBase clusters with HDFS security enabled.

          Show
          ghelmling Gary Helmling added a comment - Here are updated patches for trunk and branch-2.7. These change the setting to use a new config key: "dfs.client.socket.tcpnodelay". This defaults to true, since HADOOP-8069 has already update the default for ipc.(client|server).tcpnodelay to true. I'll see if I can do some basic benchmarks with TestDFSIO with and without the setting. I don't expect this to make things worse, but neither do I expect much improvement in a throughput test like TestDFSIO. Where we do see a difference from this is in the P95 write latencies for our HBase clusters with HDFS security enabled.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          -1 patch 0m 4s HDFS-9700 does not apply to trunk. Rebase required? Wrong Branch? See https://wiki.apache.org/hadoop/HowToContribute for help.



          Subsystem Report/Notes
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12785923/HDFS-9700_branch-2.7-v2.patch
          JIRA Issue HDFS-9700
          Powered by Apache Yetus 0.2.0-SNAPSHOT http://yetus.apache.org
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/14355/console

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment -1 patch 0m 4s HDFS-9700 does not apply to trunk. Rebase required? Wrong Branch? See https://wiki.apache.org/hadoop/HowToContribute for help. Subsystem Report/Notes JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12785923/HDFS-9700_branch-2.7-v2.patch JIRA Issue HDFS-9700 Powered by Apache Yetus 0.2.0-SNAPSHOT http://yetus.apache.org Console output https://builds.apache.org/job/PreCommit-HDFS-Build/14355/console This message was automatically generated.
          Hide
          ghelmling Gary Helmling added a comment -

          Renaming and reattaching the patches from yesterday in an attempt to make yetus happy.

          Show
          ghelmling Gary Helmling added a comment - Renaming and reattaching the patches from yesterday in an attempt to make yetus happy.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 0s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          -1 test4tests 0m 0s 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.
          0 mvndep 0m 15s Maven dependency ordering for branch
          +1 mvninstall 10m 49s trunk passed
          +1 compile 0m 56s trunk passed with JDK v1.8.0_66
          +1 compile 0m 47s trunk passed with JDK v1.7.0_91
          +1 checkstyle 0m 25s trunk passed
          +1 mvnsite 0m 52s trunk passed
          +1 mvneclipse 0m 18s trunk passed
          +1 findbugs 2m 23s trunk passed
          +1 javadoc 0m 42s trunk passed with JDK v1.8.0_66
          +1 javadoc 0m 38s trunk passed with JDK v1.7.0_91
          0 mvndep 0m 11s Maven dependency ordering for patch
          +1 mvninstall 0m 50s the patch passed
          +1 compile 0m 54s the patch passed with JDK v1.8.0_66
          +1 javac 0m 54s the patch passed
          +1 compile 0m 41s the patch passed with JDK v1.7.0_91
          +1 javac 0m 41s the patch passed
          +1 checkstyle 0m 22s the patch passed
          +1 mvnsite 0m 52s the patch passed
          +1 mvneclipse 0m 14s the patch passed
          +1 whitespace 0m 0s Patch has no whitespace issues.
          +1 findbugs 2m 40s the patch passed
          +1 javadoc 0m 38s the patch passed with JDK v1.8.0_66
          +1 javadoc 0m 36s the patch passed with JDK v1.7.0_91
          +1 unit 1m 36s hadoop-hdfs-client in the patch passed with JDK v1.8.0_66.
          +1 unit 1m 24s hadoop-hdfs-client in the patch passed with JDK v1.7.0_91.
          +1 asflicense 0m 26s Patch does not generate ASF License warnings.
          33m 29s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:0ca8df7
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12786068/HDFS-9700.002.patch
          JIRA Issue HDFS-9700
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 032d80ee8092 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / fa328e2
          Default Java 1.7.0_91
          Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_66 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_91
          findbugs v3.0.0
          JDK v1.7.0_91 Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/14371/testReport/
          modules C: hadoop-hdfs-project/hadoop-hdfs-client U: hadoop-hdfs-project/hadoop-hdfs-client
          Max memory used 77MB
          Powered by Apache Yetus 0.2.0-SNAPSHOT http://yetus.apache.org
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/14371/console

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 0s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. -1 test4tests 0m 0s 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. 0 mvndep 0m 15s Maven dependency ordering for branch +1 mvninstall 10m 49s trunk passed +1 compile 0m 56s trunk passed with JDK v1.8.0_66 +1 compile 0m 47s trunk passed with JDK v1.7.0_91 +1 checkstyle 0m 25s trunk passed +1 mvnsite 0m 52s trunk passed +1 mvneclipse 0m 18s trunk passed +1 findbugs 2m 23s trunk passed +1 javadoc 0m 42s trunk passed with JDK v1.8.0_66 +1 javadoc 0m 38s trunk passed with JDK v1.7.0_91 0 mvndep 0m 11s Maven dependency ordering for patch +1 mvninstall 0m 50s the patch passed +1 compile 0m 54s the patch passed with JDK v1.8.0_66 +1 javac 0m 54s the patch passed +1 compile 0m 41s the patch passed with JDK v1.7.0_91 +1 javac 0m 41s the patch passed +1 checkstyle 0m 22s the patch passed +1 mvnsite 0m 52s the patch passed +1 mvneclipse 0m 14s the patch passed +1 whitespace 0m 0s Patch has no whitespace issues. +1 findbugs 2m 40s the patch passed +1 javadoc 0m 38s the patch passed with JDK v1.8.0_66 +1 javadoc 0m 36s the patch passed with JDK v1.7.0_91 +1 unit 1m 36s hadoop-hdfs-client in the patch passed with JDK v1.8.0_66. +1 unit 1m 24s hadoop-hdfs-client in the patch passed with JDK v1.7.0_91. +1 asflicense 0m 26s Patch does not generate ASF License warnings. 33m 29s Subsystem Report/Notes Docker Image:yetus/hadoop:0ca8df7 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12786068/HDFS-9700.002.patch JIRA Issue HDFS-9700 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 032d80ee8092 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / fa328e2 Default Java 1.7.0_91 Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_66 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_91 findbugs v3.0.0 JDK v1.7.0_91 Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/14371/testReport/ modules C: hadoop-hdfs-project/hadoop-hdfs-client U: hadoop-hdfs-project/hadoop-hdfs-client Max memory used 77MB Powered by Apache Yetus 0.2.0-SNAPSHOT http://yetus.apache.org Console output https://builds.apache.org/job/PreCommit-HDFS-Build/14371/console This message was automatically generated.
          Hide
          cmccabe Colin P. McCabe added a comment - - edited

          Thanks for updating this, Gary Helmling.

            String  DFS_CLIENT_SOCKET_TCPNODELAY_KEY =
          	      "dfs.client.socket.tcpnodelay";
          

          So, the changes you've made in this patch only affect DatatTransferProtocol. The existing configuration key, ipc.client.tcpnodelay will continue to cover the case where Hadoop RPC is used, so it is not appropriate to claim that all client sockets are affected by this. This configuration key should be called something like data.transfer.protocol.client.tcpnodelay

          Show
          cmccabe Colin P. McCabe added a comment - - edited Thanks for updating this, Gary Helmling . String DFS_CLIENT_SOCKET_TCPNODELAY_KEY = "dfs.client.socket.tcpnodelay" ; So, the changes you've made in this patch only affect DatatTransferProtocol . The existing configuration key, ipc.client.tcpnodelay will continue to cover the case where Hadoop RPC is used, so it is not appropriate to claim that all client sockets are affected by this. This configuration key should be called something like data.transfer.protocol.client.tcpnodelay
          Hide
          ghelmling Gary Helmling added a comment -

          Thanks for taking a look. I'll rename the config key and variable names.

          Show
          ghelmling Gary Helmling added a comment - Thanks for taking a look. I'll rename the config key and variable names.
          Hide
          ghelmling Gary Helmling added a comment -

          Updated patch renaming config key to "dfs.data.transfer.client.tcpnodelay" to clearly reflect this is only associated with the data transfer protocol client.

          Show
          ghelmling Gary Helmling added a comment - Updated patch renaming config key to "dfs.data.transfer.client.tcpnodelay" to clearly reflect this is only associated with the data transfer protocol client.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 0s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          -1 test4tests 0m 0s 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.
          0 mvndep 0m 19s Maven dependency ordering for branch
          +1 mvninstall 10m 56s trunk passed
          +1 compile 0m 59s trunk passed with JDK v1.8.0_66
          +1 compile 0m 45s trunk passed with JDK v1.7.0_91
          +1 checkstyle 0m 24s trunk passed
          +1 mvnsite 0m 54s trunk passed
          +1 mvneclipse 0m 17s trunk passed
          +1 findbugs 2m 23s trunk passed
          +1 javadoc 0m 42s trunk passed with JDK v1.8.0_66
          +1 javadoc 0m 38s trunk passed with JDK v1.7.0_91
          0 mvndep 0m 10s Maven dependency ordering for patch
          +1 mvninstall 0m 49s the patch passed
          +1 compile 0m 55s the patch passed with JDK v1.8.0_66
          +1 javac 0m 55s the patch passed
          +1 compile 0m 43s the patch passed with JDK v1.7.0_91
          +1 javac 0m 43s the patch passed
          +1 checkstyle 0m 21s the patch passed
          +1 mvnsite 0m 52s the patch passed
          +1 mvneclipse 0m 13s the patch passed
          +1 whitespace 0m 0s Patch has no whitespace issues.
          +1 findbugs 2m 42s the patch passed
          +1 javadoc 0m 40s the patch passed with JDK v1.8.0_66
          +1 javadoc 0m 34s the patch passed with JDK v1.7.0_91
          +1 unit 1m 41s hadoop-hdfs-client in the patch passed with JDK v1.8.0_66.
          +1 unit 1m 23s hadoop-hdfs-client in the patch passed with JDK v1.7.0_91.
          +1 asflicense 0m 27s Patch does not generate ASF License warnings.
          33m 25s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:0ca8df7
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12786142/HDFS-9700.003.patch
          JIRA Issue HDFS-9700
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 215b0c15d58e 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / c89a14a
          Default Java 1.7.0_91
          Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_66 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_91
          findbugs v3.0.0
          JDK v1.7.0_91 Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/14377/testReport/
          modules C: hadoop-hdfs-project/hadoop-hdfs-client U: hadoop-hdfs-project/hadoop-hdfs-client
          Max memory used 77MB
          Powered by Apache Yetus 0.2.0-SNAPSHOT http://yetus.apache.org
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/14377/console

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 0s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. -1 test4tests 0m 0s 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. 0 mvndep 0m 19s Maven dependency ordering for branch +1 mvninstall 10m 56s trunk passed +1 compile 0m 59s trunk passed with JDK v1.8.0_66 +1 compile 0m 45s trunk passed with JDK v1.7.0_91 +1 checkstyle 0m 24s trunk passed +1 mvnsite 0m 54s trunk passed +1 mvneclipse 0m 17s trunk passed +1 findbugs 2m 23s trunk passed +1 javadoc 0m 42s trunk passed with JDK v1.8.0_66 +1 javadoc 0m 38s trunk passed with JDK v1.7.0_91 0 mvndep 0m 10s Maven dependency ordering for patch +1 mvninstall 0m 49s the patch passed +1 compile 0m 55s the patch passed with JDK v1.8.0_66 +1 javac 0m 55s the patch passed +1 compile 0m 43s the patch passed with JDK v1.7.0_91 +1 javac 0m 43s the patch passed +1 checkstyle 0m 21s the patch passed +1 mvnsite 0m 52s the patch passed +1 mvneclipse 0m 13s the patch passed +1 whitespace 0m 0s Patch has no whitespace issues. +1 findbugs 2m 42s the patch passed +1 javadoc 0m 40s the patch passed with JDK v1.8.0_66 +1 javadoc 0m 34s the patch passed with JDK v1.7.0_91 +1 unit 1m 41s hadoop-hdfs-client in the patch passed with JDK v1.8.0_66. +1 unit 1m 23s hadoop-hdfs-client in the patch passed with JDK v1.7.0_91. +1 asflicense 0m 27s Patch does not generate ASF License warnings. 33m 25s Subsystem Report/Notes Docker Image:yetus/hadoop:0ca8df7 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12786142/HDFS-9700.003.patch JIRA Issue HDFS-9700 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 215b0c15d58e 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / c89a14a Default Java 1.7.0_91 Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_66 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_91 findbugs v3.0.0 JDK v1.7.0_91 Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/14377/testReport/ modules C: hadoop-hdfs-project/hadoop-hdfs-client U: hadoop-hdfs-project/hadoop-hdfs-client Max memory used 77MB Powered by Apache Yetus 0.2.0-SNAPSHOT http://yetus.apache.org Console output https://builds.apache.org/job/PreCommit-HDFS-Build/14377/console This message was automatically generated.
          Hide
          cmccabe Colin P. McCabe added a comment -

          +1 from me. Masatake Iwasaki, what do you think?

          Show
          cmccabe Colin P. McCabe added a comment - +1 from me. Masatake Iwasaki , what do you think?
          Hide
          iwasakims Masatake Iwasaki added a comment -

          I'm +1 too on 003. I do not think setting the default value to true has harm. Thanks for the update, Gary Helmling.

          Show
          iwasakims Masatake Iwasaki added a comment - I'm +1 too on 003. I do not think setting the default value to true has harm. Thanks for the update, Gary Helmling .
          Hide
          iwasakims Masatake Iwasaki added a comment -

          Just 1 nit..

              sock.setTcpNoDelay(client.getConf().getDataTransferTcpNoDelay());
          

          This could be

              sock.setTcpNoDelay(conf.getDataTransferTcpNoDelay());
          
          Show
          iwasakims Masatake Iwasaki added a comment - Just 1 nit.. sock.setTcpNoDelay(client.getConf().getDataTransferTcpNoDelay()); This could be sock.setTcpNoDelay(conf.getDataTransferTcpNoDelay());
          Hide
          ghelmling Gary Helmling added a comment -

          Updated patch for trunk addressing Masatake Iwasaki last comment.

          Show
          ghelmling Gary Helmling added a comment - Updated patch for trunk addressing Masatake Iwasaki last comment.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 10s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          -1 test4tests 0m 0s 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 mvninstall 7m 13s trunk passed
          +1 compile 0m 29s trunk passed with JDK v1.8.0_66
          +1 compile 0m 30s trunk passed with JDK v1.7.0_91
          +1 checkstyle 0m 18s trunk passed
          +1 mvnsite 0m 35s trunk passed
          +1 mvneclipse 0m 13s trunk passed
          +1 findbugs 1m 49s trunk passed
          +1 javadoc 0m 22s trunk passed with JDK v1.8.0_66
          +1 javadoc 0m 26s trunk passed with JDK v1.7.0_91
          +1 mvninstall 0m 33s the patch passed
          +1 compile 0m 27s the patch passed with JDK v1.8.0_66
          +1 javac 0m 27s the patch passed
          +1 compile 0m 28s the patch passed with JDK v1.7.0_91
          +1 javac 0m 28s the patch passed
          +1 checkstyle 0m 16s the patch passed
          +1 mvnsite 0m 34s the patch passed
          +1 mvneclipse 0m 10s the patch passed
          +1 whitespace 0m 0s Patch has no whitespace issues.
          +1 findbugs 2m 1s the patch passed
          +1 javadoc 0m 19s the patch passed with JDK v1.8.0_66
          +1 javadoc 0m 24s the patch passed with JDK v1.7.0_91
          +1 unit 0m 52s hadoop-hdfs-client in the patch passed with JDK v1.8.0_66.
          +1 unit 0m 58s hadoop-hdfs-client in the patch passed with JDK v1.7.0_91.
          +1 asflicense 0m 18s Patch does not generate ASF License warnings.
          20m 27s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:0ca8df7
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12787400/HDFS-9700.004.patch
          JIRA Issue HDFS-9700
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 1cdc17cf9456 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / aeb13ef
          Default Java 1.7.0_91
          Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_66 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_91
          findbugs v3.0.0
          JDK v1.7.0_91 Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/14452/testReport/
          modules C: hadoop-hdfs-project/hadoop-hdfs-client U: hadoop-hdfs-project/hadoop-hdfs-client
          Max memory used 77MB
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/14452/console
          Powered by Apache Yetus 0.2.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 10s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. -1 test4tests 0m 0s 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 mvninstall 7m 13s trunk passed +1 compile 0m 29s trunk passed with JDK v1.8.0_66 +1 compile 0m 30s trunk passed with JDK v1.7.0_91 +1 checkstyle 0m 18s trunk passed +1 mvnsite 0m 35s trunk passed +1 mvneclipse 0m 13s trunk passed +1 findbugs 1m 49s trunk passed +1 javadoc 0m 22s trunk passed with JDK v1.8.0_66 +1 javadoc 0m 26s trunk passed with JDK v1.7.0_91 +1 mvninstall 0m 33s the patch passed +1 compile 0m 27s the patch passed with JDK v1.8.0_66 +1 javac 0m 27s the patch passed +1 compile 0m 28s the patch passed with JDK v1.7.0_91 +1 javac 0m 28s the patch passed +1 checkstyle 0m 16s the patch passed +1 mvnsite 0m 34s the patch passed +1 mvneclipse 0m 10s the patch passed +1 whitespace 0m 0s Patch has no whitespace issues. +1 findbugs 2m 1s the patch passed +1 javadoc 0m 19s the patch passed with JDK v1.8.0_66 +1 javadoc 0m 24s the patch passed with JDK v1.7.0_91 +1 unit 0m 52s hadoop-hdfs-client in the patch passed with JDK v1.8.0_66. +1 unit 0m 58s hadoop-hdfs-client in the patch passed with JDK v1.7.0_91. +1 asflicense 0m 18s Patch does not generate ASF License warnings. 20m 27s Subsystem Report/Notes Docker Image:yetus/hadoop:0ca8df7 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12787400/HDFS-9700.004.patch JIRA Issue HDFS-9700 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 1cdc17cf9456 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / aeb13ef Default Java 1.7.0_91 Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_66 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_91 findbugs v3.0.0 JDK v1.7.0_91 Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/14452/testReport/ modules C: hadoop-hdfs-project/hadoop-hdfs-client U: hadoop-hdfs-project/hadoop-hdfs-client Max memory used 77MB Console output https://builds.apache.org/job/PreCommit-HDFS-Build/14452/console Powered by Apache Yetus 0.2.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          iwasakims Masatake Iwasaki added a comment -

          +1. I will commit this to branch-2.8 and above if there is no further comment. Thanks for the update, Gary Helmling.

          Show
          iwasakims Masatake Iwasaki added a comment - +1. I will commit this to branch-2.8 and above if there is no further comment. Thanks for the update, Gary Helmling .
          Hide
          iwasakims Masatake Iwasaki added a comment -

          I would like to update title and type of the issue because this is improvement of DataTransferProtocol rather than bug fix of IPC.

          Show
          iwasakims Masatake Iwasaki added a comment - I would like to update title and type of the issue because this is improvement of DataTransferProtocol rather than bug fix of IPC.
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in Hadoop-trunk-Commit #9294 (See https://builds.apache.org/job/Hadoop-trunk-Commit/9294/)
          HDFS-9700. DFSClient and DFSOutputStream should set TCP_NODELAY on (iwasakims: rev 372d1302c63c6f49f99be5766c5da9647ebd9ca6)

          • hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          • hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/DataStreamer.java
          • hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/client/impl/DfsClientConf.java
          • hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/DFSClient.java
          • hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/client/HdfsClientConfigKeys.java
          Show
          hudson Hudson added a comment - FAILURE: Integrated in Hadoop-trunk-Commit #9294 (See https://builds.apache.org/job/Hadoop-trunk-Commit/9294/ ) HDFS-9700 . DFSClient and DFSOutputStream should set TCP_NODELAY on (iwasakims: rev 372d1302c63c6f49f99be5766c5da9647ebd9ca6) hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/DataStreamer.java hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/client/impl/DfsClientConf.java hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/DFSClient.java hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/client/HdfsClientConfigKeys.java
          Hide
          iwasakims Masatake Iwasaki added a comment -

          Committed to branch-2.8 and above. Thanks, Gary Helmling, Mingliang Liu and Colin P. McCabe.

          Show
          iwasakims Masatake Iwasaki added a comment - Committed to branch-2.8 and above. Thanks, Gary Helmling , Mingliang Liu and Colin P. McCabe .
          Hide
          arpitagarwal Arpit Agarwal added a comment -

          This is a nice improvement. In the interests of reducing our configuration, are there any objections to just reusing IPC_CLIENT_TCPNODELAY_DEFAULT instead of the new config setting DFS_DATA_TRANSFER_CLIENT_TCPNODELAY_KEY (even though data transfer does not use Hadoop IPC)?

          I can't think of a situation where an administrator would want different values for these settings.

          Show
          arpitagarwal Arpit Agarwal added a comment - This is a nice improvement. In the interests of reducing our configuration, are there any objections to just reusing IPC_CLIENT_TCPNODELAY_DEFAULT instead of the new config setting DFS_DATA_TRANSFER_CLIENT_TCPNODELAY_KEY (even though data transfer does not use Hadoop IPC)? I can't think of a situation where an administrator would want different values for these settings.
          Hide
          cmccabe Colin P. McCabe added a comment -

          Hmm. I think it's confusing to use a configuration key for Hadoop RPC to configure something that isn't Hadoop RPC. We have tons of keys named with ipc and all of them relate to Hadoop RPC, not to DataTransferProtocol. ipc.client.connect.max.retries, ipc.server.listen.queue.size, ipc.client.connect.timeout, and so forth.

          There are valid cases where you might want a different configuration for RPC versus datatransferprotocol. For example, conservative users might also want to avoid turning on TCP_NODELAY for DataTransferProtocol since it is a new feature, and not as well tested as doing what we do currently. But since we have TCP_NODELAY on for RPC, they might want to keep that on.

          I agree that in the long term, TCP_NODELAY should be used for both. But that's an argument for removing the configuration altogether, not for making it do something other than what it's named.

          Show
          cmccabe Colin P. McCabe added a comment - Hmm. I think it's confusing to use a configuration key for Hadoop RPC to configure something that isn't Hadoop RPC. We have tons of keys named with ipc and all of them relate to Hadoop RPC, not to DataTransferProtocol. ipc.client.connect.max.retries , ipc.server.listen.queue.size , ipc.client.connect.timeout , and so forth. There are valid cases where you might want a different configuration for RPC versus datatransferprotocol. For example, conservative users might also want to avoid turning on TCP_NODELAY for DataTransferProtocol since it is a new feature, and not as well tested as doing what we do currently. But since we have TCP_NODELAY on for RPC, they might want to keep that on. I agree that in the long term, TCP_NODELAY should be used for both. But that's an argument for removing the configuration altogether, not for making it do something other than what it's named.

            People

            • Assignee:
              ghelmling Gary Helmling
              Reporter:
              ghelmling Gary Helmling
            • Votes:
              0 Vote for this issue
              Watchers:
              10 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development