[
https://issues.apache.org/jira/browse/HDFS-7701?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14382758#comment-14382758
]
Xiaoyu Yao commented on HDFS-7701:
----------------------------------
[~shihaoliang], 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"
{code}
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)));
{code}
2. Parameter parsing issue: additional storage type parameter should not be
treated as a path.
{code}
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
{code}
3. The following help message can be reworded from
{code}
"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";
{code}
to
{code}
"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";
{code}
4. We don't require javadoc for private method {code}
getAndCheckStorageTypes{code} 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.
{code}import org.apache.hadoop.fs.*;{code}
6. Miss a "r" in the name of the test
"processPathWithQuotasBySSDStorageTypesHeade"
7. Fix the Jenkins failure "org.apache.hadoop.cli.TestCLI"
> Support reporting per storage type quota and usage with hadoop/hdfs shell
> -------------------------------------------------------------------------
>
> Key: HDFS-7701
> URL: https://issues.apache.org/jira/browse/HDFS-7701
> Project: Hadoop HDFS
> Issue Type: Sub-task
> Components: datanode, namenode
> Reporter: Xiaoyu Yao
> Assignee: Peter Shi
> Attachments: HDFS-7701.01.patch
>
>
> "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
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
