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

Modify NNThroughputBenchmark to be able to operate on a remote NameNode

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 2.8.0
    • Fix Version/s: 2.8.0, 2.7.4, 3.0.0-alpha1
    • Component/s: None
    • Labels:
      None
    • Target Version/s:

      Description

      Modify NNThroughputBenchmark to be able to operate on a NN that is not in process. A followon Jira will modify it some more to allow quantifying native and java heap sizes, and some latency numbers.

      1. HDFS-7847-branch-2.7.patch
        23 kB
        Konstantin Shvachko
      2. HDFS-7847.005.patch
        22 kB
        Charles Lamb
      3. HDFS-7847.004.patch
        22 kB
        Charles Lamb
      4. HDFS-7847.003.patch
        22 kB
        Charles Lamb
      5. HDFS-7847.002.patch
        21 kB
        Charles Lamb
      6. HDFS-7847.001.patch
        25 kB
        Charles Lamb
      7. HDFS-7847.000.patch
        26 kB
        Charles Lamb
      8. make_blocks.tar.gz
        34 kB
        Arpit Agarwal

        Issue Links

          Activity

          Hide
          shv Konstantin Shvachko added a comment -

          Adding HDFS-7847, HDFS-9421, and HDFS-9503 to branch-2.7.

          Show
          shv Konstantin Shvachko added a comment - Adding HDFS-7847 , HDFS-9421 , and HDFS-9503 to branch-2.7.
          Hide
          shv Konstantin Shvachko added a comment -

          I created HDFS-9503 to replace the -namenode option with -fs.
          Although I still don't understand the use case for a stand alone NameNode benchmark.

          Show
          shv Konstantin Shvachko added a comment - I created HDFS-9503 to replace the -namenode option with -fs . Although I still don't understand the use case for a stand alone NameNode benchmark.
          Hide
          cmccabe Colin P. McCabe added a comment -

          what are you measuring

          NNThroughput, measured in ops per second.

          how you set up the nodes

          Any way you like, depending on what you are benchmarking. There is no special setup needed.

          are there any particular RPC improvements or alternative implementations you are comparing

          Somewhat recently, I did some benchmarks to figure out how to tune the NN to get the most ops per second. Actually on my particular hardware, fsync turned out to be a bottleneck, since hard disk write caching was turned off. In general, fsync is not a bottleneck, but it was in my specific case. More generally, we should pay attention to benchmarks like these to make sure our performance doesn't drop.

          Show
          cmccabe Colin P. McCabe added a comment - what are you measuring NNThroughput, measured in ops per second. how you set up the nodes Any way you like, depending on what you are benchmarking. There is no special setup needed. are there any particular RPC improvements or alternative implementations you are comparing Somewhat recently, I did some benchmarks to figure out how to tune the NN to get the most ops per second. Actually on my particular hardware, fsync turned out to be a bottleneck, since hard disk write caching was turned off. In general, fsync is not a bottleneck, but it was in my specific case. More generally, we should pay attention to benchmarks like these to make sure our performance doesn't drop.
          Hide
          shv Konstantin Shvachko added a comment -

          Yes, NNThroughputBenchmark is a benchmark. Glad we established this fact.

          Now, could you (or Charles, or anybody) please explain how you plan to use it as a benchmark with a standalone NN. I mean things like

          • what are you measuring
          • how you set up the nodes
          • are there any particular RPC improvements or alternative implementations you are comparing

          Thought I was asking a simple question, but it is getting confusing.

          Show
          shv Konstantin Shvachko added a comment - Yes, NNThroughputBenchmark is a benchmark. Glad we established this fact. Now, could you (or Charles, or anybody) please explain how you plan to use it as a benchmark with a standalone NN . I mean things like what are you measuring how you set up the nodes are there any particular RPC improvements or alternative implementations you are comparing Thought I was asking a simple question, but it is getting confusing.
          Hide
          cmccabe Colin P. McCabe added a comment -

          If understand this correctly, you are not targeting to build a benchmark, but rather unit tests of NameNode RPC. Isn't there a bunch of those in HDFS source tree already? Just trying to understand what is different.

          Excuse me, I mis-spoke. NNThroughputBenchmark is a benchmark, not a unit test.

          Have you looked at org.apache.hadoop.fs.loadGenerator.LoadGenerator. Would its functionality suffice the testing needs you have in mind?

          LoadGenerator is certainly much more similar to NNThroughputBenchmark than any of the other things you mentioned. However, it performs a mix of only list, read, and write operations. NNThroughputBenchmark performs a wider range of operations. In general, I would like to avoid read and write operations in my testing of NN RPC performance, since those operations involve the DataNode heavily. I think it's good to have multiple different benchmarks testing different things.

          Show
          cmccabe Colin P. McCabe added a comment - If understand this correctly, you are not targeting to build a benchmark, but rather unit tests of NameNode RPC. Isn't there a bunch of those in HDFS source tree already? Just trying to understand what is different. Excuse me, I mis-spoke. NNThroughputBenchmark is a benchmark, not a unit test. Have you looked at org.apache.hadoop.fs.loadGenerator.LoadGenerator. Would its functionality suffice the testing needs you have in mind? LoadGenerator is certainly much more similar to NNThroughputBenchmark than any of the other things you mentioned. However, it performs a mix of only list, read, and write operations. NNThroughputBenchmark performs a wider range of operations. In general, I would like to avoid read and write operations in my testing of NN RPC performance, since those operations involve the DataNode heavily. I think it's good to have multiple different benchmarks testing different things.
          Hide
          shv Konstantin Shvachko added a comment -

          The motivation was to have a simple unit test of raw NameNode RPC throughput in the HDFS source code.

          If understand this correctly, you are not targeting to build a benchmark, but rather unit tests of NameNode RPC.
          Isn't there a bunch of those in HDFS source tree already? Just trying to understand what is different.

          DFSIO is not a test of pure NN RPC performance, since it is a MapReduce job

          It can be used to test NameNode's performance, because the MR overhead is subtracted. It is in MapReduce though, true.

          SLive is an external project

          SLive is in the same place as DFSIO. External to what?

          Have you looked at org.apache.hadoop.fs.loadGenerator.LoadGenerator. Would its functionality suffice the testing needs you have in mind?

          Show
          shv Konstantin Shvachko added a comment - The motivation was to have a simple unit test of raw NameNode RPC throughput in the HDFS source code. If understand this correctly, you are not targeting to build a benchmark, but rather unit tests of NameNode RPC. Isn't there a bunch of those in HDFS source tree already? Just trying to understand what is different. DFSIO is not a test of pure NN RPC performance, since it is a MapReduce job It can be used to test NameNode's performance, because the MR overhead is subtracted. It is in MapReduce though, true. SLive is an external project SLive is in the same place as DFSIO. External to what? Have you looked at org.apache.hadoop.fs.loadGenerator.LoadGenerator . Would its functionality suffice the testing needs you have in mind?
          Hide
          cmccabe Colin P. McCabe added a comment -

          Shouldn't we be specifying the namenode using the standard generic option -fs?

          That makes sense to me. Would you like to post a follow-on JIRA?

          I guess I did not fully understand your motivation here?

          The motivation was to have a simple unit test of raw NameNode RPC throughput in the HDFS source code. DFSIO is not a test of pure NN RPC performance, since it is a MapReduce job with all the attendant overheads. SLive is an external project not in the HDFS source tree.

          If you want to benchmark the NameNode's performance without RPC overhead, you can still do that with NNThroughputBenchmark, by using the right command-line options.

          Show
          cmccabe Colin P. McCabe added a comment - Shouldn't we be specifying the namenode using the standard generic option -fs? That makes sense to me. Would you like to post a follow-on JIRA? I guess I did not fully understand your motivation here? The motivation was to have a simple unit test of raw NameNode RPC throughput in the HDFS source code. DFSIO is not a test of pure NN RPC performance, since it is a MapReduce job with all the attendant overheads. SLive is an external project not in the HDFS source tree. If you want to benchmark the NameNode's performance without RPC overhead, you can still do that with NNThroughputBenchmark, by using the right command-line options.
          Hide
          shv Konstantin Shvachko added a comment - - edited

          Shouldn't we be specifying the namenode using the standard generic option -fs?

          Also NNThroughputBenchmark was intended to benchmark NameNode's performance without RPC overhead.
          There are other tools which can benchmark a real NameNode, like

          • DFSIO,
          • SLive,
          • synthetic load generator

          I guess I did not fully understand your motivation here?

          Show
          shv Konstantin Shvachko added a comment - - edited Shouldn't we be specifying the namenode using the standard generic option -fs ? Also NNThroughputBenchmark was intended to benchmark NameNode's performance without RPC overhead. There are other tools which can benchmark a real NameNode, like DFSIO, SLive, synthetic load generator I guess I did not fully understand your motivation here?
          Hide
          hudson Hudson added a comment -

          SUCCESS: Integrated in Hadoop-Mapreduce-trunk-Java8 #186 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Java8/186/)
          HDFS-7847. Modify NNThroughputBenchmark to be able to operate on a remote NameNode (Charles Lamb via Colin P. McCabe) (cmccabe: rev ffce9a3413277a69444fcb890460c885de56db69)

          • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/DFSTestUtil.java
          • 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-Mapreduce-trunk-Java8 #186 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Java8/186/ ) HDFS-7847 . Modify NNThroughputBenchmark to be able to operate on a remote NameNode (Charles Lamb via Colin P. McCabe) (cmccabe: rev ffce9a3413277a69444fcb890460c885de56db69) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/DFSTestUtil.java 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-Mapreduce-trunk #2135 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/2135/)
          HDFS-7847. Modify NNThroughputBenchmark to be able to operate on a remote NameNode (Charles Lamb via Colin P. McCabe) (cmccabe: rev ffce9a3413277a69444fcb890460c885de56db69)

          • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/DFSTestUtil.java
          • 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 #2135 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/2135/ ) HDFS-7847 . Modify NNThroughputBenchmark to be able to operate on a remote NameNode (Charles Lamb via Colin P. McCabe) (cmccabe: rev ffce9a3413277a69444fcb890460c885de56db69) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/DFSTestUtil.java 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-Hdfs-trunk-Java8 #176 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Java8/176/)
          HDFS-7847. Modify NNThroughputBenchmark to be able to operate on a remote NameNode (Charles Lamb via Colin P. McCabe) (cmccabe: rev ffce9a3413277a69444fcb890460c885de56db69)

          • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/DFSTestUtil.java
          • 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 #176 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Java8/176/ ) HDFS-7847 . Modify NNThroughputBenchmark to be able to operate on a remote NameNode (Charles Lamb via Colin P. McCabe) (cmccabe: rev ffce9a3413277a69444fcb890460c885de56db69) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/DFSTestUtil.java 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 #2117 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/2117/)
          HDFS-7847. Modify NNThroughputBenchmark to be able to operate on a remote NameNode (Charles Lamb via Colin P. McCabe) (cmccabe: rev ffce9a3413277a69444fcb890460c885de56db69)

          • hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/DFSTestUtil.java
          • 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-Hdfs-trunk #2117 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/2117/ ) HDFS-7847 . Modify NNThroughputBenchmark to be able to operate on a remote NameNode (Charles Lamb via Colin P. McCabe) (cmccabe: rev ffce9a3413277a69444fcb890460c885de56db69) hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/DFSTestUtil.java 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 #919 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/919/)
          HDFS-7847. Modify NNThroughputBenchmark to be able to operate on a remote NameNode (Charles Lamb via Colin P. McCabe) (cmccabe: rev ffce9a3413277a69444fcb890460c885de56db69)

          • hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/DFSTestUtil.java
          • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/NNThroughputBenchmark.java
          Show
          hudson Hudson added a comment - SUCCESS: Integrated in Hadoop-Yarn-trunk #919 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/919/ ) HDFS-7847 . Modify NNThroughputBenchmark to be able to operate on a remote NameNode (Charles Lamb via Colin P. McCabe) (cmccabe: rev ffce9a3413277a69444fcb890460c885de56db69) hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/DFSTestUtil.java 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-Yarn-trunk-Java8 #186 (See https://builds.apache.org/job/Hadoop-Yarn-trunk-Java8/186/)
          HDFS-7847. Modify NNThroughputBenchmark to be able to operate on a remote NameNode (Charles Lamb via Colin P. McCabe) (cmccabe: rev ffce9a3413277a69444fcb890460c885de56db69)

          • hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/DFSTestUtil.java
          • 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 #186 (See https://builds.apache.org/job/Hadoop-Yarn-trunk-Java8/186/ ) HDFS-7847 . Modify NNThroughputBenchmark to be able to operate on a remote NameNode (Charles Lamb via Colin P. McCabe) (cmccabe: rev ffce9a3413277a69444fcb890460c885de56db69) hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/DFSTestUtil.java 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-trunk-Commit #7739 (See https://builds.apache.org/job/Hadoop-trunk-Commit/7739/)
          HDFS-7847. Modify NNThroughputBenchmark to be able to operate on a remote NameNode (Charles Lamb via Colin P. McCabe) (cmccabe: rev ffce9a3413277a69444fcb890460c885de56db69)

          • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/DFSTestUtil.java
          • 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-trunk-Commit #7739 (See https://builds.apache.org/job/Hadoop-trunk-Commit/7739/ ) HDFS-7847 . Modify NNThroughputBenchmark to be able to operate on a remote NameNode (Charles Lamb via Colin P. McCabe) (cmccabe: rev ffce9a3413277a69444fcb890460c885de56db69) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/DFSTestUtil.java hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/NNThroughputBenchmark.java hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          Hide
          cmccabe Colin P. McCabe added a comment -

          +1. Thanks, Charles Lamb.

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



          -1 overall



          Vote Subsystem Runtime Comment
          0 pre-patch 5m 13s 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 2 new or modified test files.
          +1 javac 7m 29s There were no new javac warning messages.
          +1 release audit 0m 20s The applied patch does not increase the total number of release audit warnings.
          +1 checkstyle 2m 15s There were no new checkstyle issues.
          +1 whitespace 0m 0s The patch has no lines that end in whitespace.
          +1 install 1m 31s mvn install still works.
          +1 eclipse:eclipse 0m 32s The patch built with eclipse:eclipse.
          +1 findbugs 3m 0s The patch does not introduce any new Findbugs (version 2.0.3) warnings.
          +1 native 1m 20s Pre-build of native portion
          -1 hdfs tests 166m 19s Tests failed in hadoop-hdfs.
              188m 5s  



          Reason Tests
          Failed unit tests hadoop.hdfs.TestAppendSnapshotTruncate
            hadoop.hdfs.server.namenode.ha.TestRetryCacheWithHA



          Subsystem Report/Notes
          Patch URL http://issues.apache.org/jira/secure/attachment/12730467/HDFS-7847.005.patch
          Optional Tests javac unit findbugs checkstyle
          git revision trunk / 318081c
          hadoop-hdfs test log https://builds.apache.org/job/PreCommit-HDFS-Build/10809/artifact/patchprocess/testrun_hadoop-hdfs.txt
          Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/10809/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/10809/console

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 pre-patch 5m 13s 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 2 new or modified test files. +1 javac 7m 29s There were no new javac warning messages. +1 release audit 0m 20s The applied patch does not increase the total number of release audit warnings. +1 checkstyle 2m 15s There were no new checkstyle issues. +1 whitespace 0m 0s The patch has no lines that end in whitespace. +1 install 1m 31s mvn install still works. +1 eclipse:eclipse 0m 32s The patch built with eclipse:eclipse. +1 findbugs 3m 0s The patch does not introduce any new Findbugs (version 2.0.3) warnings. +1 native 1m 20s Pre-build of native portion -1 hdfs tests 166m 19s Tests failed in hadoop-hdfs.     188m 5s   Reason Tests Failed unit tests hadoop.hdfs.TestAppendSnapshotTruncate   hadoop.hdfs.server.namenode.ha.TestRetryCacheWithHA Subsystem Report/Notes Patch URL http://issues.apache.org/jira/secure/attachment/12730467/HDFS-7847.005.patch Optional Tests javac unit findbugs checkstyle git revision trunk / 318081c hadoop-hdfs test log https://builds.apache.org/job/PreCommit-HDFS-Build/10809/artifact/patchprocess/testrun_hadoop-hdfs.txt Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/10809/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/10809/console This message was automatically generated.
          Hide
          clamb Charles Lamb added a comment -

          Colin P. McCabe, I've rebased the patch.

          Show
          clamb Charles Lamb added a comment - Colin P. McCabe , I've rebased the patch.
          Hide
          cmccabe Colin P. McCabe added a comment -

          Charles Lamb, can you rebase this on trunk? Looks like it's gotten stale

          Show
          cmccabe Colin P. McCabe added a comment - Charles Lamb , can you rebase this on trunk? Looks like it's gotten stale
          Hide
          hadoopqa Hadoop QA added a comment -



          -1 overall



          Vote Subsystem Runtime Comment
          -1 patch 0m 0s The patch command could not apply the patch during dryrun.



          Subsystem Report/Notes
          Patch URL http://issues.apache.org/jira/secure/attachment/12728988/HDFS-7847.004.patch
          Optional Tests javac unit findbugs checkstyle
          git revision trunk / 0d6aa5d
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/10795/console

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment -1 patch 0m 0s The patch command could not apply the patch during dryrun. Subsystem Report/Notes Patch URL http://issues.apache.org/jira/secure/attachment/12728988/HDFS-7847.004.patch Optional Tests javac unit findbugs checkstyle git revision trunk / 0d6aa5d Console output https://builds.apache.org/job/PreCommit-HDFS-Build/10795/console This message was automatically generated.
          Hide
          hadoopqa Hadoop QA added a comment -



          -1 overall



          Vote Subsystem Runtime Comment
          -1 patch 0m 0s The patch command could not apply the patch during dryrun.



          Subsystem Report/Notes
          Patch URL http://issues.apache.org/jira/secure/attachment/12728988/HDFS-7847.004.patch
          Optional Tests javac unit findbugs checkstyle
          git revision trunk / bf70c5a
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/10794/console

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment -1 patch 0m 0s The patch command could not apply the patch during dryrun. Subsystem Report/Notes Patch URL http://issues.apache.org/jira/secure/attachment/12728988/HDFS-7847.004.patch Optional Tests javac unit findbugs checkstyle git revision trunk / bf70c5a Console output https://builds.apache.org/job/PreCommit-HDFS-Build/10794/console This message was automatically generated.
          Hide
          clamb Charles Lamb added a comment -

          Porting to trunk. .004 submitted.

          Show
          clamb Charles Lamb added a comment - Porting to trunk. .004 submitted.
          Hide
          clamb Charles Lamb added a comment -

          .004 is rebased onto trunk.

          Show
          clamb Charles Lamb added a comment - .004 is rebased onto trunk.
          Hide
          clamb Charles Lamb added a comment -

          Committed to HDFS-7836 branch.

          Show
          clamb Charles Lamb added a comment - Committed to HDFS-7836 branch.
          Hide
          clamb Charles Lamb added a comment -

          Thanks for the review Colin P. McCabe. I moved those two methods over to DFSTestUtils.java in .003.

          Show
          clamb Charles Lamb added a comment - Thanks for the review Colin P. McCabe . I moved those two methods over to DFSTestUtils.java in .003.
          Hide
          cmccabe Colin P. McCabe added a comment -

          getNamenodeProtocolProxy, getRefreshUserMappingsProtocolProxy: these don't belong in DFSClient. They've got nothing to do with the client, only with HDFS unit tests. Put this stuff someplace like DFSTestUtil.java

          +1 once that's fixed. Thanks, Charles Lamb.

          Show
          cmccabe Colin P. McCabe added a comment - getNamenodeProtocolProxy , getRefreshUserMappingsProtocolProxy : these don't belong in DFSClient. They've got nothing to do with the client, only with HDFS unit tests. Put this stuff someplace like DFSTestUtil.java +1 once that's fixed. Thanks, Charles Lamb .
          Hide
          clamb Charles Lamb added a comment -

          @cmccabe, @stack, thanks for the review!

          DFSClient.java: this change adds three new fields to DFSClient. But they only seem to be used by unit tests. It seems like we should just put these inside the unit test(s) that are using these-- if necessary, by adding a helper method. There's no reason to add more fields to DFSClient. Also remember that when using FileContext, we create new DFSClients all the time.

          Good point. I've left the existing

          ClientProtocol namenode

          field alone. The other 3 proxies are created on-demand by their getters. That means no change in DFSClient instance size.

          It seems kind of odd to have NameNodeProxies#createProxy create a proxy to the datanode.

          It's actually a proxy to the NN for the DatanodeProtocol. That's the same protocol that the DN uses to speak with the NN when it's sending (among other things) block reports. But with some ideas from @stack, I got rid of the changes to NameNodeProxies.

          Of course the NameNode may or may not be remote here. It seems like --nnuri or just --namenode or something like that would be more descriptive.

          Yeah, I agree. I changed it to -namenode.

          Instead of this boilerplate, just use StringUtils#popOptionWithArgument.

          Changed. I was just trying to match the existing code, but the using StringUtils is for the better.

          -          replication, BLOCK_SIZE, null);
          +          replication, BLOCK_SIZE, CryptoProtocolVersion.supported());
          

          This fix is a little bit separate, right? I suppose we can do it in this JIRA, though.

          Without this, the relevant PBHelper.convert code throws NPE on the supportVersions arg.

          Show
          clamb Charles Lamb added a comment - @cmccabe, @stack, thanks for the review! DFSClient.java: this change adds three new fields to DFSClient. But they only seem to be used by unit tests. It seems like we should just put these inside the unit test(s) that are using these-- if necessary, by adding a helper method. There's no reason to add more fields to DFSClient. Also remember that when using FileContext, we create new DFSClients all the time. Good point. I've left the existing ClientProtocol namenode field alone. The other 3 proxies are created on-demand by their getters. That means no change in DFSClient instance size. It seems kind of odd to have NameNodeProxies#createProxy create a proxy to the datanode. It's actually a proxy to the NN for the DatanodeProtocol. That's the same protocol that the DN uses to speak with the NN when it's sending (among other things) block reports. But with some ideas from @stack, I got rid of the changes to NameNodeProxies. Of course the NameNode may or may not be remote here. It seems like --nnuri or just --namenode or something like that would be more descriptive. Yeah, I agree. I changed it to -namenode. Instead of this boilerplate, just use StringUtils#popOptionWithArgument. Changed. I was just trying to match the existing code, but the using StringUtils is for the better. - replication, BLOCK_SIZE, null ); + replication, BLOCK_SIZE, CryptoProtocolVersion.supported()); This fix is a little bit separate, right? I suppose we can do it in this JIRA, though. Without this, the relevant PBHelper.convert code throws NPE on the supportVersions arg.
          Hide
          clamb Charles Lamb added a comment -

          @cmccabe, @stack, thanks for the review!

          DFSClient.java: this change adds three new fields to DFSClient. But they only seem to be used by unit tests. It seems like we should just put these inside the unit test(s) that are using these-- if necessary, by adding a helper method. There's no reason to add more fields to DFSClient. Also remember that when using FileContext, we create new DFSClients all the time.

          Good point. I've left the existing

          ClientProtocol namenode

          field alone. The other 3 proxies are created on-demand by their getters. That means no change in DFSClient instance size.

          It seems kind of odd to have NameNodeProxies#createProxy create a proxy to the datanode.

          It's actually a proxy to the NN for the DatanodeProtocol. That's the same protocol that the DN uses to speak with the NN when it's sending (among other things) block reports.

          In general, when you see "NameNodeProxies" I think "proxies used by the NameNode" and this doesn't fit with that.

          These are actually proxies used to talk to the NN, not proxies used by the NN. I didn't make the name.

          Can you give a little more context about why this is a good idea (as opposed to just having some custom code in the unit test or in a unit test util class that creates a proxy)

          While the name DatanodeProtocol makes us think of an RPC protocol to the datanode, it is in fact yet another one of the many protocols to the namenode which is embodied in the NamenodeProtocols (plural) omnibus interface. The problem this is addressing is that when we are talking to an in-process NN in the NNThroughputBenchmark, then it's easy to get our hands on a NamenodeProtocols instance – you simply call NameNode.getRpcServer(). However, the idea of this patch is to let you run the benchmark against a non-in-process NN, so there's no NameNode instance to use. That means we have to create RPC proxy objects for each of the NN protocols that we need to use.

          It would be nice if we could create a single proxy for the omnibus NamenodeProtocols interface, but we can't. Instead, we have to pick and choose the different namenode protocols that we want to use – ClientProtocol, NamenodeProtocol, RefreshUserMappingProtocol, and DatanodeProtocol – and create proxies for them. Code to create proxies for the first three of these already existed in NameNodeProxies.java, but we have to add a few new lines to create the DatanodeProtocol proxy.

          @stack I looked into your (offline) suggestion to try calling through the TinyDatanode, but it's just doing the same thing that my patch does – it uses the same ClientProtocol instance that the rest of the test uses. TinyDataNode is really just a skeleton and doesn't really borrow much code from the real DN.

          Of course the NameNode may or may not be remote here. It seems like --nnuri or just --namenode or something like that would be more descriptive.

          Yeah, I agree. I changed it to -namenode.

          Instead of this boilerplate, just use StringUtils#popOptionWithArgument.

          Changed. I was just trying to match the existing code, but the using StringUtils is for the better.

          -          replication, BLOCK_SIZE, null);
          +          replication, BLOCK_SIZE, CryptoProtocolVersion.supported());
          

          This fix is a little bit separate, right? I suppose we can do it in this JIRA, though.

          Without this, the relevant PBHelper.convert code throws NPE on the supportVersions arg.

          Show
          clamb Charles Lamb added a comment - @cmccabe, @stack, thanks for the review! DFSClient.java: this change adds three new fields to DFSClient. But they only seem to be used by unit tests. It seems like we should just put these inside the unit test(s) that are using these-- if necessary, by adding a helper method. There's no reason to add more fields to DFSClient. Also remember that when using FileContext, we create new DFSClients all the time. Good point. I've left the existing ClientProtocol namenode field alone. The other 3 proxies are created on-demand by their getters. That means no change in DFSClient instance size. It seems kind of odd to have NameNodeProxies#createProxy create a proxy to the datanode. It's actually a proxy to the NN for the DatanodeProtocol. That's the same protocol that the DN uses to speak with the NN when it's sending (among other things) block reports. In general, when you see "NameNodeProxies" I think "proxies used by the NameNode" and this doesn't fit with that. These are actually proxies used to talk to the NN, not proxies used by the NN. I didn't make the name. Can you give a little more context about why this is a good idea (as opposed to just having some custom code in the unit test or in a unit test util class that creates a proxy) While the name DatanodeProtocol makes us think of an RPC protocol to the datanode, it is in fact yet another one of the many protocols to the namenode which is embodied in the NamenodeProtocols (plural) omnibus interface. The problem this is addressing is that when we are talking to an in-process NN in the NNThroughputBenchmark, then it's easy to get our hands on a NamenodeProtocols instance – you simply call NameNode.getRpcServer(). However, the idea of this patch is to let you run the benchmark against a non-in-process NN, so there's no NameNode instance to use. That means we have to create RPC proxy objects for each of the NN protocols that we need to use. It would be nice if we could create a single proxy for the omnibus NamenodeProtocols interface, but we can't. Instead, we have to pick and choose the different namenode protocols that we want to use – ClientProtocol, NamenodeProtocol, RefreshUserMappingProtocol, and DatanodeProtocol – and create proxies for them. Code to create proxies for the first three of these already existed in NameNodeProxies.java, but we have to add a few new lines to create the DatanodeProtocol proxy. @stack I looked into your (offline) suggestion to try calling through the TinyDatanode, but it's just doing the same thing that my patch does – it uses the same ClientProtocol instance that the rest of the test uses. TinyDataNode is really just a skeleton and doesn't really borrow much code from the real DN. Of course the NameNode may or may not be remote here. It seems like --nnuri or just --namenode or something like that would be more descriptive. Yeah, I agree. I changed it to -namenode. Instead of this boilerplate, just use StringUtils#popOptionWithArgument. Changed. I was just trying to match the existing code, but the using StringUtils is for the better. - replication, BLOCK_SIZE, null ); + replication, BLOCK_SIZE, CryptoProtocolVersion.supported()); This fix is a little bit separate, right? I suppose we can do it in this JIRA, though. Without this, the relevant PBHelper.convert code throws NPE on the supportVersions arg.
          Hide
          stack stack added a comment -

          (Offlist) Charles Lamb set me right, that this patch actually allows you go bang on remote NN... so scratch my above remark (I don't seem to be able to edit it).

          Show
          stack stack added a comment - (Offlist) Charles Lamb set me right, that this patch actually allows you go bang on remote NN... so scratch my above remark (I don't seem to be able to edit it).
          Hide
          stack stack added a comment -

          Mighty Charles Lamb I forgot about that proxy stuff. It was never pretty....

          So idea is that you add proxies for protocols' currently hardwired? That sounds needed if we want to fake out DFSClient that there are thousands of DNs with millions of blocks each. How comes I don't see your added code replacing current hard-wiring of DatanodeProtocol, etc?

          On NNThroughputBenchmark, I think you should just steal the good bits and leave the rest behind you. I say this because I don't think you will be able to put up a load on a NN in a unit test that emulates thousands of nodes making mega-reports about millions upon millions of blocks while simulating thousands of clients. I'd think you'll want to stand up a NN and then batter it for hours on end from many processes distributed over a cluster. Just a suggestion.

          Show
          stack stack added a comment - Mighty Charles Lamb I forgot about that proxy stuff. It was never pretty.... So idea is that you add proxies for protocols' currently hardwired? That sounds needed if we want to fake out DFSClient that there are thousands of DNs with millions of blocks each. How comes I don't see your added code replacing current hard-wiring of DatanodeProtocol, etc? On NNThroughputBenchmark, I think you should just steal the good bits and leave the rest behind you. I say this because I don't think you will be able to put up a load on a NN in a unit test that emulates thousands of nodes making mega-reports about millions upon millions of blocks while simulating thousands of clients. I'd think you'll want to stand up a NN and then batter it for hours on end from many processes distributed over a cluster. Just a suggestion.
          Hide
          cmccabe Colin P. McCabe added a comment -

          DFSClient.java: this change adds three new fields to DFSClient. But they only seem to be used by unit tests. It seems like we should just put these inside the unit test(s) that are using these-- if necessary, by adding a helper method. There's no reason to add more fields to DFSClient. Also remember that when using FileContext, we create new DFSClients all the time.

          this.datanodeProtocolProxy = NameNodeProxies.createProxy...

          It seems kind of odd to have NameNodeProxies#createProxy create a proxy to the datanode. In general, when you see "NameNodeProxies" I think "proxies used by the NameNode" and this doesn't fit with that. Can you give a little more context about why this is a good idea (as opposed to just having some custom code in the unit test or in a unit test util class that creates a proxy)

          121	    " [-remoteNamenode <namenode URI>]\n" +
          122	    "     If using -remoteNamenode, set the namenode's" +
          123	    "         dfs.namenode.fs-limits.min-block-size to 16.";
          

          Of course the NameNode may or may not be remote here. It seems like --nnuri or just --namenode or something like that would be more descriptive.

          378	      final int remoteNNIndex = args.indexOf("-remoteNamenode");
          379	      if (remoteNNIndex >= 0) {
          380	        if (args.size() <= remoteNNIndex + 1) {
          381	          printUsage();
          382	        }
          383	        remoteNamenode = args.get(remoteNNIndex + 1);
          384	        args.remove(remoteNNIndex + 1);
          385	        args.remove(remoteNNIndex);
          386	      }
          

          Instead of this boilerplate, just use StringUtils#popOptionWithArgument

          -          replication, BLOCK_SIZE, null);
          +          replication, BLOCK_SIZE, CryptoProtocolVersion.supported());
          

          This fix is a little bit separate, right? I suppose we can do it in this JIRA, though.

          Show
          cmccabe Colin P. McCabe added a comment - DFSClient.java: this change adds three new fields to DFSClient. But they only seem to be used by unit tests. It seems like we should just put these inside the unit test(s) that are using these-- if necessary, by adding a helper method. There's no reason to add more fields to DFSClient. Also remember that when using FileContext, we create new DFSClients all the time. this.datanodeProtocolProxy = NameNodeProxies.createProxy... It seems kind of odd to have NameNodeProxies#createProxy create a proxy to the datanode. In general, when you see "NameNodeProxies" I think "proxies used by the NameNode" and this doesn't fit with that. Can you give a little more context about why this is a good idea (as opposed to just having some custom code in the unit test or in a unit test util class that creates a proxy) 121 " [-remoteNamenode <namenode URI>]\n" + 122 " If using -remoteNamenode, set the namenode's" + 123 " dfs.namenode.fs-limits.min-block-size to 16." ; Of course the NameNode may or may not be remote here. It seems like --nnuri or just --namenode or something like that would be more descriptive. 378 final int remoteNNIndex = args.indexOf( "-remoteNamenode" ); 379 if (remoteNNIndex >= 0) { 380 if (args.size() <= remoteNNIndex + 1) { 381 printUsage(); 382 } 383 remoteNamenode = args.get(remoteNNIndex + 1); 384 args.remove(remoteNNIndex + 1); 385 args.remove(remoteNNIndex); 386 } Instead of this boilerplate, just use StringUtils#popOptionWithArgument - replication, BLOCK_SIZE, null ); + replication, BLOCK_SIZE, CryptoProtocolVersion.supported()); This fix is a little bit separate, right? I suppose we can do it in this JIRA, though.
          Hide
          clamb Charles Lamb added a comment -

          Add a new -remoteNamenode option to the CLI which takes a URI of a remote NN.

          The existing NNThroughputBenchmark uses the umbrella NamenodeProtocols (plural) interface, but you can only create proxies for the underlying RPC interfaces. This separates all the calls made in NNThroughputBenchmark out into the smaller sub-interfaces.

          Modify DFSClient so that proxies for each of the required interfaces can be created.

          Minor typo fixes encountered along the way.

          Show
          clamb Charles Lamb added a comment - Add a new -remoteNamenode option to the CLI which takes a URI of a remote NN. The existing NNThroughputBenchmark uses the umbrella NamenodeProtocols (plural) interface, but you can only create proxies for the underlying RPC interfaces. This separates all the calls made in NNThroughputBenchmark out into the smaller sub-interfaces. Modify DFSClient so that proxies for each of the required interfaces can be created. Minor typo fixes encountered along the way.
          Hide
          clamb Charles Lamb added a comment - - edited

          Thanks Arpit Agarwal. I'll take a look at them.

          Show
          clamb Charles Lamb added a comment - - edited Thanks Arpit Agarwal . I'll take a look at them.
          Hide
          arpitagarwal Arpit Agarwal added a comment - - edited

          Hi Charles Lamb, I have attached a couple of rudimentary scripts I used to create DN blocks. The scripts use template block+meta files to create as many blocks as you need. It needs updating post HDFS-6482 to put blocks into the correct subdirectories based on their block ID but the fix should be simple.

          On the NN side you can use CreateEditsLog.java which is in our source tree to generate files with the generated block IDs. The edits log can be copied over to a freshly formatted namenode.

          HTH.

          Show
          arpitagarwal Arpit Agarwal added a comment - - edited Hi Charles Lamb , I have attached a couple of rudimentary scripts I used to create DN blocks. The scripts use template block+meta files to create as many blocks as you need. It needs updating post HDFS-6482 to put blocks into the correct subdirectories based on their block ID but the fix should be simple. On the NN side you can use CreateEditsLog.java which is in our source tree to generate files with the generated block IDs. The edits log can be copied over to a freshly formatted namenode. HTH.

            People

            • Assignee:
              clamb Charles Lamb
              Reporter:
              cmccabe Colin P. McCabe
            • Votes:
              0 Vote for this issue
              Watchers:
              15 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development