Uploaded image for project: 'Hadoop HDFS'
  1. Hadoop HDFS
  2. HDFS-7584 Enable Quota Support for Storage Types
  3. HDFS-7701

Support reporting per storage type quota and usage with hadoop/hdfs shell

    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: datanode, namenode
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      "hadoop fs -count -q" or "hdfs dfs -count -q" currently shows name space/disk space quota and remaining quota information. With HDFS-7584, we want to display per storage type quota and its remaining information as well.

      The current output format as shown below may not easily accomodate 6 more columns = 3 (existing storage types) * 2 (quota/remaining quota). With new storage types added in future, this will make the output even more crowded. There are also compatibility issues as we don't want to break any existing scripts monitoring hadoop fs -count -q output.

      $ hadoop fs -count -q -v /test
      QUOTA REM_QUOTA SPACE_QUOTA REM_SPACE_QUOTA DIR_COUNT FILE_COUNT CONTENT_SIZE PATHNAME
      none inf 524288000 524266569 1 15 21431 /test

      Propose to add -t parameter to display ONLY the storage type quota information of the directory in the separately. This way, existing scripts will work as-is without using -t parameter.

      1) When -t is not followed by a specific storage type, quota and usage information for all storage types will be displayed.
      $ hadoop fs -count -q -t -h -v /test
      SSD_QUOTA REM_SSD_QUOTA DISK_QUOTA REM_DISK_QUOTA ARCHIVAL_QUOTA REM_ARCHIVAL_QUOTA PATHNAME
      512MB 256MB none inf none inf /test

      2) If -t is followed by a storage type, only the quota and remaining quota of the storage type is displayed.
      $ hadoop fs -count -q -t SSD -h -v /test

      SSD_QUOTA REM_SSD_QUOTA PATHNAME
      512 MB 256 MB /test

      1. HDFS-7701.06.patch
        21 kB
        Arpit Agarwal
      2. HDFS-7701.05.patch
        21 kB
        Peter Shi
      3. HDFS-7701.04.patch
        21 kB
        Peter Shi
      4. HDFS-7701.03.patch
        20 kB
        Peter Shi
      5. HDFS-7701.02.patch
        19 kB
        Peter Shi
      6. HDFS-7701.01.patch
        17 kB
        Peter Shi

        Issue Links

          Activity

          Hide
          hudson Hudson added a comment -

          SUCCESS: Integrated in Hadoop-Mapreduce-trunk #2113 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/2113/)
          HDFS-7701. Support reporting per storage type quota and usage with hadoop/hdfs shell. (Contributed by Peter Shi) (arp: rev 18a3dad44afd8061643fffc5bbe50fa66e47b72c)

          • hadoop-common-project/hadoop-common/src/test/resources/testConf.xml
          • hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/ContentSummary.java
          • hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/shell/TestCount.java
          • hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/CommandFormat.java
          • hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          • hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/Count.java
          Show
          hudson Hudson added a comment - SUCCESS: Integrated in Hadoop-Mapreduce-trunk #2113 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/2113/ ) HDFS-7701 . Support reporting per storage type quota and usage with hadoop/hdfs shell. (Contributed by Peter Shi) (arp: rev 18a3dad44afd8061643fffc5bbe50fa66e47b72c) hadoop-common-project/hadoop-common/src/test/resources/testConf.xml hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/ContentSummary.java hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/shell/TestCount.java hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/CommandFormat.java hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/Count.java
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in Hadoop-Mapreduce-trunk-Java8 #164 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Java8/164/)
          HDFS-7701. Support reporting per storage type quota and usage with hadoop/hdfs shell. (Contributed by Peter Shi) (arp: rev 18a3dad44afd8061643fffc5bbe50fa66e47b72c)

          • hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/CommandFormat.java
          • hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          • hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/ContentSummary.java
          • hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/Count.java
          • hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/shell/TestCount.java
          • hadoop-common-project/hadoop-common/src/test/resources/testConf.xml
          Show
          hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Mapreduce-trunk-Java8 #164 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Java8/164/ ) HDFS-7701 . Support reporting per storage type quota and usage with hadoop/hdfs shell. (Contributed by Peter Shi) (arp: rev 18a3dad44afd8061643fffc5bbe50fa66e47b72c) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/CommandFormat.java hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/ContentSummary.java hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/Count.java hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/shell/TestCount.java hadoop-common-project/hadoop-common/src/test/resources/testConf.xml
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in Hadoop-Hdfs-trunk-Java8 #154 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Java8/154/)
          HDFS-7701. Support reporting per storage type quota and usage with hadoop/hdfs shell. (Contributed by Peter Shi) (arp: rev 18a3dad44afd8061643fffc5bbe50fa66e47b72c)

          • hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/ContentSummary.java
          • hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/Count.java
          • hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/CommandFormat.java
          • hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/shell/TestCount.java
          • hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          • hadoop-common-project/hadoop-common/src/test/resources/testConf.xml
          Show
          hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Hdfs-trunk-Java8 #154 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Java8/154/ ) HDFS-7701 . Support reporting per storage type quota and usage with hadoop/hdfs shell. (Contributed by Peter Shi) (arp: rev 18a3dad44afd8061643fffc5bbe50fa66e47b72c) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/ContentSummary.java hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/Count.java hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/CommandFormat.java hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/shell/TestCount.java hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt hadoop-common-project/hadoop-common/src/test/resources/testConf.xml
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in Hadoop-Yarn-trunk #897 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/897/)
          HDFS-7701. Support reporting per storage type quota and usage with hadoop/hdfs shell. (Contributed by Peter Shi) (arp: rev 18a3dad44afd8061643fffc5bbe50fa66e47b72c)

          • hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/shell/TestCount.java
          • hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/Count.java
          • hadoop-common-project/hadoop-common/src/test/resources/testConf.xml
          • hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/CommandFormat.java
          • hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          • hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/ContentSummary.java
          Show
          hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Yarn-trunk #897 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/897/ ) HDFS-7701 . Support reporting per storage type quota and usage with hadoop/hdfs shell. (Contributed by Peter Shi) (arp: rev 18a3dad44afd8061643fffc5bbe50fa66e47b72c) hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/shell/TestCount.java hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/Count.java hadoop-common-project/hadoop-common/src/test/resources/testConf.xml hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/CommandFormat.java hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/ContentSummary.java
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in Hadoop-Hdfs-trunk #2095 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/2095/)
          HDFS-7701. Support reporting per storage type quota and usage with hadoop/hdfs shell. (Contributed by Peter Shi) (arp: rev 18a3dad44afd8061643fffc5bbe50fa66e47b72c)

          • hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/Count.java
          • hadoop-common-project/hadoop-common/src/test/resources/testConf.xml
          • hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/CommandFormat.java
          • hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/shell/TestCount.java
          • hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/ContentSummary.java
          • hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          Show
          hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Hdfs-trunk #2095 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/2095/ ) HDFS-7701 . Support reporting per storage type quota and usage with hadoop/hdfs shell. (Contributed by Peter Shi) (arp: rev 18a3dad44afd8061643fffc5bbe50fa66e47b72c) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/Count.java hadoop-common-project/hadoop-common/src/test/resources/testConf.xml hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/CommandFormat.java hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/shell/TestCount.java hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/ContentSummary.java hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in Hadoop-Yarn-trunk-Java8 #163 (See https://builds.apache.org/job/Hadoop-Yarn-trunk-Java8/163/)
          HDFS-7701. Support reporting per storage type quota and usage with hadoop/hdfs shell. (Contributed by Peter Shi) (arp: rev 18a3dad44afd8061643fffc5bbe50fa66e47b72c)

          • hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/Count.java
          • hadoop-common-project/hadoop-common/src/test/resources/testConf.xml
          • hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/CommandFormat.java
          • hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/ContentSummary.java
          • hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          • hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/shell/TestCount.java
          Show
          hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Yarn-trunk-Java8 #163 (See https://builds.apache.org/job/Hadoop-Yarn-trunk-Java8/163/ ) HDFS-7701 . Support reporting per storage type quota and usage with hadoop/hdfs shell. (Contributed by Peter Shi) (arp: rev 18a3dad44afd8061643fffc5bbe50fa66e47b72c) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/Count.java hadoop-common-project/hadoop-common/src/test/resources/testConf.xml hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/CommandFormat.java hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/ContentSummary.java hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/shell/TestCount.java
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in Hadoop-trunk-Commit #7580 (See https://builds.apache.org/job/Hadoop-trunk-Commit/7580/)
          HDFS-7701. Support reporting per storage type quota and usage with hadoop/hdfs shell. (Contributed by Peter Shi) (arp: rev 18a3dad44afd8061643fffc5bbe50fa66e47b72c)

          • hadoop-common-project/hadoop-common/src/test/resources/testConf.xml
          • hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          • hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/shell/TestCount.java
          • hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/ContentSummary.java
          • hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/CommandFormat.java
          • hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/Count.java
          Show
          hudson Hudson added a comment - FAILURE: Integrated in Hadoop-trunk-Commit #7580 (See https://builds.apache.org/job/Hadoop-trunk-Commit/7580/ ) HDFS-7701 . Support reporting per storage type quota and usage with hadoop/hdfs shell. (Contributed by Peter Shi) (arp: rev 18a3dad44afd8061643fffc5bbe50fa66e47b72c) hadoop-common-project/hadoop-common/src/test/resources/testConf.xml hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/shell/TestCount.java hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/ContentSummary.java hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/CommandFormat.java hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/Count.java
          Hide
          arpitagarwal Arpit Agarwal added a comment - - edited

          I've committed this to trunk and branch-2.

          Thanks for the contribution Peter Shi and thanks Xiaoyu Yao for all the reviews!

          Show
          arpitagarwal Arpit Agarwal added a comment - - edited I've committed this to trunk and branch-2. Thanks for the contribution Peter Shi and thanks Xiaoyu Yao for all the reviews!
          Hide
          arpitagarwal Arpit Agarwal added a comment -

          Thanks for the (binding) +1 Xiaoyu Yao. Will commit on Jenkins +1.

          Show
          arpitagarwal Arpit Agarwal added a comment - Thanks for the (binding) +1 Xiaoyu Yao . Will commit on Jenkins +1.
          Hide
          xyao Xiaoyu Yao added a comment -

          Thanks Arpit Agarwal for the update. +1 for patch v6, which looks pretty good to me.

          Show
          xyao Xiaoyu Yao added a comment - Thanks Arpit Agarwal for the update. +1 for patch v6, which looks pretty good to me.
          Hide
          hadoopqa Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12725131/HDFS-7701.06.patch
          against trunk revision 838b06a.

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 2 new or modified test files.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 javadoc. There were no new javadoc warning messages.

          +1 eclipse:eclipse. The patch built with eclipse:eclipse.

          +1 findbugs. The patch does not introduce any new Findbugs (version 2.0.3) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          +1 core tests. The patch passed unit tests in hadoop-common-project/hadoop-common.

          Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/10271//testReport/
          Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/10271//console

          This message is automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - +1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12725131/HDFS-7701.06.patch against trunk revision 838b06a. +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 2 new or modified test files. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . There were no new javadoc warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any new Findbugs (version 2.0.3) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. +1 core tests . The patch passed unit tests in hadoop-common-project/hadoop-common. Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/10271//testReport/ Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/10271//console This message is automatically generated.
          Hide
          arpitagarwal Arpit Agarwal added a comment -

          Thanks Peter Shi.

          I made a v6 patch with a trivial edit to the command usage. Here is the delta from v5.

          Count.java
          @@ -70,7 +70,7 @@ public static void registerCommands(CommandFactory factory) {
          -          "If storage types(separated by comma) are given after -" +
          +          "If a comma-separated list of storage types is given after the -" +
          
          TestCount.java
          @@ -423,7 +423,7 @@ public void getDescription() {
          -        + "If storage types(separated by comma) are given after -t option, \n"
          +        + "If a comma-separated list of storage types is given after the -t option, \n"
          

          If you guys are okay with the edit I can commit the v6 patch.

          Show
          arpitagarwal Arpit Agarwal added a comment - Thanks Peter Shi . I made a v6 patch with a trivial edit to the command usage. Here is the delta from v5. Count.java @@ -70,7 +70,7 @@ public static void registerCommands(CommandFactory factory) { - "If storage types(separated by comma) are given after -" + + "If a comma-separated list of storage types is given after the -" + TestCount.java @@ -423,7 +423,7 @@ public void getDescription() { - + "If storage types(separated by comma) are given after -t option, \n" + + "If a comma-separated list of storage types is given after the -t option, \n" If you guys are okay with the edit I can commit the v6 patch.
          Hide
          shihaoliang Peter Shi added a comment -

          My modification should not affect this testcases. And my local machine passes these testcases.

          Show
          shihaoliang Peter Shi added a comment - My modification should not affect this testcases. And my local machine passes these testcases.
          Hide
          hadoopqa Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12724861/HDFS-7701.05.patch
          against trunk revision f8f5887.

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 2 new or modified test files.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 javadoc. There were no new javadoc warning messages.

          +1 eclipse:eclipse. The patch built with eclipse:eclipse.

          +1 findbugs. The patch does not introduce any new Findbugs (version 2.0.3) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          -1 core tests. The patch failed these unit tests in hadoop-common-project/hadoop-common:

          org.apache.hadoop.ipc.TestRPCWaitForProxy

          Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/10260//testReport/
          Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/10260//console

          This message is automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12724861/HDFS-7701.05.patch against trunk revision f8f5887. +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 2 new or modified test files. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . There were no new javadoc warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any new Findbugs (version 2.0.3) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. -1 core tests . The patch failed these unit tests in hadoop-common-project/hadoop-common: org.apache.hadoop.ipc.TestRPCWaitForProxy Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/10260//testReport/ Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/10260//console This message is automatically generated.
          Hide
          shihaoliang Peter Shi added a comment -

          fix the hard-code print format string and the description of the command.

          Show
          shihaoliang Peter Shi added a comment - fix the hard-code print format string and the description of the command.
          Hide
          shihaoliang Peter Shi added a comment -

          Thanks Arpit Agarwal for review this. I will attach patch soon.

          Show
          shihaoliang Peter Shi added a comment - Thanks Arpit Agarwal for review this. I will attach patch soon.
          Hide
          arpitagarwal Arpit Agarwal added a comment - - edited

          Sorry for the delay in looking at this. Couple of nitpicks below:

          edit: Okay that check makes sense when I read it again.

          Did you intend to replace the hard-coded format string with STORAGE_TYPE_SUMMARY_FORMAT?

                header.append(String.format("%13s %17s ", storageName + "_QUOTA",
          

          It looks like count accepts a comma separated list of storage types. Update Count#DESCRIPTION accordingly?

          I'll take a look at the test cases.

          Show
          arpitagarwal Arpit Agarwal added a comment - - edited Sorry for the delay in looking at this. Couple of nitpicks below: edit: Okay that check makes sense when I read it again. Did you intend to replace the hard-coded format string with STORAGE_TYPE_SUMMARY_FORMAT ? header.append( String .format( "%13s %17s " , storageName + "_QUOTA" , It looks like count accepts a comma separated list of storage types. Update Count#DESCRIPTION accordingly? I'll take a look at the test cases.
          Hide
          xyao Xiaoyu Yao added a comment -

          Thanks Peter for the update! +1 (non-binding) for patch v4.
          Arpit Agarwal, can you help review the patch?

          Show
          xyao Xiaoyu Yao added a comment - Thanks Peter for the update! +1 (non-binding) for patch v4. Arpit Agarwal , can you help review the patch?
          Hide
          hadoopqa Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12708388/HDFS-7701.04.patch
          against trunk revision b5a22e9.

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 2 new or modified test files.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 javadoc. There were no new javadoc warning messages.

          +1 eclipse:eclipse. The patch built with eclipse:eclipse.

          +1 findbugs. The patch does not introduce any new Findbugs (version 2.0.3) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          +1 core tests. The patch passed unit tests in hadoop-common-project/hadoop-common.

          Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/10128//testReport/
          Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/10128//console

          This message is automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - +1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12708388/HDFS-7701.04.patch against trunk revision b5a22e9. +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 2 new or modified test files. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . There were no new javadoc warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any new Findbugs (version 2.0.3) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. +1 core tests . The patch passed unit tests in hadoop-common-project/hadoop-common. Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/10128//testReport/ Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/10128//console This message is automatically generated.
          Hide
          shihaoliang Peter Shi added a comment -

          Thanks for giving such detailed suggestion, i will upload the fixed patth ASAP.

          Show
          shihaoliang Peter Shi added a comment - Thanks for giving such detailed suggestion, i will upload the fixed patth ASAP.
          Hide
          xyao Xiaoyu Yao added a comment -

          Patch v3 looks pretty good to me. Thanks Peter for addressing the comments and unit test failures. I Just have a couple of Nits below.

          ContentSummary.java
          line 292: Can you rename getStorageHeader() to getStorageTypeHeader()?
          Line 296: Can you remove the comment on RAM_DISK_QUOTA as it is the type that does not support quota by storage type. Also the 14/18 format for quota/remaining quota print might also need to adjust?

          Line 353: Can you add JavaDoc for the new ContentSummary#toString() method?

          Line 362: Can you consolidate the string literals as static const of the class for "none" and "inf"?

          Line 400: Can you remove the extra empty line?

          CommandFormat.java
          There is an extra empty line in line 244.
          Can you reword the comment on line 247? "Used when a duplicated option is supplied to a command.

          Count.java
          Can you remove the extra empty line in line: 120 and 145?
          Can you break the line at line: 150 so that each line is within 80.

          TestCount.java
          Line:30 There is an extra empty line
          Can you remove the unused import "Import org.apache.hadoop.util.StringUtils;"

          Show
          xyao Xiaoyu Yao added a comment - Patch v3 looks pretty good to me. Thanks Peter for addressing the comments and unit test failures. I Just have a couple of Nits below. ContentSummary.java line 292: Can you rename getStorageHeader() to getStorageTypeHeader()? Line 296: Can you remove the comment on RAM_DISK_QUOTA as it is the type that does not support quota by storage type. Also the 14/18 format for quota/remaining quota print might also need to adjust? Line 353: Can you add JavaDoc for the new ContentSummary#toString() method? Line 362: Can you consolidate the string literals as static const of the class for "none" and "inf"? Line 400: Can you remove the extra empty line? CommandFormat.java There is an extra empty line in line 244. Can you reword the comment on line 247? "Used when a duplicated option is supplied to a command. Count.java Can you remove the extra empty line in line: 120 and 145? Can you break the line at line: 150 so that each line is within 80. TestCount.java Line:30 There is an extra empty line Can you remove the unused import "Import org.apache.hadoop.util.StringUtils;"
          Hide
          hadoopqa Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12707948/HDFS-7701.03.patch
          against trunk revision 3836ad6.

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 2 new or modified test files.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 javadoc. There were no new javadoc warning messages.

          +1 eclipse:eclipse. The patch built with eclipse:eclipse.

          +1 findbugs. The patch does not introduce any new Findbugs (version 2.0.3) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          +1 core tests. The patch passed unit tests in hadoop-common-project/hadoop-common.

          Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/10099//testReport/
          Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/10099//console

          This message is automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - +1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12707948/HDFS-7701.03.patch against trunk revision 3836ad6. +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 2 new or modified test files. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . There were no new javadoc warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any new Findbugs (version 2.0.3) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. +1 core tests . The patch passed unit tests in hadoop-common-project/hadoop-common. Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/10099//testReport/ Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/10099//console This message is automatically generated.
          Hide
          hadoopqa Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12707719/HDFS-7701.02.patch
          against trunk revision af618f2.

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 2 new or modified test files.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          -1 javadoc. The javadoc tool appears to have generated 1 warning messages.
          See https://builds.apache.org/job/PreCommit-HDFS-Build/10086//artifact/patchprocess/diffJavadocWarnings.txt for details.

          +1 eclipse:eclipse. The patch built with eclipse:eclipse.

          +1 findbugs. The patch does not introduce any new Findbugs (version 2.0.3) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          +1 core tests. The patch passed unit tests in hadoop-common-project/hadoop-common.

          Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/10086//testReport/
          Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/10086//console

          This message is automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12707719/HDFS-7701.02.patch against trunk revision af618f2. +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 2 new or modified test files. +1 javac . The applied patch does not increase the total number of javac compiler warnings. -1 javadoc . The javadoc tool appears to have generated 1 warning messages. See https://builds.apache.org/job/PreCommit-HDFS-Build/10086//artifact/patchprocess/diffJavadocWarnings.txt for details. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any new Findbugs (version 2.0.3) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. +1 core tests . The patch passed unit tests in hadoop-common-project/hadoop-common. Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/10086//testReport/ Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/10086//console This message is automatically generated.
          Hide
          shihaoliang Peter Shi added a comment -

          Thanks you for comments, for 4th comment, i just remove the function comment. Others are all fixed.

          Show
          shihaoliang Peter Shi added a comment - Thanks you for comments, for 4th comment, i just remove the function comment. Others are all fixed.
          Hide
          xyao Xiaoyu Yao added a comment -

          8. We want to display only storage types that support quota. You can use StorageType#getTypesSupportingQuota() instead of StroageType#asList() to get it.

          Show
          xyao Xiaoyu Yao added a comment - 8. We want to display only storage types that support quota. You can use StorageType#getTypesSupportingQuota() instead of StroageType#asList() to get it.
          Hide
          xyao Xiaoyu Yao added a comment -

          Peter Shi, thanks for the contribution. Overall, the patch looks good to me. I also built and tested the patch on my local machine.
          I will +1 (non-binding) with the following issues addressed.

          1. We should check type quota is set or not ( quota > 0) before calling formatSize. If it is not set, we should use default string "none" and "inf" for quota and rem_quota instead of showing "-1", "-1"

                          long quota, consumed;
          361	        quota = getTypeQuota(st);
          362	        consumed = getTypeConsumed(st);
          363	        content.append(String.format(STORAGE_TYPE_SUMMARY_FORMAT,
          364	            formatSize(quota, hOption),
          365	            formatSize(quota - consumed, hOption)));
          

          2. Parameter parsing issue: additional storage type parameter should not be treated as a path.

          HW11217:deploy xyao$ hdfs dfs -count  -q  -v -h -t SSD /q1
               SSD_QUOTA      REM_SSD_QUOTA PATHNAME
          count: `SSD': No such file or directory
                   300 M              300 M /q1
          

          3. The following help message can be reworded from

          "The -" + OPTION_HEADER + " option displays a header line.\n" +
          71	          "The -" + OPTION_TYPE + " option is only works when -" + OPTION_QUOTA +
          72	          " option is enabled,\n" +
          73	          "if storage type is given, \n" +
          74	          "display the quota and usage for the type, if no storage type, \n" +
          75	          "display the quota and usage for all the storage types";
          

          to

                    "The -" + OPTION_HEADER + " option displays a header line.\n" +
                    "The -" + OPTION_TYPE + " option displays quota by storage types.\n" +
                    "It must be used with -" + OPTION_QUOTA + " option.\n" +
                    "If a storage type is given after -" + OPTION_TYPE + " option, \n" +
                    "it displays the quota and usage for the specified type. \n" +
                    "Otherwise, it displays the quota and usage for all the storage \n" +
                    "types that support quota";
          

          4. We don't require javadoc for private method

           getAndCheckStorageTypes

          unless you need to explain the logic inside it. If you do, making sure the @return is updated.

          5. In TestCount.java, please import specific package individually and don't import package with wild card.

          import org.apache.hadoop.fs.*;

          6. Miss a "r" in the name of the test "processPathWithQuotasBySSDStorageTypesHeade"

          7. Fix the Jenkins failure "org.apache.hadoop.cli.TestCLI"

          Show
          xyao Xiaoyu Yao added a comment - Peter Shi , thanks for the contribution. Overall, the patch looks good to me. I also built and tested the patch on my local machine. I will +1 (non-binding) with the following issues addressed. 1. We should check type quota is set or not ( quota > 0) before calling formatSize. If it is not set, we should use default string "none" and "inf" for quota and rem_quota instead of showing "-1", "-1" long quota, consumed; 361 quota = getTypeQuota(st); 362 consumed = getTypeConsumed(st); 363 content.append( String .format(STORAGE_TYPE_SUMMARY_FORMAT, 364 formatSize(quota, hOption), 365 formatSize(quota - consumed, hOption))); 2. Parameter parsing issue: additional storage type parameter should not be treated as a path. HW11217:deploy xyao$ hdfs dfs -count -q -v -h -t SSD /q1 SSD_QUOTA REM_SSD_QUOTA PATHNAME count: `SSD': No such file or directory 300 M 300 M /q1 3. The following help message can be reworded from "The -" + OPTION_HEADER + " option displays a header line.\n" + 71 "The -" + OPTION_TYPE + " option is only works when -" + OPTION_QUOTA + 72 " option is enabled,\n" + 73 " if storage type is given, \n" + 74 "display the quota and usage for the type, if no storage type, \n" + 75 "display the quota and usage for all the storage types" ; to "The -" + OPTION_HEADER + " option displays a header line.\n" + "The -" + OPTION_TYPE + " option displays quota by storage types.\n" + "It must be used with -" + OPTION_QUOTA + " option.\n" + "If a storage type is given after -" + OPTION_TYPE + " option, \n" + "it displays the quota and usage for the specified type. \n" + "Otherwise, it displays the quota and usage for all the storage \n" + "types that support quota" ; 4. We don't require javadoc for private method getAndCheckStorageTypes unless you need to explain the logic inside it. If you do, making sure the @return is updated. 5. In TestCount.java, please import specific package individually and don't import package with wild card. import org.apache.hadoop.fs.*; 6. Miss a "r" in the name of the test "processPathWithQuotasBySSDStorageTypesHeade" 7. Fix the Jenkins failure "org.apache.hadoop.cli.TestCLI"
          Hide
          hadoopqa Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12707499/HDFS-7701.01.patch
          against trunk revision 3d0708b.

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 1 new or modified test files.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          -1 javadoc. The javadoc tool appears to have generated 1 warning messages.
          See https://builds.apache.org/job/PreCommit-HDFS-Build/10077//artifact/patchprocess/diffJavadocWarnings.txt for details.

          +1 eclipse:eclipse. The patch built with eclipse:eclipse.

          +1 findbugs. The patch does not introduce any new Findbugs (version 2.0.3) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          -1 core tests. The patch failed these unit tests in hadoop-common-project/hadoop-common:

          org.apache.hadoop.cli.TestCLI

          Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/10077//testReport/
          Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/10077//console

          This message is automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12707499/HDFS-7701.01.patch against trunk revision 3d0708b. +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 1 new or modified test files. +1 javac . The applied patch does not increase the total number of javac compiler warnings. -1 javadoc . The javadoc tool appears to have generated 1 warning messages. See https://builds.apache.org/job/PreCommit-HDFS-Build/10077//artifact/patchprocess/diffJavadocWarnings.txt for details. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any new Findbugs (version 2.0.3) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. -1 core tests . The patch failed these unit tests in hadoop-common-project/hadoop-common: org.apache.hadoop.cli.TestCLI Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/10077//testReport/ Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/10077//console This message is automatically generated.
          Hide
          xyao Xiaoyu Yao added a comment -

          Peter Shi, Thanks for working on this! Can you rebase your patch to trunk as HDFS-7824 is in trunk? I will review it today.

          Show
          xyao Xiaoyu Yao added a comment - Peter Shi , Thanks for working on this! Can you rebase your patch to trunk as HDFS-7824 is in trunk? I will review it today.
          Hide
          shihaoliang Peter Shi added a comment -

          The patch is based on HDFS-7824.05.patch

          Show
          shihaoliang Peter Shi added a comment - The patch is based on HDFS-7824 .05.patch
          Hide
          shihaoliang Peter Shi added a comment -

          This is my first patch, if any stupid error, please tell me. Thanks.

          Show
          shihaoliang Peter Shi added a comment - This is my first patch, if any stupid error, please tell me. Thanks.
          Hide
          shihaoliang Peter Shi added a comment -

          Xiaoyu Yao, Ok, please let me do this.

          Show
          shihaoliang Peter Shi added a comment - Xiaoyu Yao , Ok, please let me do this.
          Hide
          xyao Xiaoyu Yao added a comment -

          @Peter Shi, this one has a dependency on HDFS-7824. Free free to post a patch if you are interested in working on it, I will help reviewing it.

          Show
          xyao Xiaoyu Yao added a comment - @Peter Shi, this one has a dependency on HDFS-7824 . Free free to post a patch if you are interested in working on it, I will help reviewing it.
          Hide
          shihaoliang Peter Shi added a comment -

          Hi, Xiaoyu Yao

          What is the progress for this? Anything i can help?

          Show
          shihaoliang Peter Shi added a comment - Hi, Xiaoyu Yao What is the progress for this? Anything i can help?

            People

            • Assignee:
              shihaoliang Peter Shi
              Reporter:
              xyao Xiaoyu Yao
            • Votes:
              0 Vote for this issue
              Watchers:
              6 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development