Details

    • Type: Sub-task
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 2.8.0, 3.0.0-alpha1
    • Component/s: test
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      Currently, the NNThroughputBenchmark test BlockReportStats relies on sorted datanodes array in the lexicographical order of datanode's xferAddr.

      • There is an assertion of datanode's xferAddr lexicographical order when filling the datanodes, see the code.
      • When searching the datanode by DatanodeInfo, it uses binary search against the datanodes array, see the code

      In DatanodeID, the xferAddr is defined as host:port. In NNThroughputBenchmark, the port is simply the index of the tiny datanode plus one.

      The problem here is that, when there are more than 9 tiny datanodes (numThreads), the lexicographical order of datanode's xferAddr will be invalid as the string value of datanode index is not in lexicographical order any more. For example,

      ...
      192.168.54.40:8
      192.168.54.40:9
      192.168.54.40:10
      192.168.54.40:11
      ...
      

      192.168.54.40:9 is greater than 192.168.54.40:10. The assertion will fail and the binary search won't work.

      The simple fix is to calculate the datanode index by port directly, instead of using binary search.

        Issue Links

          Activity

          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 5s docker + precommit patch detected.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
          +1 mvninstall 3m 3s trunk passed
          +1 compile 0m 34s trunk passed with JDK v1.8.0_60
          +1 compile 0m 32s trunk passed with JDK v1.7.0_79
          +1 checkstyle 0m 15s trunk passed
          +1 mvneclipse 0m 15s trunk passed
          -1 findbugs 1m 54s hadoop-hdfs-project/hadoop-hdfs in trunk cannot run convertXmlToText from findbugs
          +1 javadoc 1m 6s trunk passed with JDK v1.8.0_60
          +1 javadoc 1m 51s trunk passed with JDK v1.7.0_79
          +1 mvninstall 0m 39s the patch passed
          +1 compile 0m 32s the patch passed with JDK v1.8.0_60
          +1 javac 0m 32s the patch passed
          +1 compile 0m 32s the patch passed with JDK v1.7.0_79
          +1 javac 0m 32s the patch passed
          +1 checkstyle 0m 17s the patch passed
          +1 mvneclipse 0m 14s the patch passed
          +1 whitespace 0m 0s Patch has no whitespace issues.
          +1 findbugs 2m 10s the patch passed
          +1 javadoc 1m 8s the patch passed with JDK v1.8.0_60
          +1 javadoc 1m 49s the patch passed with JDK v1.7.0_79
          -1 unit 50m 37s hadoop-hdfs in the patch failed with JDK v1.8.0_60.
          +1 unit 49m 50s hadoop-hdfs in the patch passed with JDK v1.7.0_79.
          -1 asflicense 0m 19s Patch generated 56 ASF License warnings.
          120m 14s



          Reason Tests
          JDK v1.8.0_60 Failed junit tests hadoop.hdfs.server.balancer.TestBalancerWithMultipleNameNodes
            hadoop.hdfs.server.balancer.TestBalancerWithSaslDataTransfer



          Subsystem Report/Notes
          Docker Client=1.7.1 Server=1.7.1 Image:test-patch-base-hadoop-date2015-11-05
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12770741/HDFS-9379.000.patch
          JIRA Issue HDFS-9379
          Optional Tests asflicense javac javadoc mvninstall unit findbugs checkstyle compile
          uname Linux 9f2461e5ff8e 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /home/jenkins/jenkins-slave/workspace/PreCommit-HDFS-Build@2/patchprocess/apache-yetus-e8bd3ad/precommit/personality/hadoop.sh
          git revision trunk / 5667129
          Default Java 1.7.0_79
          Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_60 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_79
          findbugs v3.0.0
          findbugs https://builds.apache.org/job/PreCommit-HDFS-Build/13396/artifact/patchprocess/branch-findbugs-hadoop-hdfs-project_hadoop-hdfs.html
          unit https://builds.apache.org/job/PreCommit-HDFS-Build/13396/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_60.txt
          unit test logs https://builds.apache.org/job/PreCommit-HDFS-Build/13396/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_60.txt
          JDK v1.7.0_79 Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/13396/testReport/
          asflicense https://builds.apache.org/job/PreCommit-HDFS-Build/13396/artifact/patchprocess/patch-asflicense-problems.txt
          modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs
          Max memory used 227MB
          Powered by Apache Yetus http://yetus.apache.org
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/13396/console

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 5s docker + precommit patch detected. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. +1 mvninstall 3m 3s trunk passed +1 compile 0m 34s trunk passed with JDK v1.8.0_60 +1 compile 0m 32s trunk passed with JDK v1.7.0_79 +1 checkstyle 0m 15s trunk passed +1 mvneclipse 0m 15s trunk passed -1 findbugs 1m 54s hadoop-hdfs-project/hadoop-hdfs in trunk cannot run convertXmlToText from findbugs +1 javadoc 1m 6s trunk passed with JDK v1.8.0_60 +1 javadoc 1m 51s trunk passed with JDK v1.7.0_79 +1 mvninstall 0m 39s the patch passed +1 compile 0m 32s the patch passed with JDK v1.8.0_60 +1 javac 0m 32s the patch passed +1 compile 0m 32s the patch passed with JDK v1.7.0_79 +1 javac 0m 32s the patch passed +1 checkstyle 0m 17s the patch passed +1 mvneclipse 0m 14s the patch passed +1 whitespace 0m 0s Patch has no whitespace issues. +1 findbugs 2m 10s the patch passed +1 javadoc 1m 8s the patch passed with JDK v1.8.0_60 +1 javadoc 1m 49s the patch passed with JDK v1.7.0_79 -1 unit 50m 37s hadoop-hdfs in the patch failed with JDK v1.8.0_60. +1 unit 49m 50s hadoop-hdfs in the patch passed with JDK v1.7.0_79. -1 asflicense 0m 19s Patch generated 56 ASF License warnings. 120m 14s Reason Tests JDK v1.8.0_60 Failed junit tests hadoop.hdfs.server.balancer.TestBalancerWithMultipleNameNodes   hadoop.hdfs.server.balancer.TestBalancerWithSaslDataTransfer Subsystem Report/Notes Docker Client=1.7.1 Server=1.7.1 Image:test-patch-base-hadoop-date2015-11-05 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12770741/HDFS-9379.000.patch JIRA Issue HDFS-9379 Optional Tests asflicense javac javadoc mvninstall unit findbugs checkstyle compile uname Linux 9f2461e5ff8e 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /home/jenkins/jenkins-slave/workspace/PreCommit-HDFS-Build@2/patchprocess/apache-yetus-e8bd3ad/precommit/personality/hadoop.sh git revision trunk / 5667129 Default Java 1.7.0_79 Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_60 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_79 findbugs v3.0.0 findbugs https://builds.apache.org/job/PreCommit-HDFS-Build/13396/artifact/patchprocess/branch-findbugs-hadoop-hdfs-project_hadoop-hdfs.html unit https://builds.apache.org/job/PreCommit-HDFS-Build/13396/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_60.txt unit test logs https://builds.apache.org/job/PreCommit-HDFS-Build/13396/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_60.txt JDK v1.7.0_79 Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/13396/testReport/ asflicense https://builds.apache.org/job/PreCommit-HDFS-Build/13396/artifact/patchprocess/patch-asflicense-problems.txt modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs Max memory used 227MB Powered by Apache Yetus http://yetus.apache.org Console output https://builds.apache.org/job/PreCommit-HDFS-Build/13396/console This message was automatically generated.
          Hide
          liuml07 Mingliang Liu added a comment -

          Failing tests seem not related. findbugs and asflicense issues are known.

          Show
          liuml07 Mingliang Liu added a comment - Failing tests seem not related. findbugs and asflicense issues are known.
          Hide
          arpitagarwal Arpit Agarwal added a comment -

          +1 for the patch. Did you get a chance to test it manually? The unit test Test NNThroughputBenchmark looks inadequate. It passed even when I replaced the dnIdx computation with zero.

          I looked through the remaining usages of datanodes for any dependencies on lexical ordering and didn't find any.

          Show
          arpitagarwal Arpit Agarwal added a comment - +1 for the patch. Did you get a chance to test it manually? The unit test Test NNThroughputBenchmark looks inadequate. It passed even when I replaced the dnIdx computation with zero. I looked through the remaining usages of datanodes for any dependencies on lexical ordering and didn't find any.
          Hide
          liuml07 Mingliang Liu added a comment -

          Thanks for your review Arpit Agarwal.

          Did you get a chance to test it manually?

          Yes I did test this manually. I ran the test with different combination of arguments, including -namenode, -datanodes and blocksPerReport. If the -datatnodes is greater than 9, the trunk code will run the benchmark successfully with this patch, and it will fail without this patch. The failing code is the assertion which checks the lexicographical order of datanodes.

          The unit test Test NNThroughputBenchmark looks inadequate. It passed even when I replaced the dnIdx computation with zero.

          The TestNNThroughputBenchmark seems a driver to run the benchmark, instead of unit testing the benchmark itself. Thus I did not change it. If we make the dnIdx always as zero when searching the datatnode index in datanodes array given datanode info, the test could pass as the generated block will always be added to the first datanode. The benchmark itself allows this, though the test results will be dubious.

          I looked through the remaining usages of datanodes for any dependencies on lexical ordering and didn't find any.

          That's true. The BlockReportStats is the only use case I found that depends on the lexical ordering of datanodes array. I ran other tests and they look good when the -datanodes or -threads is greater than 10.

          Show
          liuml07 Mingliang Liu added a comment - Thanks for your review Arpit Agarwal . Did you get a chance to test it manually? Yes I did test this manually. I ran the test with different combination of arguments, including -namenode , -datanodes and blocksPerReport . If the -datatnodes is greater than 9, the trunk code will run the benchmark successfully with this patch, and it will fail without this patch. The failing code is the assertion which checks the lexicographical order of datanodes. The unit test Test NNThroughputBenchmark looks inadequate. It passed even when I replaced the dnIdx computation with zero. The TestNNThroughputBenchmark seems a driver to run the benchmark, instead of unit testing the benchmark itself. Thus I did not change it. If we make the dnIdx always as zero when searching the datatnode index in datanodes array given datanode info, the test could pass as the generated block will always be added to the first datanode. The benchmark itself allows this, though the test results will be dubious. I looked through the remaining usages of datanodes for any dependencies on lexical ordering and didn't find any. That's true. The BlockReportStats is the only use case I found that depends on the lexical ordering of datanodes array. I ran other tests and they look good when the -datanodes or -threads is greater than 10.
          Hide
          arpitagarwal Arpit Agarwal added a comment -

          Thanks for confirming you tested it manually. I will commit this shortly.

          Show
          arpitagarwal Arpit Agarwal added a comment - Thanks for confirming you tested it manually. I will commit this shortly.
          Hide
          arpitagarwal Arpit Agarwal added a comment -

          Committed for 2.8.0. Thanks for the contribution Mingliang Liu.

          Show
          arpitagarwal Arpit Agarwal added a comment - Committed for 2.8.0. Thanks for the contribution Mingliang Liu .
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in Hadoop-trunk-Commit #8770 (See https://builds.apache.org/job/Hadoop-trunk-Commit/8770/)
          HDFS-9379. Make NNThroughputBenchmark support more than 10 datanodes. (arp: rev 2801b42a7e178ad6a0e6b0f29f22f3571969c530)

          • hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/NNThroughputBenchmark.java
          Show
          hudson Hudson added a comment - FAILURE: Integrated in Hadoop-trunk-Commit #8770 (See https://builds.apache.org/job/Hadoop-trunk-Commit/8770/ ) HDFS-9379 . Make NNThroughputBenchmark support more than 10 datanodes. (arp: rev 2801b42a7e178ad6a0e6b0f29f22f3571969c530) hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/NNThroughputBenchmark.java
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in Hadoop-Mapreduce-trunk-Java8 #639 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Java8/639/)
          HDFS-9379. Make NNThroughputBenchmark support more than 10 datanodes. (arp: rev 2801b42a7e178ad6a0e6b0f29f22f3571969c530)

          • hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/NNThroughputBenchmark.java
          Show
          hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Mapreduce-trunk-Java8 #639 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Java8/639/ ) HDFS-9379 . Make NNThroughputBenchmark support more than 10 datanodes. (arp: rev 2801b42a7e178ad6a0e6b0f29f22f3571969c530) hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/NNThroughputBenchmark.java
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in Hadoop-Mapreduce-trunk #2579 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/2579/)
          HDFS-9379. Make NNThroughputBenchmark support more than 10 datanodes. (arp: rev 2801b42a7e178ad6a0e6b0f29f22f3571969c530)

          • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/NNThroughputBenchmark.java
          • hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          Show
          hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Mapreduce-trunk #2579 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/2579/ ) HDFS-9379 . Make NNThroughputBenchmark support more than 10 datanodes. (arp: rev 2801b42a7e178ad6a0e6b0f29f22f3571969c530) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/NNThroughputBenchmark.java hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in Hadoop-Yarn-trunk-Java8 #649 (See https://builds.apache.org/job/Hadoop-Yarn-trunk-Java8/649/)
          HDFS-9379. Make NNThroughputBenchmark support more than 10 datanodes. (arp: rev 2801b42a7e178ad6a0e6b0f29f22f3571969c530)

          • hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/NNThroughputBenchmark.java
          Show
          hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Yarn-trunk-Java8 #649 (See https://builds.apache.org/job/Hadoop-Yarn-trunk-Java8/649/ ) HDFS-9379 . Make NNThroughputBenchmark support more than 10 datanodes. (arp: rev 2801b42a7e178ad6a0e6b0f29f22f3571969c530) hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/NNThroughputBenchmark.java
          Hide
          hudson Hudson added a comment -

          SUCCESS: Integrated in Hadoop-Yarn-trunk #1372 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/1372/)
          HDFS-9379. Make NNThroughputBenchmark support more than 10 datanodes. (arp: rev 2801b42a7e178ad6a0e6b0f29f22f3571969c530)

          • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/NNThroughputBenchmark.java
          • hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          Show
          hudson Hudson added a comment - SUCCESS: Integrated in Hadoop-Yarn-trunk #1372 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/1372/ ) HDFS-9379 . Make NNThroughputBenchmark support more than 10 datanodes. (arp: rev 2801b42a7e178ad6a0e6b0f29f22f3571969c530) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/NNThroughputBenchmark.java hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in Hadoop-Hdfs-trunk-Java8 #580 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Java8/580/)
          HDFS-9379. Make NNThroughputBenchmark support more than 10 datanodes. (arp: rev 2801b42a7e178ad6a0e6b0f29f22f3571969c530)

          • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/NNThroughputBenchmark.java
          • hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          Show
          hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Hdfs-trunk-Java8 #580 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Java8/580/ ) HDFS-9379 . Make NNThroughputBenchmark support more than 10 datanodes. (arp: rev 2801b42a7e178ad6a0e6b0f29f22f3571969c530) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/NNThroughputBenchmark.java hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in Hadoop-Hdfs-trunk #2519 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/2519/)
          HDFS-9379. Make NNThroughputBenchmark support more than 10 datanodes. (arp: rev 2801b42a7e178ad6a0e6b0f29f22f3571969c530)

          • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/NNThroughputBenchmark.java
          • hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          Show
          hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Hdfs-trunk #2519 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/2519/ ) HDFS-9379 . Make NNThroughputBenchmark support more than 10 datanodes. (arp: rev 2801b42a7e178ad6a0e6b0f29f22f3571969c530) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/NNThroughputBenchmark.java hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          Hide
          liuml07 Mingliang Liu added a comment -

          Thanks for your review and commit, Arpit Agarwal!

          Show
          liuml07 Mingliang Liu added a comment - Thanks for your review and commit, Arpit Agarwal !
          Hide
          shv Konstantin Shvachko added a comment -

          It would make sense to change the default for numThreads = 3 and may be numOpsRequired in BlockReportStats just to make sure the condition is tested in the future.

          Show
          shv Konstantin Shvachko added a comment - It would make sense to change the default for numThreads = 3 and may be numOpsRequired in BlockReportStats just to make sure the condition is tested in the future.
          Hide
          liuml07 Mingliang Liu added a comment -

          Thanks for your comment Konstantin Shvachko. If the default value numThreads and numOpsRequired are chosen only for test purpose, we can change them so this patch is tested in the future.

          Show
          liuml07 Mingliang Liu added a comment - Thanks for your comment Konstantin Shvachko . If the default value numThreads and numOpsRequired are chosen only for test purpose, we can change them so this patch is tested in the future.
          Hide
          shv Konstantin Shvachko added a comment -

          Yes, the test for NNThroughputBenchmark uses default values for all operations. So changing the default number of threads for BlockReportStats will make this case tested.
          For actual benchmarking the defaults are rarely used. And if you do, you clearly see all the parameters in the output and can adjust whatever is needed.

          Show
          shv Konstantin Shvachko added a comment - Yes, the test for NNThroughputBenchmark uses default values for all operations. So changing the default number of threads for BlockReportStats will make this case tested. For actual benchmarking the defaults are rarely used. And if you do, you clearly see all the parameters in the output and can adjust whatever is needed.
          Hide
          liuml07 Mingliang Liu added a comment -

          Thanks for your confirmation. I filed HDFS-9436 to track any follow-up effort.

          Show
          liuml07 Mingliang Liu added a comment - Thanks for your confirmation. I filed HDFS-9436 to track any follow-up effort.

            People

            • Assignee:
              liuml07 Mingliang Liu
              Reporter:
              liuml07 Mingliang Liu
            • Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development