[
https://issues.apache.org/jira/browse/HDFS-10996?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15649705#comment-15649705
]
Rakesh R commented on HDFS-10996:
---------------------------------
Thanks [~Sammi] for the good work. I've few comments on the patch, please take
care.
# Few typos:
{code}
"file's parent director" should be "file's parent directory"
"erasureCodingPlolicy" should be "erasureCodingPolicy"
{code}
# I couldn't see any test cases by taking a valid or invalid
{{erasureCodingPolicy}} paramter behaviors. Appreciate adding more test cases
covering the same.
# Please provide javadoc for the new API DistributedFileSystem#create()
# I'd suggest to rephrase this javadoc saying that, dfs will support arbitrary
replication factors, not just 3x
{code}
+ * @param erasureCodingPolicy the name of erasure coding policy. A empty
+ * string value means this file will inherit its
+ * parent group's policy, either 3x replication or
+ * erasure coding policy.
{code}
This could be like below or a better one.
{code}
+ * @param erasureCodingPolicy the name of erasure coding policy. A empty
+ * string value means this file will inherit its
+ * parent group's policy. If parent doesn't have
+ * erasure code policy configured then continue
+ * with the traditional file replication mode.
{code}
# Could you simplify {{if (erasureCodingPolicy != null &&
(!erasureCodingPolicy.isEmpty())) {}} check with
{code}
org.apache.commons.lang.StringUtils.isBlank(erasureCodingPolicy)
{code}
# Missing indentation for {{erasureCodingPolicy+ " ] does not match any of the
" +}}. There should a space between {{erasureCodingPolicy+ }}
{code}
erasureCodingPolicy + " ] does not match any of the " +
{code}
# It looks like test case failures are related to the patch. Just analysed
{{TestFcHdfsCreateMkdir/testMkdirRecursiveWithExistingDir_2/}} and could see
that the test failed due to {{null}} value to
{{protos#setErasureCodingPolicy}}. Please take care this. Thanks!
In general, I hope you might have tried improving the
{{setErasureCodingPolicy}} API based on [your previous
comments|https://issues.apache.org/jira/browse/HDFS-10996?focusedCommentId=15614112&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-15614112]
and due to its complexity you have gone with the {{#create}} API, right?
> Ability to specify per-file EC policy at create time
> ----------------------------------------------------
>
> Key: HDFS-10996
> URL: https://issues.apache.org/jira/browse/HDFS-10996
> Project: Hadoop HDFS
> Issue Type: Sub-task
> Components: erasure-coding
> Affects Versions: 3.0.0-alpha1
> Reporter: Andrew Wang
> Assignee: SammiChen
> Attachments: HDFS-10996-v1.patch
>
>
> Based on discussion in HDFS-10971, it would be useful to specify the EC
> policy when the file is created. This is useful for situations where app
> requirements do not map nicely to the current directory-level policies.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
