Hadoop HDFS
  1. Hadoop HDFS
  2. HDFS-4350

Make enabling of stale marking on read and write paths independent

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 3.0.0
    • Fix Version/s: 1.2.0, 2.0.3-alpha
    • Component/s: None
    • Labels:
      None
    • Hadoop Flags:
      Incompatible change, Reviewed
    • Release Note:
      Hide
      This patch makes an incompatible configuration change, as described below:
      In releases 1.1.0 and other point releases 1.1.x, the configuration parameter "dfs.namenode.check.stale.datanode" could be used to turn on checking for the stale nodes. This configuration is no longer supported in release 1.2.0 onwards and is renamed as "dfs.namenode.avoid.read.stale.datanode".

      How feature works and configuring this feature:
      As described in HDFS-3703 release notes, datanode stale period can be configured using parameter "dfs.namenode.stale.datanode.interval" in seconds (default value is 30 seconds). NameNode can be configured to use this staleness information for reads using configuration "dfs.namenode.avoid.read.stale.datanode". When this parameter is set to true, namenode picks a stale datanode as the last target to read from when returning block locations for reads. Using staleness information for writes is as described in the releases notes of HDFS-3912.
      Show
      This patch makes an incompatible configuration change, as described below: In releases 1.1.0 and other point releases 1.1.x, the configuration parameter "dfs.namenode.check.stale.datanode" could be used to turn on checking for the stale nodes. This configuration is no longer supported in release 1.2.0 onwards and is renamed as "dfs.namenode.avoid.read.stale.datanode". How feature works and configuring this feature: As described in HDFS-3703 release notes, datanode stale period can be configured using parameter "dfs.namenode.stale.datanode.interval" in seconds (default value is 30 seconds). NameNode can be configured to use this staleness information for reads using configuration "dfs.namenode.avoid.read.stale.datanode". When this parameter is set to true, namenode picks a stale datanode as the last target to read from when returning block locations for reads. Using staleness information for writes is as described in the releases notes of HDFS-3912 .
    • Target Version/s:

      Description

      Marking of datanodes as stale for the read and write path was introduced in HDFS-3703 and HDFS-3912 respectively. This is enabled using two new keys, DFS_NAMENODE_CHECK_STALE_DATANODE_KEY and DFS_NAMENODE_AVOID_STALE_DATANODE_FOR_WRITE_KEY. However, there currently exists a dependency, since you cannot enable write marking without also enabling read marking, since the first key enables both checking of staleness and read marking.

      I propose renaming the first key to DFS_NAMENODE_AVOID_STALE_DATANODE_FOR_READ_KEY, and make checking enabled if either of the keys are set. This will allow read and write marking to be enabled independently.

      1. hdfs-4350-1.patch
        22 kB
        Andrew Wang
      2. hdfs-4350-2.patch
        22 kB
        Andrew Wang
      3. hdfs-4350-3.patch
        22 kB
        Andrew Wang
      4. hdfs-4350-4.patch
        23 kB
        Andrew Wang
      5. hdfs-4350.txt
        26 kB
        Todd Lipcon
      6. hdfs-4350-5.patch
        26 kB
        Andrew Wang
      7. hdfs-4350-branch-1-1.patch
        22 kB
        Andrew Wang
      8. hdfs-4350-6.patch
        27 kB
        Andrew Wang
      9. hdfs-4350-branch-1-2.patch
        22 kB
        Andrew Wang
      10. hdfs-4350-7.patch
        27 kB
        Andrew Wang
      11. hdfs-4350-branch-1-3.patch
        22 kB
        Andrew Wang

        Issue Links

          Activity

          Hide
          Hudson added a comment -

          Integrated in Hadoop-Mapreduce-trunk #1333 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1333/)
          HDFS-4350. Make enabling of stale marking on read and write paths independent. Contributed by Andrew Wang. (Revision 1441819)

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

          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSConfigKeys.java
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSUtil.java
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeManager.java
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/HeartbeatManager.java
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/resources/hdfs-default.xml
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestGetBlocks.java
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestReplicationPolicy.java
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/metrics/TestNameNodeMetrics.java
          Show
          Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk #1333 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1333/ ) HDFS-4350 . Make enabling of stale marking on read and write paths independent. Contributed by Andrew Wang. (Revision 1441819) Result = FAILURE suresh : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1441819 Files : /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSConfigKeys.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSUtil.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeManager.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/HeartbeatManager.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/resources/hdfs-default.xml /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestGetBlocks.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestReplicationPolicy.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/metrics/TestNameNodeMetrics.java
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Hdfs-trunk #1305 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1305/)
          HDFS-4350. Make enabling of stale marking on read and write paths independent. Contributed by Andrew Wang. (Revision 1441819)

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

          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSConfigKeys.java
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSUtil.java
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeManager.java
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/HeartbeatManager.java
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/resources/hdfs-default.xml
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestGetBlocks.java
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestReplicationPolicy.java
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/metrics/TestNameNodeMetrics.java
          Show
          Hudson added a comment - Integrated in Hadoop-Hdfs-trunk #1305 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1305/ ) HDFS-4350 . Make enabling of stale marking on read and write paths independent. Contributed by Andrew Wang. (Revision 1441819) Result = SUCCESS suresh : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1441819 Files : /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSConfigKeys.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSUtil.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeManager.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/HeartbeatManager.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/resources/hdfs-default.xml /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestGetBlocks.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestReplicationPolicy.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/metrics/TestNameNodeMetrics.java
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Yarn-trunk #116 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/116/)
          HDFS-4350. Make enabling of stale marking on read and write paths independent. Contributed by Andrew Wang. (Revision 1441819)

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

          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSConfigKeys.java
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSUtil.java
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeManager.java
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/HeartbeatManager.java
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/resources/hdfs-default.xml
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestGetBlocks.java
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestReplicationPolicy.java
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/metrics/TestNameNodeMetrics.java
          Show
          Hudson added a comment - Integrated in Hadoop-Yarn-trunk #116 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/116/ ) HDFS-4350 . Make enabling of stale marking on read and write paths independent. Contributed by Andrew Wang. (Revision 1441819) Result = SUCCESS suresh : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1441819 Files : /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSConfigKeys.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSUtil.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeManager.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/HeartbeatManager.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/resources/hdfs-default.xml /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestGetBlocks.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestReplicationPolicy.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/metrics/TestNameNodeMetrics.java
          Hide
          Suresh Srinivas added a comment -

          I committed the patch to trunk, branch-2 and branch-1.

          Thank you Andrew!

          Show
          Suresh Srinivas added a comment - I committed the patch to trunk, branch-2 and branch-1. Thank you Andrew!
          Hide
          Hudson added a comment -

          Integrated in Hadoop-trunk-Commit #3315 (See https://builds.apache.org/job/Hadoop-trunk-Commit/3315/)
          HDFS-4350. Make enabling of stale marking on read and write paths independent. Contributed by Andrew Wang. (Revision 1441819)

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

          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSConfigKeys.java
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSUtil.java
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeManager.java
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/HeartbeatManager.java
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/resources/hdfs-default.xml
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestGetBlocks.java
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestReplicationPolicy.java
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/metrics/TestNameNodeMetrics.java
          Show
          Hudson added a comment - Integrated in Hadoop-trunk-Commit #3315 (See https://builds.apache.org/job/Hadoop-trunk-Commit/3315/ ) HDFS-4350 . Make enabling of stale marking on read and write paths independent. Contributed by Andrew Wang. (Revision 1441819) Result = SUCCESS suresh : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1441819 Files : /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSConfigKeys.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSUtil.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeManager.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/HeartbeatManager.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/resources/hdfs-default.xml /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestGetBlocks.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestReplicationPolicy.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/metrics/TestNameNodeMetrics.java
          Hide
          Suresh Srinivas added a comment -

          +1 for the branch-1 patch as well.

          Show
          Suresh Srinivas added a comment - +1 for the branch-1 patch as well.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12567307/hdfs-4350-branch-1-3.patch
          against trunk revision .

          -1 patch. The patch command could not apply the patch.

          Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/3914//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/12567307/hdfs-4350-branch-1-3.patch against trunk revision . -1 patch . The patch command could not apply the patch. Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/3914//console This message is automatically generated.
          Hide
          Andrew Wang added a comment -

          Nice catch Jing. I changed it to use the static function. Ran TestReplicationPolicy,TestGetBlocks which passed fine.

          Show
          Andrew Wang added a comment - Nice catch Jing. I changed it to use the static function. Ran TestReplicationPolicy,TestGetBlocks which passed fine.
          Hide
          Jing Zhao added a comment -

          The branch-1 patch looks good to me. One minor issue: FSNamesystem#getRatioUseStaleNodesForWriteFromConf is never used. You may either use it to get the value of ratioUseStaleDataNodesForWrite from configuration, or remove it but add some similar value checking for ratioUseStaleDataNodesForWrite.

          Besides, I've run ant test locally for the patch, and all the tests passed except TestSubmitJob and TestJvmReuse, which should be un-related.

          Show
          Jing Zhao added a comment - The branch-1 patch looks good to me. One minor issue: FSNamesystem#getRatioUseStaleNodesForWriteFromConf is never used. You may either use it to get the value of ratioUseStaleDataNodesForWrite from configuration, or remove it but add some similar value checking for ratioUseStaleDataNodesForWrite. Besides, I've run ant test locally for the patch, and all the tests passed except TestSubmitJob and TestJvmReuse, which should be un-related.
          Hide
          Suresh Srinivas added a comment -

          Andrew Wang +1 for the trunk patch. I will review the branch-1 patch shortly.

          Show
          Suresh Srinivas added a comment - Andrew Wang +1 for the trunk patch. I will review the branch-1 patch shortly.
          Hide
          Matt Foley added a comment -

          I think this is a good change, although introducing a minor incompatibility.
          1.2 will have other incompatible changes, so it should be okay to add this to the branch-1 line.
          Please do not put it in the branch-1.1 line. Thanks.

          Show
          Matt Foley added a comment - I think this is a good change, although introducing a minor incompatibility. 1.2 will have other incompatible changes, so it should be okay to add this to the branch-1 line. Please do not put it in the branch-1.1 line. Thanks.
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12566341/hdfs-4350-7.patch
          against trunk revision .

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

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

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

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

          +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 hadoop-hdfs-project/hadoop-hdfs.

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

          Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/3877//testReport/
          Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/3877//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/12566341/hdfs-4350-7.patch against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 3 new or modified test files. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . The javadoc tool did not generate any warning messages. +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 hadoop-hdfs-project/hadoop-hdfs. +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/3877//testReport/ Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/3877//console This message is automatically generated.
          Hide
          Suresh Srinivas added a comment -

          Marking this jira incompatible.

          Show
          Suresh Srinivas added a comment - Marking this jira incompatible.
          Hide
          Andrew Wang added a comment -

          Rev both patches. We were incorrectly using < instead of <= when doing the ratio comparison, which made the HalfStale test fail. Tested by running TestReplicationPolicy for both.

          Show
          Andrew Wang added a comment - Rev both patches. We were incorrectly using < instead of <= when doing the ratio comparison, which made the HalfStale test fail. Tested by running TestReplicationPolicy for both.
          Hide
          Nicolas Liochon added a comment -

          yep, I confirm there is no code change in HBase.

          Show
          Nicolas Liochon added a comment - yep, I confirm there is no code change in HBase.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12566226/hdfs-4350-6.patch
          against trunk revision .

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

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

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

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

          +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 failed these unit tests in hadoop-hdfs-project/hadoop-hdfs:

          org.apache.hadoop.hdfs.server.blockmanagement.TestReplicationPolicy

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

          Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/3876//testReport/
          Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/3876//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/12566226/hdfs-4350-6.patch against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 3 new or modified test files. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . The javadoc tool did not generate any warning messages. +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 failed these unit tests in hadoop-hdfs-project/hadoop-hdfs: org.apache.hadoop.hdfs.server.blockmanagement.TestReplicationPolicy +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/3876//testReport/ Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/3876//console This message is automatically generated.
          Hide
          Andrew Wang added a comment -

          branch-1 backport posted. Tested via running TestReplicationPolicy and TestGetBlocks. Also slightly updated the existing trunk patch with more explicit heartbeatChecks in TestReplicationPolicy.

          Show
          Andrew Wang added a comment - branch-1 backport posted. Tested via running TestReplicationPolicy and TestGetBlocks. Also slightly updated the existing trunk patch with more explicit heartbeatChecks in TestReplicationPolicy.
          Hide
          Andrew Wang added a comment -

          Hey Suresh, plan sounds good. I'll spin up a branch-1 patch and ping Matt about it. I don't have a usecase for making these independent, but it's nice to remove the configuration dependency and make the name more clear.

          My understanding is that no code changes should be needed on the HBase side, but perhaps Nicolas Liochon can confirm.

          Show
          Andrew Wang added a comment - Hey Suresh, plan sounds good. I'll spin up a branch-1 patch and ping Matt about it. I don't have a usecase for making these independent, but it's nice to remove the configuration dependency and make the name more clear. My understanding is that no code changes should be needed on the HBase side, but perhaps Nicolas Liochon can confirm.
          Hide
          Nicolas Liochon added a comment -

          For HBase, it's not a big deal:

          • in all case, both write & read path needs to use the stale mode.
          • the burden is for the HDFS admin, as it needs to be configured in HDFS.
          Show
          Nicolas Liochon added a comment - For HBase, it's not a big deal: in all case, both write & read path needs to use the stale mode. the burden is for the HDFS admin, as it needs to be configured in HDFS.
          Hide
          Suresh Srinivas added a comment -

          One additional comment - if the config name is changed, it should go into both branch-1 and branch-2. That way the migration from 1.x release to 2.x is not complicated due to the need to change config name.

          Show
          Suresh Srinivas added a comment - One additional comment - if the config name is changed, it should go into both branch-1 and branch-2. That way the migration from 1.x release to 2.x is not complicated due to the need to change config name.
          Hide
          Suresh Srinivas added a comment -

          Suresh, what's our policy on changing config strings and behavior for alpha and beta releases? The current name would be misleading, and it'd be kind of unfortunate to have to deprecate it already.

          I think this is a good change. Out of curiosity, is there a use case for using stale datanode information for write path alone (which is what currently not supported).

          Changing configuration name and semantics will be an incompatible change. I am okay making the assumption that this was used only for HBase MTTR work for now and make these changes both in branch-2 and branch-2. For branch-1, it is good to loop in the release manager Matt Foley. Given 2.0.2 is marked alpha, I am okay letting this incompatible change into 2.0.3.

          Additionally, related change needs to go into HBase as well, related to HBASE-5843. Not sure the backward compatibility impact on HBase.

          Show
          Suresh Srinivas added a comment - Suresh, what's our policy on changing config strings and behavior for alpha and beta releases? The current name would be misleading, and it'd be kind of unfortunate to have to deprecate it already. I think this is a good change. Out of curiosity, is there a use case for using stale datanode information for write path alone (which is what currently not supported). Changing configuration name and semantics will be an incompatible change. I am okay making the assumption that this was used only for HBase MTTR work for now and make these changes both in branch-2 and branch-2. For branch-1, it is good to loop in the release manager Matt Foley . Given 2.0.2 is marked alpha, I am okay letting this incompatible change into 2.0.3. Additionally, related change needs to go into HBase as well, related to HBASE-5843 . Not sure the backward compatibility impact on HBase.
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12566107/hdfs-4350-5.patch
          against trunk revision .

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

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

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

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

          +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 hadoop-hdfs-project/hadoop-hdfs.

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

          Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/3872//testReport/
          Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/3872//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/12566107/hdfs-4350-5.patch against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 3 new or modified test files. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . The javadoc tool did not generate any warning messages. +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 hadoop-hdfs-project/hadoop-hdfs. +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/3872//testReport/ Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/3872//console This message is automatically generated.
          Hide
          Andrew Wang added a comment -

          Thanks for reviews, Aaron and Jing. Improved the comment and removed the extra import, and axed some trailing whitespace.

          Suresh, what's our policy on changing config strings and behavior for alpha and beta releases? The current name would be misleading, and it'd be kind of unfortunate to have to deprecate it already.

          Show
          Andrew Wang added a comment - Thanks for reviews, Aaron and Jing. Improved the comment and removed the extra import, and axed some trailing whitespace. Suresh, what's our policy on changing config strings and behavior for alpha and beta releases? The current name would be misleading, and it'd be kind of unfortunate to have to deprecate it already.
          Hide
          Jing Zhao added a comment -

          The patch also looks good to me. One minor issue: the unused "import com.google.common.base.Preconditions;" can be removed from HeartbeatManager.java.

          Show
          Jing Zhao added a comment - The patch also looks good to me. One minor issue: the unused "import com.google.common.base.Preconditions;" can be removed from HeartbeatManager.java.
          Hide
          Suresh Srinivas added a comment -

          This change was made available in 2.0.2-alpha and has been backported to 1.1.0. So changing the configuration name and semantics will have impact on those releases. How is it going to be handled?

          Show
          Suresh Srinivas added a comment - This change was made available in 2.0.2-alpha and has been backported to 1.1.0. So changing the configuration name and semantics will have impact on those releases. How is it going to be handled?
          Hide
          Aaron T. Myers added a comment -

          Patch looks pretty good to me, and I agree that the test failure seems unrelated. One little comment, this doesn't parse so well:

          // When the number stale datanodes marked as stale reaches this ratio,

          +1 once this is addressed.

          Show
          Aaron T. Myers added a comment - Patch looks pretty good to me, and I agree that the test failure seems unrelated. One little comment, this doesn't parse so well: // When the number stale datanodes marked as stale reaches this ratio, +1 once this is addressed.
          Hide
          Andrew Wang added a comment -

          Todd's patch looks good to me. I ran the failed tests a couple times locally and they passed, and earlier run on this jira were fine.

          Show
          Andrew Wang added a comment - Todd's patch looks good to me. I ran the failed tests a couple times locally and they passed, and earlier run on this jira were fine.
          Hide
          Hadoop QA added a comment -

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

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

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

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

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

          +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 failed these unit tests in hadoop-hdfs-project/hadoop-hdfs:

          org.apache.hadoop.hdfs.server.blockmanagement.TestBlockManager

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

          Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/3845//testReport/
          Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/3845//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/12565057/hdfs-4350.txt against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 3 new or modified test files. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . The javadoc tool did not generate any warning messages. +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 failed these unit tests in hadoop-hdfs-project/hadoop-hdfs: org.apache.hadoop.hdfs.server.blockmanagement.TestBlockManager +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/3845//testReport/ Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/3845//console This message is automatically generated.
          Hide
          Todd Lipcon added a comment -

          Andrew's on vacation, so I took a whack at implementing the above suggestion against his patch. This patch seems to pass the TestReplicationPolicy tests. I also ran that test through jcarder to double check there weren't any lock inversions.

          Show
          Todd Lipcon added a comment - Andrew's on vacation, so I took a whack at implementing the above suggestion against his patch. This patch seems to pass the TestReplicationPolicy tests. I also ran that test through jcarder to double check there weren't any lock inversions.
          Hide
          Todd Lipcon added a comment -

          Hey Andrew. I took another look at this and think it can be cleaned up a bit more... having two separate booleans (and respective getters) for isAvoidingStaleDataNodesForWrite vs shouldAvoidStaleDatanodesForWrite is pretty messy.

          What if we did the following?

          • kill off the boolean for isAvoidingStaleDatanodesForWrite
          • change heartbeatCheck to always check stale nodes. It seems like there is practically zero cost to this. In the usual case where the heartbeat check does not find a dead node, we already iterate over the whole list of nodes. Checking isStale() is just a comparison, so essentially free.
          • then DatanodeManager only needs to expose the current state: shouldAvoidStaleDataNodesForWrite, which would be implemented something like":
          return avoidStaleDatanodesForWrite && numStaleNodes < datanodeMap.size() * ratioUseStaleDatanodesForWrite
          

          (or still keep a cached boolean inside DatanodeManager, but set inside setNumStaleNodes)

          That also allows you to move the ratio into DatanodeManager, which makes more sense in my opinion. I think all these changes together could allow you to remove the reference backward from HeartbeatManager into DatanodeManager - the circular reference between the classes is the thing that smells bad to me.

          Show
          Todd Lipcon added a comment - Hey Andrew. I took another look at this and think it can be cleaned up a bit more... having two separate booleans (and respective getters) for isAvoidingStaleDataNodesForWrite vs shouldAvoidStaleDatanodesForWrite is pretty messy. What if we did the following? kill off the boolean for isAvoidingStaleDatanodesForWrite change heartbeatCheck to always check stale nodes. It seems like there is practically zero cost to this. In the usual case where the heartbeat check does not find a dead node, we already iterate over the whole list of nodes. Checking isStale() is just a comparison, so essentially free. then DatanodeManager only needs to expose the current state: shouldAvoidStaleDataNodesForWrite , which would be implemented something like": return avoidStaleDatanodesForWrite && numStaleNodes < datanodeMap.size() * ratioUseStaleDatanodesForWrite (or still keep a cached boolean inside DatanodeManager, but set inside setNumStaleNodes) That also allows you to move the ratio into DatanodeManager, which makes more sense in my opinion. I think all these changes together could allow you to remove the reference backward from HeartbeatManager into DatanodeManager - the circular reference between the classes is the thing that smells bad to me.
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12564544/hdfs-4350-4.patch
          against trunk revision .

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

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

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

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

          +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 hadoop-hdfs-project/hadoop-hdfs.

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

          Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/3829//testReport/
          Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/3829//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/12564544/hdfs-4350-4.patch against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 3 new or modified test files. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . The javadoc tool did not generate any warning messages. +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 hadoop-hdfs-project/hadoop-hdfs. +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/3829//testReport/ Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/3829//console This message is automatically generated.
          Hide
          Andrew Wang added a comment -

          Switched the config strings in javadoc over to @link, based on Jing's advice.

          Show
          Andrew Wang added a comment - Switched the config strings in javadoc over to @link , based on Jing's advice.
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12563793/hdfs-4350-3.patch
          against trunk revision .

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

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

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

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

          +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 hadoop-hdfs-project/hadoop-hdfs.

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

          Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/3797//testReport/
          Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/3797//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/12563793/hdfs-4350-3.patch against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 3 new or modified test files. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . The javadoc tool did not generate any warning messages. +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 hadoop-hdfs-project/hadoop-hdfs. +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/3797//testReport/ Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/3797//console This message is automatically generated.
          Hide
          Jing Zhao added a comment -

          Config strings are still unfortunately hardcoded in the javadoc comments

          How about using

          {@link DFSConfigKeys#xxxx_KEY}

          ?

          Show
          Jing Zhao added a comment - Config strings are still unfortunately hardcoded in the javadoc comments How about using {@link DFSConfigKeys#xxxx_KEY} ?
          Hide
          Andrew Wang added a comment -

          Thanks for the review Todd, think I hit all your feedback. Config strings are still unfortunately hardcoded in the javadoc comments, but I don't know how to avoid that.

          Show
          Andrew Wang added a comment - Thanks for the review Todd, think I hit all your feedback. Config strings are still unfortunately hardcoded in the javadoc comments, but I don't know how to avoid that.
          Hide
          Todd Lipcon added a comment -
          +   * Set the value of {@link DatanodeManager#isAvoidingStaleDataNodesForWrite}.
          +   * The HeartbeatManager disables write avoidance when more than
          +   * dfs.namenode.write.stale.datanode.ratio of DataNodes are marked as stale.
          

          The double negative here is confusing. I think better to say: "The HeartbeatManager will allow writes to stale datanodes when more than ..."


          +  boolean getCheckForStaleDataNodes() {
          

          I think for a boolean-type return value, something like shouldCheckForStale...} is better than {{getCheckForStale...


             HeartbeatManager(final Namesystem namesystem,
          -      final BlockManager blockManager, final Configuration conf) {
          +      final BlockManager blockManager, final Configuration conf,
          +      final boolean avoidStaleDataNodesForWrite, final long staleInterval) {
          

          Not a fan of proliferating constructor parameters here. Since we already have the conf, and those two new parameters just come from the conf, I think the earlier approach was better (having both classes access the conf).


          +      this.heartbeatRecheckInterval = staleInterval;
          +      LOG.info("Setting hearbeat interval to " + staleInterval
          +          + " since dfs.namenode.stale.datanode.interval < "
          +          + "dfs.namenode.heartbeat.recheck-interval");
          

          This info message is a little unclear – the heartbeat interval isn't actually being changed, just the interval at which the HeartbeatManager wakes up to check for expired DNs. The message makes it sound like the datanodes will heartbeat more often, but in fact it's only an NN-side frequency that's being clamped down.

          Also, please interpolate the BLAH_BLAH_KEY constants instead of actually writing the configuration keys in the log message.


          +    Indicate whether or not to avoid reading from "stale" datanodes whose
          

          Should use " here, for valid XML.

          Show
          Todd Lipcon added a comment - + * Set the value of {@link DatanodeManager#isAvoidingStaleDataNodesForWrite}. + * The HeartbeatManager disables write avoidance when more than + * dfs.namenode.write.stale.datanode.ratio of DataNodes are marked as stale. The double negative here is confusing. I think better to say: "The HeartbeatManager will allow writes to stale datanodes when more than ..." + boolean getCheckForStaleDataNodes() { I think for a boolean-type return value, something like shouldCheckForStale...} is better than {{getCheckForStale... HeartbeatManager( final Namesystem namesystem, - final BlockManager blockManager, final Configuration conf) { + final BlockManager blockManager, final Configuration conf, + final boolean avoidStaleDataNodesForWrite, final long staleInterval) { Not a fan of proliferating constructor parameters here. Since we already have the conf, and those two new parameters just come from the conf, I think the earlier approach was better (having both classes access the conf). + this .heartbeatRecheckInterval = staleInterval; + LOG.info( "Setting hearbeat interval to " + staleInterval + + " since dfs.namenode.stale.datanode.interval < " + + "dfs.namenode.heartbeat.recheck-interval" ); This info message is a little unclear – the heartbeat interval isn't actually being changed, just the interval at which the HeartbeatManager wakes up to check for expired DNs. The message makes it sound like the datanodes will heartbeat more often, but in fact it's only an NN-side frequency that's being clamped down. Also, please interpolate the BLAH_BLAH_KEY constants instead of actually writing the configuration keys in the log message. + Indicate whether or not to avoid reading from "stale" datanodes whose Should use " here, for valid XML.
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12563594/hdfs-4350-1.patch
          against trunk revision .

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

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

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

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

          +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 hadoop-hdfs-project/hadoop-hdfs.

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

          Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/3751//testReport/
          Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/3751//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/12563594/hdfs-4350-1.patch against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 3 new or modified test files. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . The javadoc tool did not generate any warning messages. +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 hadoop-hdfs-project/hadoop-hdfs. +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/3751//testReport/ Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/3751//console This message is automatically generated.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12563592/hdfs-4350-1.patch
          against trunk revision .

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

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

          -1 javac. The patch appears to cause the build to fail.

          Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/3750//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/12563592/hdfs-4350-1.patch against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 3 new or modified test files. -1 javac . The patch appears to cause the build to fail. Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/3750//console This message is automatically generated.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12563592/hdfs-4350-1.patch
          against trunk revision .

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

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

          -1 javac. The patch appears to cause the build to fail.

          Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/3749//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/12563592/hdfs-4350-1.patch against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 3 new or modified test files. -1 javac . The patch appears to cause the build to fail. Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/3749//console This message is automatically generated.
          Hide
          Andrew Wang added a comment -

          Tried to bump Jenkins (cancel patch, upload, submit patch?). I think it's currently ready for review though.

          Show
          Andrew Wang added a comment - Tried to bump Jenkins (cancel patch, upload, submit patch?). I think it's currently ready for review though.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12562980/hdfs-4350-1.patch
          against trunk revision .

          -1 patch. The patch command could not apply the patch.

          Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/3714//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/12562980/hdfs-4350-1.patch against trunk revision . -1 patch . The patch command could not apply the patch. Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/3714//console This message is automatically generated.
          Hide
          Andrew Wang added a comment -

          Patch makes the configurations independent. Did some refactors along the way to avoid reading config parameters in multiple places, so the patch actually removes more lines than it adds. Some renaming for consistency.

          I based this off of the patch in HDFS-4351, but my `git apply --check` indicates it applies to trunk okay too.

          Show
          Andrew Wang added a comment - Patch makes the configurations independent. Did some refactors along the way to avoid reading config parameters in multiple places, so the patch actually removes more lines than it adds. Some renaming for consistency. I based this off of the patch in HDFS-4351 , but my `git apply --check` indicates it applies to trunk okay too.

            People

            • Assignee:
              Andrew Wang
              Reporter:
              Andrew Wang
            • Votes:
              0 Vote for this issue
              Watchers:
              11 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development