Uploaded image for project: 'Hadoop HDFS'
  1. Hadoop HDFS
  2. HDFS-10576 DiskBalancer followup work items
  3. HDFS-10553

DiskBalancer: Rename Tools/DiskBalancer class to Tools/DiskBalancerCLI

    Details

    • Type: Sub-task
    • Status: Resolved
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 3.0.0-alpha2
    • Fix Version/s: 3.0.0-alpha2
    • Component/s: balancer & mover
    • Labels:
      None

      Description

      Rename the Tools/DiskBalancer, since we have server/DiskBalancer class. This is confusing when reading code.

      1. HDFS-10553.001.patch
        56 kB
        Manoj Govindassamy
      2. HDFS-10553.002.patch
        56 kB
        Manoj Govindassamy

        Activity

        Hide
        hudson Hudson added a comment -

        SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #10416 (See https://builds.apache.org/job/Hadoop-trunk-Commit/10416/)
        HDFS-10553. DiskBalancer: Rename Tools/DiskBalancer class to (aengineer: rev 35c5943b8ba394191405555cdfc5e6127053ee97)

        • (edit) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/diskbalancer/command/HelpCommand.java
        • (edit) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/diskbalancer/command/Command.java
        • (edit) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/diskbalancer/command/ReportCommand.java
        • (delete) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/tools/DiskBalancer.java
        • (add) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/tools/DiskBalancerCLI.java
        • (edit) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/diskbalancer/command/ExecuteCommand.java
        • (edit) hadoop-hdfs-project/hadoop-hdfs/src/main/bin/hdfs
        • (edit) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/diskbalancer/command/CancelCommand.java
        • (edit) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/diskbalancer/command/PlanCommand.java
        • (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/diskbalancer/command/TestDiskBalancerCommand.java
        • (edit) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/diskbalancer/command/QueryCommand.java
        Show
        hudson Hudson added a comment - SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #10416 (See https://builds.apache.org/job/Hadoop-trunk-Commit/10416/ ) HDFS-10553 . DiskBalancer: Rename Tools/DiskBalancer class to (aengineer: rev 35c5943b8ba394191405555cdfc5e6127053ee97) (edit) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/diskbalancer/command/HelpCommand.java (edit) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/diskbalancer/command/Command.java (edit) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/diskbalancer/command/ReportCommand.java (delete) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/tools/DiskBalancer.java (add) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/tools/DiskBalancerCLI.java (edit) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/diskbalancer/command/ExecuteCommand.java (edit) hadoop-hdfs-project/hadoop-hdfs/src/main/bin/hdfs (edit) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/diskbalancer/command/CancelCommand.java (edit) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/diskbalancer/command/PlanCommand.java (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/diskbalancer/command/TestDiskBalancerCommand.java (edit) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/diskbalancer/command/QueryCommand.java
        Hide
        anu Anu Engineer added a comment -

        Manoj Govindassamy Thank you for the contribution. I have committed this to trunk.

        Show
        anu Anu Engineer added a comment - Manoj Govindassamy Thank you for the contribution. I have committed this to trunk.
        Hide
        anu Anu Engineer added a comment -

        +1, LGTM. Manoj Govindassamy Thanks for updating the patch, I will commit this shortly.

        Show
        anu Anu Engineer added a comment - +1, LGTM. Manoj Govindassamy Thanks for updating the patch, I will commit this shortly.
        Hide
        hadoopqa Hadoop QA added a comment -
        +1 overall



        Vote Subsystem Runtime Comment
        0 reexec 0m 18s Docker mode activated.
        0 shelldocs 0m 4s Shelldocs was not available.
        +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 8m 10s trunk passed
        +1 compile 0m 54s trunk passed
        +1 checkstyle 0m 30s trunk passed
        +1 mvnsite 0m 58s trunk passed
        +1 mvneclipse 0m 15s trunk passed
        +1 findbugs 1m 54s trunk passed
        +1 javadoc 0m 59s trunk passed
        +1 mvninstall 0m 53s the patch passed
        +1 compile 0m 51s the patch passed
        +1 javac 0m 51s the patch passed
        +1 checkstyle 0m 26s the patch passed
        +1 mvnsite 0m 56s the patch passed
        +1 mvneclipse 0m 12s the patch passed
        +1 shellcheck 0m 13s The patch generated 0 new + 74 unchanged - 1 fixed = 74 total (was 75)
        +1 whitespace 0m 0s The patch has no whitespace issues.
        +1 findbugs 1m 58s the patch passed
        +1 javadoc 0m 57s the patch passed
        +1 unit 62m 51s hadoop-hdfs in the patch passed.
        +1 asflicense 0m 18s The patch does not generate ASF License warnings.
        85m 1s



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:9560f25
        JIRA Issue HDFS-10553
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12827680/HDFS-10553.002.patch
        Optional Tests asflicense mvnsite unit shellcheck shelldocs compile javac javadoc mvninstall findbugs checkstyle
        uname Linux 7c2add7f68e8 3.13.0-92-generic #139-Ubuntu SMP Tue Jun 28 20:42:26 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
        Build tool maven
        Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
        git revision trunk / b6d839a
        Default Java 1.8.0_101
        shellcheck v0.4.4
        findbugs v3.0.0
        Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/16683/testReport/
        modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs
        Console output https://builds.apache.org/job/PreCommit-HDFS-Build/16683/console
        Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - +1 overall Vote Subsystem Runtime Comment 0 reexec 0m 18s Docker mode activated. 0 shelldocs 0m 4s Shelldocs was not available. +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 8m 10s trunk passed +1 compile 0m 54s trunk passed +1 checkstyle 0m 30s trunk passed +1 mvnsite 0m 58s trunk passed +1 mvneclipse 0m 15s trunk passed +1 findbugs 1m 54s trunk passed +1 javadoc 0m 59s trunk passed +1 mvninstall 0m 53s the patch passed +1 compile 0m 51s the patch passed +1 javac 0m 51s the patch passed +1 checkstyle 0m 26s the patch passed +1 mvnsite 0m 56s the patch passed +1 mvneclipse 0m 12s the patch passed +1 shellcheck 0m 13s The patch generated 0 new + 74 unchanged - 1 fixed = 74 total (was 75) +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 58s the patch passed +1 javadoc 0m 57s the patch passed +1 unit 62m 51s hadoop-hdfs in the patch passed. +1 asflicense 0m 18s The patch does not generate ASF License warnings. 85m 1s Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Issue HDFS-10553 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12827680/HDFS-10553.002.patch Optional Tests asflicense mvnsite unit shellcheck shelldocs compile javac javadoc mvninstall findbugs checkstyle uname Linux 7c2add7f68e8 3.13.0-92-generic #139-Ubuntu SMP Tue Jun 28 20:42:26 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / b6d839a Default Java 1.8.0_101 shellcheck v0.4.4 findbugs v3.0.0 Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/16683/testReport/ modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs Console output https://builds.apache.org/job/PreCommit-HDFS-Build/16683/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
        Hide
        manojg Manoj Govindassamy added a comment -

        Attached v002 patch.

        – updated bin/hdfs script to contain the right class name DiskBalancerCLI
        – verified hdfs diskbalancer command launches the refactored class and help message shows up

        # echo $HADOOP_CONF_DIR 
        HADOOP_CONF_DIR=/Users/manoj/work/ups-hadoop/hadoop-dist//target/hadoop-3.0.0-alpha2-SNAPSHOT/etc/hadoop
        
        # hdfs diskbalancer
        usage: hdfs diskbalancer [command] [options]
        
        DiskBalancer distributes data evenly between different disks on a
        datanode. DiskBalancer operates by generating a plan, that tells datanode
        how to move data between disks. Users can execute a plan by submitting it
        to the datanode.
        To get specific help on a particular command please run
        hdfs diskbalancer -help <command>.
            --help <arg>   valid commands are plan | execute | query | cancel |
                           report
        

        Thanks for pointing out the script issue Anu Engineer.

        Show
        manojg Manoj Govindassamy added a comment - Attached v002 patch. – updated bin/hdfs script to contain the right class name DiskBalancerCLI – verified hdfs diskbalancer command launches the refactored class and help message shows up # echo $HADOOP_CONF_DIR HADOOP_CONF_DIR=/Users/manoj/work/ups-hadoop/hadoop-dist//target/hadoop-3.0.0-alpha2-SNAPSHOT/etc/hadoop # hdfs diskbalancer usage: hdfs diskbalancer [command] [options] DiskBalancer distributes data evenly between different disks on a datanode. DiskBalancer operates by generating a plan, that tells datanode how to move data between disks. Users can execute a plan by submitting it to the datanode. To get specific help on a particular command please run hdfs diskbalancer -help <command>. --help <arg> valid commands are plan | execute | query | cancel | report Thanks for pointing out the script issue Anu Engineer .
        Hide
        hadoopqa Hadoop QA added a comment -
        +1 overall



        Vote Subsystem Runtime Comment
        0 reexec 0m 11s Docker mode activated.
        +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 7m 2s trunk passed
        +1 compile 0m 46s trunk passed
        +1 checkstyle 0m 25s trunk passed
        +1 mvnsite 0m 53s trunk passed
        +1 mvneclipse 0m 12s trunk passed
        +1 findbugs 1m 41s trunk passed
        +1 javadoc 0m 55s trunk passed
        +1 mvninstall 0m 46s the patch passed
        +1 compile 0m 41s the patch passed
        +1 javac 0m 41s the patch passed
        +1 checkstyle 0m 22s the patch passed
        +1 mvnsite 0m 49s the patch passed
        +1 mvneclipse 0m 9s the patch passed
        +1 whitespace 0m 0s The patch has no whitespace issues.
        +1 findbugs 1m 47s the patch passed
        +1 javadoc 0m 51s the patch passed
        +1 unit 58m 41s hadoop-hdfs in the patch passed.
        +1 asflicense 0m 20s The patch does not generate ASF License warnings.
        77m 53s



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:9560f25
        JIRA Issue HDFS-10553
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12827646/HDFS-10553.001.patch
        Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
        uname Linux 0a6a1151cf6e 3.13.0-93-generic #140-Ubuntu SMP Mon Jul 18 21:21:05 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
        Build tool maven
        Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
        git revision trunk / b6d839a
        Default Java 1.8.0_101
        findbugs v3.0.0
        Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/16680/testReport/
        modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs
        Console output https://builds.apache.org/job/PreCommit-HDFS-Build/16680/console
        Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - +1 overall Vote Subsystem Runtime Comment 0 reexec 0m 11s Docker mode activated. +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 7m 2s trunk passed +1 compile 0m 46s trunk passed +1 checkstyle 0m 25s trunk passed +1 mvnsite 0m 53s trunk passed +1 mvneclipse 0m 12s trunk passed +1 findbugs 1m 41s trunk passed +1 javadoc 0m 55s trunk passed +1 mvninstall 0m 46s the patch passed +1 compile 0m 41s the patch passed +1 javac 0m 41s the patch passed +1 checkstyle 0m 22s the patch passed +1 mvnsite 0m 49s the patch passed +1 mvneclipse 0m 9s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 47s the patch passed +1 javadoc 0m 51s the patch passed +1 unit 58m 41s hadoop-hdfs in the patch passed. +1 asflicense 0m 20s The patch does not generate ASF License warnings. 77m 53s Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Issue HDFS-10553 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12827646/HDFS-10553.001.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 0a6a1151cf6e 3.13.0-93-generic #140-Ubuntu SMP Mon Jul 18 21:21:05 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / b6d839a Default Java 1.8.0_101 findbugs v3.0.0 Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/16680/testReport/ modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs Console output https://builds.apache.org/job/PreCommit-HDFS-Build/16680/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
        Hide
        anu Anu Engineer added a comment - - edited

        Manoj Govindassamy The changes look good. There is one more place where we have to change. Diskbalancer is invoked via a script called hdfs that is located at hadoop-hdfs-project/hadoop-hdfs/src/main/bin/hdfs

        it is a bash script and the HADOOP_CLASSNAME variable should change too. We should replace this line

              HADOOP_CLASSNAME=org.apache.hadoop.hdfs.tools.DiskBalancer
        

        with

              HADOOP_CLASSNAME=org.apache.hadoop.hdfs.tools.DiskBalancerCLI
        

        Our current set of tests will not catch this, may be you can address this issue also as part of addressing HDFS-10599. Right now to verify I suggest that you deploy a pseudo node and make sure that you can run hdfs diskbalancer and you are able to see help.

        Show
        anu Anu Engineer added a comment - - edited Manoj Govindassamy The changes look good. There is one more place where we have to change. Diskbalancer is invoked via a script called hdfs that is located at hadoop-hdfs-project/hadoop-hdfs/src/main/bin/hdfs it is a bash script and the HADOOP_CLASSNAME variable should change too. We should replace this line HADOOP_CLASSNAME=org.apache.hadoop.hdfs.tools.DiskBalancer with HADOOP_CLASSNAME=org.apache.hadoop.hdfs.tools.DiskBalancerCLI Our current set of tests will not catch this, may be you can address this issue also as part of addressing HDFS-10599 . Right now to verify I suggest that you deploy a pseudo node and make sure that you can run hdfs diskbalancer and you are able to see help.
        Hide
        manojg Manoj Govindassamy added a comment -

        Thanks Anu Engineer for the clarification. Attaching the new one.

        Show
        manojg Manoj Govindassamy added a comment - Thanks Anu Engineer for the clarification. Attaching the new one.
        Hide
        anu Anu Engineer added a comment -

        Manoj Govindassamy Thanks for the patch. Could you please rename this patch so that this will not fail in jenkins. I think you want to name the patch HDFS-10553.001.patch, instead of HDFS-10553-HDFS-1312.001.patch. The format of patches is JIRA-BRANCH.rev.patch. In this case jenkins will try to apply this patch to HDFS-1312 branch which is already merged to trunk and now defunct. Since you want your patch to be applied to trunk, you don't need to specify a name.

        Show
        anu Anu Engineer added a comment - Manoj Govindassamy Thanks for the patch. Could you please rename this patch so that this will not fail in jenkins. I think you want to name the patch HDFS-10553 .001.patch , instead of HDFS-10553 - HDFS-1312 .001.patch. The format of patches is JIRA-BRANCH.rev.patch . In this case jenkins will try to apply this patch to HDFS-1312 branch which is already merged to trunk and now defunct. Since you want your patch to be applied to trunk, you don't need to specify a name.
        Hide
        manojg Manoj Govindassamy added a comment -

        Attaching v001 patch: org.apache.hadoop.hdfs.tools.DiskBalancer refactored to org.apache.hadoop.hdfs.tools.DiskBalancerCLI.

        Verified unit tests - TestDiskBalancer*

        Lei (Eddy) Xu, Anu Engineer, can you please take a look ?

        Show
        manojg Manoj Govindassamy added a comment - Attaching v001 patch: org.apache.hadoop.hdfs.tools.DiskBalancer refactored to org.apache.hadoop.hdfs.tools.DiskBalancerCLI. Verified unit tests - TestDiskBalancer* Lei (Eddy) Xu , Anu Engineer , can you please take a look ?
        Hide
        manojg Manoj Govindassamy added a comment -

        I see the CLI class suffix used more widely compared to Tool suffix for Tool implementers. Will go with org.apache.hadoop.hdfs.tools.DiskBalancerCLI for now.

        Show
        manojg Manoj Govindassamy added a comment - I see the CLI class suffix used more widely compared to Tool suffix for Tool implementers. Will go with org.apache.hadoop.hdfs.tools.DiskBalancerCLI for now.
        Hide
        anu Anu Engineer added a comment -

        Please go ahead and use any name that you think is good. The intend was to avoid using the same name for the class in the datanode package and in the tools package. As long as that is addressed I don't have a strong preference.

        Show
        anu Anu Engineer added a comment - Please go ahead and use any name that you think is good. The intend was to avoid using the same name for the class in the datanode package and in the tools package. As long as that is addressed I don't have a strong preference.
        Hide
        manojg Manoj Govindassamy added a comment - - edited

        org.apache.hadoop.hdfs.tools.DiskBalancer is more of a Tool/App than of a CLI Shell. Wondering if the refactoring org.apache.hadoop.hdfs.tools.DiskBalancer to org.apache.hadoop.hdfs.tools.DiskBalancerTool / org.apache.hadoop.hdfs.tools.DiskBalancerCommand would be better compared to org.apache.hadoop.hdfs.tools.DiskBalancerCLI. I am ok with either names. Anu Engineer, your thoughts ?

        Show
        manojg Manoj Govindassamy added a comment - - edited org.apache.hadoop.hdfs.tools.DiskBalancer is more of a Tool/App than of a CLI Shell. Wondering if the refactoring org.apache.hadoop.hdfs.tools.DiskBalancer to org.apache.hadoop.hdfs.tools.DiskBalancerTool / org.apache.hadoop.hdfs.tools.DiskBalancerCommand would be better compared to org.apache.hadoop.hdfs.tools.DiskBalancerCLI. I am ok with either names. Anu Engineer , your thoughts ?
        Hide
        manojg Manoj Govindassamy added a comment -

        Based on discussion with Anu Engineer, taking up this task.

        Show
        manojg Manoj Govindassamy added a comment - Based on discussion with Anu Engineer , taking up this task.
        Hide
        anu Anu Engineer added a comment -

        From the mail that points out this issue from Tsz Wo Nicholas Sze

        • There are a few TODOs in the code.
        • Tried the help command "hdfs diskbalancer -help plan". There is a typo "wetolerate" in --thresholdPercentage. Also, we should mention the unit for --bandwidth.
        • We should avoid using the same class name such as DiskBalancer, which is defined in both the datanode and tools packages. It may be better to call it DiskBalancerCli for the one in tools.
        • I still think that it is better to use weighted mean and weighted variance in the calculation.
        Show
        anu Anu Engineer added a comment - From the mail that points out this issue from Tsz Wo Nicholas Sze There are a few TODOs in the code. Tried the help command "hdfs diskbalancer -help plan". There is a typo "wetolerate" in --thresholdPercentage. Also, we should mention the unit for --bandwidth. We should avoid using the same class name such as DiskBalancer, which is defined in both the datanode and tools packages. It may be better to call it DiskBalancerCli for the one in tools. I still think that it is better to use weighted mean and weighted variance in the calculation.

          People

          • Assignee:
            manojg Manoj Govindassamy
            Reporter:
            anu Anu Engineer
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development