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. Text File
            hdfs-4350-1.patch
            22 kB
            Andrew Wang
          2. Text File
            hdfs-4350-2.patch
            22 kB
            Andrew Wang
          3. Text File
            hdfs-4350-3.patch
            22 kB
            Andrew Wang
          4. Text File
            hdfs-4350-4.patch
            23 kB
            Andrew Wang
          5. Text File
            hdfs-4350.txt
            26 kB
            Todd Lipcon
          6. Text File
            hdfs-4350-5.patch
            26 kB
            Andrew Wang
          7. Text File
            hdfs-4350-branch-1-1.patch
            22 kB
            Andrew Wang
          8. Text File
            hdfs-4350-6.patch
            27 kB
            Andrew Wang
          9. Text File
            hdfs-4350-branch-1-2.patch
            22 kB
            Andrew Wang
          10. Text File
            hdfs-4350-7.patch
            27 kB
            Andrew Wang
          11. Text File
            hdfs-4350-branch-1-3.patch
            22 kB
            Andrew Wang

            Issue Links

            relates to

            Sub-task - The sub-task of the issue HDFS-3912 Detecting and avoiding stale datanodes for writing

            • Major - Major loss of function.
            • Closed

            Improvement - An improvement or enhancement to an existing feature or task. HDFS-3703 Decrease the datanode failure detection time

            • Major - Major loss of function.
            • Closed

              Activity

              Descending order - Click to sort in ascending order
              Hide
              Permalink
              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
              Permalink
              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
              Permalink
              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
              Permalink
              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
              Permalink
              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
              Permalink
              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
              Permalink
              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
              Permalink
              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
              Permalink
              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
              Permalink
              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
              Permalink
              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
              Permalink
              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
              Permalink
              Suresh Srinivas added a comment -

              Marking this jira incompatible.

              Show
              Suresh Srinivas added a comment - Marking this jira incompatible.
              Hide
              Permalink
              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
              Permalink
              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
              Permalink
              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
              Permalink
              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
              Permalink
              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
              Permalink
              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
              Permalink
              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
              Permalink
              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
              Permalink
              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
              Permalink
              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
              Permalink
              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
              Permalink
              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
              Permalink
              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
              Permalink
              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
              Permalink
              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
              Permalink
              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
              Permalink
              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
              Permalink
              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
              Permalink
              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
              Permalink
              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
              Permalink
              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
              Permalink
              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
              Permalink
              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
              Permalink
              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
              Permalink
              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
              Permalink
              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
              Permalink
              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
              Permalink
              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
              Permalink
              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