[
https://issues.apache.org/jira/browse/HDFS-11647?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16041757#comment-16041757
]
Lei (Eddy) Xu commented on HDFS-11647:
--------------------------------------
Hey, [~luhuichun]
Thanks a lot for working on this. Looks good overall. Some minor comments:
* Should {{ContentSummary#ecPolicy}} has a default value? Does user expect
{{getErasureCodingPolicy}} to return {{null}}?
* {code}
getErasureCodingPolicy() == right.getErasureCodingPolicy() &&
{code}
Because the ec policy is a string, we should check null && {{.equals()}} here.
Also, please fix the indent of this line and in {{hashCode}}.
* In {{Count.java}}, please add spaces on both sides of symbols like "+" and
"=". You could take a look of [hadoop code
style|https://wiki.apache.org/hadoop/CodeReviewChecklist] for reference.
* We can use StringBuilder in {processOptions}.
* {{hdfs.proto}}, why dont you name {{optional string redundancyPolicy = 13;}}
as {{ecPolicy}} as used everywhere else? It'd be nice to keep them consistent.
* {{ContentSummaryComputationContext#REPLICATED}} please change the signature
to {{public static final String...}} for consistency.
* {{public String getErasureCodingPolicyName(INode inode) }}, we might want to
be consistent of the signature between modules. In some places, we call it
{{ecPolicy()}}, in some places it is {{getErasureCodingPolicy}}.
* {code}
} catch (IOException ioe) {
LOG.warn("Encountered error getting ec policy for " +
inode.getFullPathName(), ioe);
return "";
}
{code}
If the {{IOE}} is HDFS related, we should throw it instead of ignore it here.
{code}
if (isStriped()) {
String ecPolicyName = summary.getErasureCodingPolicyName(this);
}
{code}
This has no side-effect to {{summary}}?
Thanks!
> Add -E option in hdfs "count" command to show erasure policy summarization
> --------------------------------------------------------------------------
>
> Key: HDFS-11647
> URL: https://issues.apache.org/jira/browse/HDFS-11647
> Project: Hadoop HDFS
> Issue Type: Improvement
> Reporter: SammiChen
> Assignee: luhuichun
> Labels: hdfs-ec-3.0-nice-to-have
> Attachments: HADOOP-11647.patch
>
>
> Add -E option in hdfs "count" command to show erasure policy summarization
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
