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

NameNode refresh doesn't remove DataNodes that are no longer in the allowed list

    Details

    • Target Version/s:

      Description

      If you remove a DN from NN's allowed host list (HDFS was HA) and then do NN refresh, it doesn't remove it actually and the NN UI keeps showing that node. It may try to allocate some blocks to that DN as well during an MR job. This issue is independent from DN decommission.

      To reproduce:
      1. Add a DN to dfs_hosts_allow
      2. Refresh NN
      3. Start DN. Now NN starts seeing DN.
      4. Stop DN
      5. Remove DN from dfs_hosts_allow
      6. Refresh NN -> NN is still reporting DN as being used by HDFS.

      This is different from decom because there DN is added to exclude list in addition to being removed from allowed list, and in that case everything works correctly.

      1. HDFS-8950.001.patch
        8 kB
        Daniel Templeton
      2. HDFS-8950.002.patch
        8 kB
        Daniel Templeton
      3. HDFS-8950.003.patch
        10 kB
        Daniel Templeton
      4. HDFS-8950.004.patch
        10 kB
        Daniel Templeton
      5. HDFS-8950.005.patch
        12 kB
        Daniel Templeton
      6. HDFS-8950.branch-2.7.patch
        12 kB
        Kihwal Lee

        Issue Links

          Activity

          Hide
          kihwal Kihwal Lee added a comment -

          Committed to branch-2.7.

          Show
          kihwal Kihwal Lee added a comment - Committed to branch-2.7.
          Hide
          daryn Daryn Sharp added a comment -

          +1 Good change for 2.7.

          Show
          daryn Daryn Sharp added a comment - +1 Good change for 2.7.
          Hide
          kihwal Kihwal Lee added a comment -

          Attaching the patch for branch-2.7. Essentially the same patch plus the above static method in the test case.

          Show
          kihwal Kihwal Lee added a comment - Attaching the patch for branch-2.7. Essentially the same patch plus the above static method in the test case.
          Hide
          kihwal Kihwal Lee added a comment -

          I tried to cherry-pick it, but TestDatanodeManager needs the following additional code.

            private static DatanodeManager mockDatanodeManager(
                FSNamesystem fsn, Configuration conf) throws IOException {
              BlockManager bm = Mockito.mock(BlockManager.class);
              DatanodeManager dm = new DatanodeManager(bm, fsn, conf);
              return dm;
            }
          

          After this, TestDecommission, TestDatanodeManager and TestHostFileManager all pass.

          Show
          kihwal Kihwal Lee added a comment - I tried to cherry-pick it, but TestDatanodeManager needs the following additional code. private static DatanodeManager mockDatanodeManager( FSNamesystem fsn, Configuration conf) throws IOException { BlockManager bm = Mockito.mock(BlockManager.class); DatanodeManager dm = new DatanodeManager(bm, fsn, conf); return dm; } After this, TestDecommission , TestDatanodeManager and TestHostFileManager all pass.
          Hide
          kihwal Kihwal Lee added a comment -

          This has been a source of a lot of confusion lately. Should be fixed in 2.7.

          Show
          kihwal Kihwal Lee added a comment - This has been a source of a lot of confusion lately. Should be fixed in 2.7.
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in Hadoop-Mapreduce-trunk #2268 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/2268/)
          HDFS-8950. NameNode refresh doesn't remove DataNodes that are no longer in the allowed list (Daniel Templeton) (cmccabe: rev b94b56806d3d6e04984e229b479f7ac15b62bbfa)

          • hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/HostFileManager.java
          • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestHostFileManager.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeManager.java
          • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestDatanodeManager.java
          • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDecommission.java
          Show
          hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Mapreduce-trunk #2268 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/2268/ ) HDFS-8950 . NameNode refresh doesn't remove DataNodes that are no longer in the allowed list (Daniel Templeton) (cmccabe: rev b94b56806d3d6e04984e229b479f7ac15b62bbfa) hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/HostFileManager.java hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestHostFileManager.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeManager.java hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestDatanodeManager.java hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDecommission.java
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in Hadoop-Mapreduce-trunk-Java8 #319 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Java8/319/)
          HDFS-8950. NameNode refresh doesn't remove DataNodes that are no longer in the allowed list (Daniel Templeton) (cmccabe: rev b94b56806d3d6e04984e229b479f7ac15b62bbfa)

          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeManager.java
          • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestHostFileManager.java
          • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestDatanodeManager.java
          • hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDecommission.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/HostFileManager.java
          Show
          hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Mapreduce-trunk-Java8 #319 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Java8/319/ ) HDFS-8950 . NameNode refresh doesn't remove DataNodes that are no longer in the allowed list (Daniel Templeton) (cmccabe: rev b94b56806d3d6e04984e229b479f7ac15b62bbfa) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeManager.java hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestHostFileManager.java hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestDatanodeManager.java hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDecommission.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/HostFileManager.java
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in Hadoop-Hdfs-trunk #2248 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/2248/)
          HDFS-8950. NameNode refresh doesn't remove DataNodes that are no longer in the allowed list (Daniel Templeton) (cmccabe: rev b94b56806d3d6e04984e229b479f7ac15b62bbfa)

          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/HostFileManager.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeManager.java
          • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDecommission.java
          • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestDatanodeManager.java
          • hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestHostFileManager.java
          Show
          hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Hdfs-trunk #2248 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/2248/ ) HDFS-8950 . NameNode refresh doesn't remove DataNodes that are no longer in the allowed list (Daniel Templeton) (cmccabe: rev b94b56806d3d6e04984e229b479f7ac15b62bbfa) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/HostFileManager.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeManager.java hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDecommission.java hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestDatanodeManager.java hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestHostFileManager.java
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in Hadoop-Hdfs-trunk-Java8 #310 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Java8/310/)
          HDFS-8950. NameNode refresh doesn't remove DataNodes that are no longer in the allowed list (Daniel Templeton) (cmccabe: rev b94b56806d3d6e04984e229b479f7ac15b62bbfa)

          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/HostFileManager.java
          • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestDatanodeManager.java
          • hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDecommission.java
          • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestHostFileManager.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeManager.java
          Show
          hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Hdfs-trunk-Java8 #310 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Java8/310/ ) HDFS-8950 . NameNode refresh doesn't remove DataNodes that are no longer in the allowed list (Daniel Templeton) (cmccabe: rev b94b56806d3d6e04984e229b479f7ac15b62bbfa) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/HostFileManager.java hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestDatanodeManager.java hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDecommission.java hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestHostFileManager.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeManager.java
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in Hadoop-Yarn-trunk #1052 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/1052/)
          HDFS-8950. NameNode refresh doesn't remove DataNodes that are no longer in the allowed list (Daniel Templeton) (cmccabe: rev b94b56806d3d6e04984e229b479f7ac15b62bbfa)

          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeManager.java
          • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDecommission.java
          • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestHostFileManager.java
          • hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestDatanodeManager.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/HostFileManager.java
          Show
          hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Yarn-trunk #1052 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/1052/ ) HDFS-8950 . NameNode refresh doesn't remove DataNodes that are no longer in the allowed list (Daniel Templeton) (cmccabe: rev b94b56806d3d6e04984e229b479f7ac15b62bbfa) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeManager.java hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDecommission.java hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestHostFileManager.java hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestDatanodeManager.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/HostFileManager.java
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in Hadoop-Yarn-trunk-Java8 #324 (See https://builds.apache.org/job/Hadoop-Yarn-trunk-Java8/324/)
          HDFS-8950. NameNode refresh doesn't remove DataNodes that are no longer in the allowed list (Daniel Templeton) (cmccabe: rev b94b56806d3d6e04984e229b479f7ac15b62bbfa)

          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/HostFileManager.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeManager.java
          • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestHostFileManager.java
          • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestDatanodeManager.java
          • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDecommission.java
          • hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          Show
          hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Yarn-trunk-Java8 #324 (See https://builds.apache.org/job/Hadoop-Yarn-trunk-Java8/324/ ) HDFS-8950 . NameNode refresh doesn't remove DataNodes that are no longer in the allowed list (Daniel Templeton) (cmccabe: rev b94b56806d3d6e04984e229b479f7ac15b62bbfa) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/HostFileManager.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeManager.java hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestHostFileManager.java hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestDatanodeManager.java hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDecommission.java hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in Hadoop-trunk-Commit #8367 (See https://builds.apache.org/job/Hadoop-trunk-Commit/8367/)
          HDFS-8950. NameNode refresh doesn't remove DataNodes that are no longer in the allowed list (Daniel Templeton) (cmccabe: rev b94b56806d3d6e04984e229b479f7ac15b62bbfa)

          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/HostFileManager.java
          • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeManager.java
          • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDecommission.java
          • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestHostFileManager.java
          • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestDatanodeManager.java
          • hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          Show
          hudson Hudson added a comment - FAILURE: Integrated in Hadoop-trunk-Commit #8367 (See https://builds.apache.org/job/Hadoop-trunk-Commit/8367/ ) HDFS-8950 . NameNode refresh doesn't remove DataNodes that are no longer in the allowed list (Daniel Templeton) (cmccabe: rev b94b56806d3d6e04984e229b479f7ac15b62bbfa) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/HostFileManager.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeManager.java hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDecommission.java hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestHostFileManager.java hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestDatanodeManager.java hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          Hide
          cmccabe Colin P. McCabe added a comment -

          +1. Thanks, Daniel Templeton.

          Show
          cmccabe Colin P. McCabe added a comment - +1. Thanks, Daniel Templeton .
          Hide
          hadoopqa Hadoop QA added a comment -



          +1 overall



          Vote Subsystem Runtime Comment
          0 pre-patch 17m 30s Pre-patch trunk compilation is healthy.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 tests included 0m 0s The patch appears to include 3 new or modified test files.
          +1 javac 7m 49s There were no new javac warning messages.
          +1 javadoc 9m 56s There were no new javadoc warning messages.
          +1 release audit 0m 22s The applied patch does not increase the total number of release audit warnings.
          +1 checkstyle 1m 21s There were no new checkstyle issues.
          +1 whitespace 0m 1s The patch has no lines that end in whitespace.
          +1 install 1m 28s mvn install still works.
          +1 eclipse:eclipse 0m 33s The patch built with eclipse:eclipse.
          +1 findbugs 2m 27s The patch does not introduce any new Findbugs (version 3.0.0) warnings.
          +1 native 3m 7s Pre-build of native portion
          +1 hdfs tests 161m 37s Tests passed in hadoop-hdfs.
              206m 14s  



          Subsystem Report/Notes
          Patch URL http://issues.apache.org/jira/secure/attachment/12752995/HDFS-8950.005.patch
          Optional Tests javadoc javac unit findbugs checkstyle
          git revision trunk / beb65c9
          hadoop-hdfs test log https://builds.apache.org/job/PreCommit-HDFS-Build/12196/artifact/patchprocess/testrun_hadoop-hdfs.txt
          Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/12196/testReport/
          Java 1.7.0_55
          uname Linux asf900.gq1.ygridcore.net 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/12196/console

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - +1 overall Vote Subsystem Runtime Comment 0 pre-patch 17m 30s Pre-patch trunk compilation is healthy. +1 @author 0m 0s The patch does not contain any @author tags. +1 tests included 0m 0s The patch appears to include 3 new or modified test files. +1 javac 7m 49s There were no new javac warning messages. +1 javadoc 9m 56s There were no new javadoc warning messages. +1 release audit 0m 22s The applied patch does not increase the total number of release audit warnings. +1 checkstyle 1m 21s There were no new checkstyle issues. +1 whitespace 0m 1s The patch has no lines that end in whitespace. +1 install 1m 28s mvn install still works. +1 eclipse:eclipse 0m 33s The patch built with eclipse:eclipse. +1 findbugs 2m 27s The patch does not introduce any new Findbugs (version 3.0.0) warnings. +1 native 3m 7s Pre-build of native portion +1 hdfs tests 161m 37s Tests passed in hadoop-hdfs.     206m 14s   Subsystem Report/Notes Patch URL http://issues.apache.org/jira/secure/attachment/12752995/HDFS-8950.005.patch Optional Tests javadoc javac unit findbugs checkstyle git revision trunk / beb65c9 hadoop-hdfs test log https://builds.apache.org/job/PreCommit-HDFS-Build/12196/artifact/patchprocess/testrun_hadoop-hdfs.txt Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/12196/testReport/ Java 1.7.0_55 uname Linux asf900.gq1.ygridcore.net 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Console output https://builds.apache.org/job/PreCommit-HDFS-Build/12196/console This message was automatically generated.
          Hide
          templedf Daniel Templeton added a comment -

          Tests are all passing. Again.

          Show
          templedf Daniel Templeton added a comment - Tests are all passing. Again.
          Hide
          cmccabe Colin P. McCabe added a comment -

          Looks good overall, but TestHostFileManager is failing in the latest patch.

          Show
          cmccabe Colin P. McCabe added a comment - Looks good overall, but TestHostFileManager is failing in the latest patch.
          Hide
          hadoopqa Hadoop QA added a comment -



          -1 overall



          Vote Subsystem Runtime Comment
          -1 pre-patch 16m 9s Findbugs (version ) appears to be broken on trunk.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 tests included 0m 0s The patch appears to include 2 new or modified test files.
          +1 javac 8m 15s There were no new javac warning messages.
          +1 javadoc 10m 1s There were no new javadoc warning messages.
          +1 release audit 0m 24s The applied patch does not increase the total number of release audit warnings.
          +1 checkstyle 0m 32s There were no new checkstyle issues.
          +1 whitespace 0m 0s The patch has no lines that end in whitespace.
          +1 install 1m 40s mvn install still works.
          +1 eclipse:eclipse 0m 34s The patch built with eclipse:eclipse.
          -1 findbugs 2m 33s The patch appears to introduce 4 new Findbugs (version 3.0.0) warnings.
          +1 native 3m 15s Pre-build of native portion
          -1 hdfs tests 73m 20s Tests failed in hadoop-hdfs.
              116m 46s  



          Reason Tests
          FindBugs module:hadoop-hdfs
          Failed unit tests hadoop.hdfs.server.blockmanagement.TestNodeCount
            hadoop.hdfs.TestBalancerBandwidth
            hadoop.hdfs.server.blockmanagement.TestHostFileManager
          Timed out tests org.apache.hadoop.hdfs.TestDFSClientRetries



          Subsystem Report/Notes
          Patch URL http://issues.apache.org/jira/secure/attachment/12752784/HDFS-8950.004.patch
          Optional Tests javadoc javac unit findbugs checkstyle
          git revision trunk / 0bf2854
          Findbugs warnings https://builds.apache.org/job/PreCommit-HDFS-Build/12166/artifact/patchprocess/newPatchFindbugsWarningshadoop-hdfs.html
          hadoop-hdfs test log https://builds.apache.org/job/PreCommit-HDFS-Build/12166/artifact/patchprocess/testrun_hadoop-hdfs.txt
          Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/12166/testReport/
          Java 1.7.0_55
          uname Linux asf909.gq1.ygridcore.net 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/12166/console

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment -1 pre-patch 16m 9s Findbugs (version ) appears to be broken on trunk. +1 @author 0m 0s The patch does not contain any @author tags. +1 tests included 0m 0s The patch appears to include 2 new or modified test files. +1 javac 8m 15s There were no new javac warning messages. +1 javadoc 10m 1s There were no new javadoc warning messages. +1 release audit 0m 24s The applied patch does not increase the total number of release audit warnings. +1 checkstyle 0m 32s There were no new checkstyle issues. +1 whitespace 0m 0s The patch has no lines that end in whitespace. +1 install 1m 40s mvn install still works. +1 eclipse:eclipse 0m 34s The patch built with eclipse:eclipse. -1 findbugs 2m 33s The patch appears to introduce 4 new Findbugs (version 3.0.0) warnings. +1 native 3m 15s Pre-build of native portion -1 hdfs tests 73m 20s Tests failed in hadoop-hdfs.     116m 46s   Reason Tests FindBugs module:hadoop-hdfs Failed unit tests hadoop.hdfs.server.blockmanagement.TestNodeCount   hadoop.hdfs.TestBalancerBandwidth   hadoop.hdfs.server.blockmanagement.TestHostFileManager Timed out tests org.apache.hadoop.hdfs.TestDFSClientRetries Subsystem Report/Notes Patch URL http://issues.apache.org/jira/secure/attachment/12752784/HDFS-8950.004.patch Optional Tests javadoc javac unit findbugs checkstyle git revision trunk / 0bf2854 Findbugs warnings https://builds.apache.org/job/PreCommit-HDFS-Build/12166/artifact/patchprocess/newPatchFindbugsWarningshadoop-hdfs.html hadoop-hdfs test log https://builds.apache.org/job/PreCommit-HDFS-Build/12166/artifact/patchprocess/testrun_hadoop-hdfs.txt Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/12166/testReport/ Java 1.7.0_55 uname Linux asf909.gq1.ygridcore.net 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Console output https://builds.apache.org/job/PreCommit-HDFS-Build/12166/console This message was automatically generated.
          Hide
          templedf Daniel Templeton added a comment -

          Here's another pass that replaces most of the Whitebox stuff with a refactored refresh() method in HostFileManager. Please review when you have a chance.

          Show
          templedf Daniel Templeton added a comment - Here's another pass that replaces most of the Whitebox stuff with a refactored refresh() method in HostFileManager. Please review when you have a chance.
          Hide
          templedf Daniel Templeton added a comment -

          Thanks! On the last point, I was erring on the side of not modifying the source tree. If adding new methods only for testing is a preferred to Mockito acrobatics, then that would simplify things quite a bit. I'll take another pass.

          Show
          templedf Daniel Templeton added a comment - Thanks! On the last point, I was erring on the side of not modifying the source tree. If adding new methods only for testing is a preferred to Mockito acrobatics, then that would simplify things quite a bit. I'll take another pass.
          Hide
          cmccabe Colin P. McCabe added a comment -

          Looks good overall.

          1288	            (includedNodes.isEmpty() || includedNodes.match(addr))) {
          

          Should be includedNodes.isIncluded, which already has the logic to treat empty include lists as "everything is included"

          427	    Assert.assertThat("Incorrect number of hosts reported",
          428	        both.size(), is(2));
          

          This could just be assertEquals(2, ...), which seems better since it would tell you "this is 3 instead of 2" or something like that rather than just telling you that it wasn't what was expected.

          	450	    System.out.println(dm.getDatanodeListForReport(HdfsConstants.DatanodeReportType.ALL));
          

          Should use the LOG variable in this test class

          405	    // Set the includes and excludes lists the hard way because the
          406	    // HostFileManager wasn't designed for TDD.  We have to also set
          407	    // excludes because it would otherwise be null, causing an NPE.
          408	    // (The refresh() method would normally fix that, but we don't call it.)
          409	    Whitebox.setInternalState(hm, "includes", twoNodes);
          410	    Whitebox.setInternalState(hm, "excludes", noNodes);
          411	    Whitebox.setInternalState(dm, "hostFileManager", hm);
          

          I disagree that the "HostFileManager wasn't designed for TDD." It had unit tests ever since it was added to the source tree. It was designed to use immutable state as much as possible in order to avoid the many locking problems that we previously had with mutable host file state inside the DatanodeManager.

          For this particular unit test, we don't need complex Mockito stuff. Why not just add a new HostFileManager#refresh method annotated with @VisibleForTesting that just takes the new includes and excludes that the unit test wants to set?

          thanks

          Show
          cmccabe Colin P. McCabe added a comment - Looks good overall. 1288 (includedNodes.isEmpty() || includedNodes.match(addr))) { Should be includedNodes.isIncluded , which already has the logic to treat empty include lists as "everything is included" 427 Assert.assertThat( "Incorrect number of hosts reported" , 428 both.size(), is(2)); This could just be assertEquals(2, ...), which seems better since it would tell you "this is 3 instead of 2" or something like that rather than just telling you that it wasn't what was expected. 450 System .out.println(dm.getDatanodeListForReport(HdfsConstants.DatanodeReportType.ALL)); Should use the LOG variable in this test class 405 // Set the includes and excludes lists the hard way because the 406 // HostFileManager wasn't designed for TDD. We have to also set 407 // excludes because it would otherwise be null , causing an NPE. 408 // (The refresh() method would normally fix that, but we don't call it.) 409 Whitebox.setInternalState(hm, "includes" , twoNodes); 410 Whitebox.setInternalState(hm, "excludes" , noNodes); 411 Whitebox.setInternalState(dm, "hostFileManager" , hm); I disagree that the "HostFileManager wasn't designed for TDD." It had unit tests ever since it was added to the source tree. It was designed to use immutable state as much as possible in order to avoid the many locking problems that we previously had with mutable host file state inside the DatanodeManager . For this particular unit test, we don't need complex Mockito stuff. Why not just add a new HostFileManager#refresh method annotated with @VisibleForTesting that just takes the new includes and excludes that the unit test wants to set? thanks
          Hide
          hadoopqa Hadoop QA added a comment -



          -1 overall



          Vote Subsystem Runtime Comment
          -1 pre-patch 17m 40s Pre-patch trunk has 4 extant Findbugs (version 3.0.0) warnings.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 tests included 0m 0s The patch appears to include 2 new or modified test files.
          +1 javac 7m 54s There were no new javac warning messages.
          +1 javadoc 10m 2s There were no new javadoc warning messages.
          +1 release audit 0m 22s The applied patch does not increase the total number of release audit warnings.
          +1 checkstyle 1m 21s There were no new checkstyle issues.
          +1 whitespace 0m 1s The patch has no lines that end in whitespace.
          +1 install 1m 26s mvn install still works.
          +1 eclipse:eclipse 0m 32s The patch built with eclipse:eclipse.
          +1 findbugs 2m 33s The patch does not introduce any new Findbugs (version 3.0.0) warnings.
          +1 native 3m 9s Pre-build of native portion
          +1 hdfs tests 163m 45s Tests passed in hadoop-hdfs.
              208m 49s  



          Subsystem Report/Notes
          Patch URL http://issues.apache.org/jira/secure/attachment/12752543/HDFS-8950.003.patch
          Optional Tests javadoc javac unit findbugs checkstyle
          git revision trunk / a4d9acc
          Pre-patch Findbugs warnings https://builds.apache.org/job/PreCommit-HDFS-Build/12141/artifact/patchprocess/trunkFindbugsWarningshadoop-hdfs.html
          hadoop-hdfs test log https://builds.apache.org/job/PreCommit-HDFS-Build/12141/artifact/patchprocess/testrun_hadoop-hdfs.txt
          Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/12141/testReport/
          Java 1.7.0_55
          uname Linux asf900.gq1.ygridcore.net 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/12141/console

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment -1 pre-patch 17m 40s Pre-patch trunk has 4 extant Findbugs (version 3.0.0) warnings. +1 @author 0m 0s The patch does not contain any @author tags. +1 tests included 0m 0s The patch appears to include 2 new or modified test files. +1 javac 7m 54s There were no new javac warning messages. +1 javadoc 10m 2s There were no new javadoc warning messages. +1 release audit 0m 22s The applied patch does not increase the total number of release audit warnings. +1 checkstyle 1m 21s There were no new checkstyle issues. +1 whitespace 0m 1s The patch has no lines that end in whitespace. +1 install 1m 26s mvn install still works. +1 eclipse:eclipse 0m 32s The patch built with eclipse:eclipse. +1 findbugs 2m 33s The patch does not introduce any new Findbugs (version 3.0.0) warnings. +1 native 3m 9s Pre-build of native portion +1 hdfs tests 163m 45s Tests passed in hadoop-hdfs.     208m 49s   Subsystem Report/Notes Patch URL http://issues.apache.org/jira/secure/attachment/12752543/HDFS-8950.003.patch Optional Tests javadoc javac unit findbugs checkstyle git revision trunk / a4d9acc Pre-patch Findbugs warnings https://builds.apache.org/job/PreCommit-HDFS-Build/12141/artifact/patchprocess/trunkFindbugsWarningshadoop-hdfs.html hadoop-hdfs test log https://builds.apache.org/job/PreCommit-HDFS-Build/12141/artifact/patchprocess/testrun_hadoop-hdfs.txt Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/12141/testReport/ Java 1.7.0_55 uname Linux asf900.gq1.ygridcore.net 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Console output https://builds.apache.org/job/PreCommit-HDFS-Build/12141/console This message was automatically generated.
          Hide
          templedf Daniel Templeton added a comment -

          New patch that includes changes to TestDecommission.testHostsFile(). All tests appear to pass now.

          Show
          templedf Daniel Templeton added a comment - New patch that includes changes to TestDecommission.testHostsFile(). All tests appear to pass now.
          Hide
          templedf Daniel Templeton added a comment -

          Looking more closely at TestDecommission, the reason the two tests are failing there is that they depend on the existence of the flaw represented by this JIRA to work. Fixing the flaw breaks the tests. I'll need a little time to figure out how to restructure the tests to be independent of the flaw.

          Show
          templedf Daniel Templeton added a comment - Looking more closely at TestDecommission, the reason the two tests are failing there is that they depend on the existence of the flaw represented by this JIRA to work. Fixing the flaw breaks the tests. I'll need a little time to figure out how to restructure the tests to be independent of the flaw.
          Hide
          templedf Daniel Templeton added a comment -

          No idea why TestAuditLogs and TestShortCircuitCache are failing. They work for me locally, and they're unrelated to the change in this patch. TestDecomission is a legit fail. Looks like I missed that package when testing locally.

          Hold, please.

          Show
          templedf Daniel Templeton added a comment - No idea why TestAuditLogs and TestShortCircuitCache are failing. They work for me locally, and they're unrelated to the change in this patch. TestDecomission is a legit fail. Looks like I missed that package when testing locally. Hold, please.
          Hide
          hadoopqa Hadoop QA added a comment -



          -1 overall



          Vote Subsystem Runtime Comment
          -1 pre-patch 17m 43s Pre-patch trunk has 4 extant Findbugs (version 3.0.0) warnings.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 tests included 0m 0s The patch appears to include 1 new or modified test files.
          +1 javac 7m 51s There were no new javac warning messages.
          +1 javadoc 10m 13s There were no new javadoc warning messages.
          +1 release audit 0m 23s The applied patch does not increase the total number of release audit warnings.
          +1 checkstyle 1m 21s There were no new checkstyle issues.
          +1 whitespace 0m 0s The patch has no lines that end in whitespace.
          +1 install 1m 28s mvn install still works.
          +1 eclipse:eclipse 0m 32s The patch built with eclipse:eclipse.
          +1 findbugs 2m 31s The patch does not introduce any new Findbugs (version 3.0.0) warnings.
          +1 native 3m 8s Pre-build of native portion
          -1 hdfs tests 166m 42s Tests failed in hadoop-hdfs.
              211m 55s  



          Reason Tests
          Failed unit tests hadoop.hdfs.shortcircuit.TestShortCircuitCache
            hadoop.hdfs.TestDecommission
            hadoop.hdfs.server.namenode.TestAuditLogs



          Subsystem Report/Notes
          Patch URL http://issues.apache.org/jira/secure/attachment/12752359/HDFS-8950.002.patch
          Optional Tests javadoc javac unit findbugs checkstyle
          git revision trunk / a4d9acc
          Pre-patch Findbugs warnings https://builds.apache.org/job/PreCommit-HDFS-Build/12128/artifact/patchprocess/trunkFindbugsWarningshadoop-hdfs.html
          hadoop-hdfs test log https://builds.apache.org/job/PreCommit-HDFS-Build/12128/artifact/patchprocess/testrun_hadoop-hdfs.txt
          Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/12128/testReport/
          Java 1.7.0_55
          uname Linux asf900.gq1.ygridcore.net 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/12128/console

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment -1 pre-patch 17m 43s Pre-patch trunk has 4 extant Findbugs (version 3.0.0) warnings. +1 @author 0m 0s The patch does not contain any @author tags. +1 tests included 0m 0s The patch appears to include 1 new or modified test files. +1 javac 7m 51s There were no new javac warning messages. +1 javadoc 10m 13s There were no new javadoc warning messages. +1 release audit 0m 23s The applied patch does not increase the total number of release audit warnings. +1 checkstyle 1m 21s There were no new checkstyle issues. +1 whitespace 0m 0s The patch has no lines that end in whitespace. +1 install 1m 28s mvn install still works. +1 eclipse:eclipse 0m 32s The patch built with eclipse:eclipse. +1 findbugs 2m 31s The patch does not introduce any new Findbugs (version 3.0.0) warnings. +1 native 3m 8s Pre-build of native portion -1 hdfs tests 166m 42s Tests failed in hadoop-hdfs.     211m 55s   Reason Tests Failed unit tests hadoop.hdfs.shortcircuit.TestShortCircuitCache   hadoop.hdfs.TestDecommission   hadoop.hdfs.server.namenode.TestAuditLogs Subsystem Report/Notes Patch URL http://issues.apache.org/jira/secure/attachment/12752359/HDFS-8950.002.patch Optional Tests javadoc javac unit findbugs checkstyle git revision trunk / a4d9acc Pre-patch Findbugs warnings https://builds.apache.org/job/PreCommit-HDFS-Build/12128/artifact/patchprocess/trunkFindbugsWarningshadoop-hdfs.html hadoop-hdfs test log https://builds.apache.org/job/PreCommit-HDFS-Build/12128/artifact/patchprocess/testrun_hadoop-hdfs.txt Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/12128/testReport/ Java 1.7.0_55 uname Linux asf900.gq1.ygridcore.net 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Console output https://builds.apache.org/job/PreCommit-HDFS-Build/12128/console This message was automatically generated.
          Hide
          templedf Daniel Templeton added a comment -

          Just noticed an extraneous import in the patch. (Swing?!? WTF?) Removed and reposting patch.

          Show
          templedf Daniel Templeton added a comment - Just noticed an extraneous import in the patch. (Swing?!? WTF?) Removed and reposting patch.
          Hide
          templedf Daniel Templeton added a comment -

          Ready for review

          Show
          templedf Daniel Templeton added a comment - Ready for review
          Hide
          templedf Daniel Templeton added a comment -

          Once more with feeling.

          Show
          templedf Daniel Templeton added a comment - Once more with feeling.
          Hide
          templedf Daniel Templeton added a comment -

          Pulled the patch temporarily while test_patch is running locally. Will repost with the proper naming conventions shortly.

          Show
          templedf Daniel Templeton added a comment - Pulled the patch temporarily while test_patch is running locally. Will repost with the proper naming conventions shortly.
          Hide
          templedf Daniel Templeton added a comment -

          Here's a patch for a unit test and fix. The HostFileManager now excludes any nodes not present in a non-empty includes file from the node report. All tests in the HDFS project pass.

          Show
          templedf Daniel Templeton added a comment - Here's a patch for a unit test and fix. The HostFileManager now excludes any nodes not present in a non-empty includes file from the node report. All tests in the HDFS project pass.

            People

            • Assignee:
              templedf Daniel Templeton
              Reporter:
              templedf Daniel Templeton
            • Votes:
              0 Vote for this issue
              Watchers:
              12 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development