[ 
https://issues.apache.org/jira/browse/HDFS-12682?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16226023#comment-16226023
 ] 

Lei (Eddy) Xu commented on HDFS-12682:
--------------------------------------

Hi, [~xiaochen]
Thanks for the updates. Overall LGTM.

Minor nits:

{code}
final ErasureCodingPolicyInfo info = policiesByName.get(defaultPolicyName);
{code}

It should use {{defaultPolicyName.trim()}} as well.

{code}
public void setState(final ErasureCodingPolicyState newState) {
  state = newState;
}
{code}

Maybe do a precondition check not null here. 

{code}
 private ErasureCodingPolicyState getPolicyState(
802           final ErasureCodingPolicy policy) {
803         ErasureCodingPolicyInfo[] policyInfos =
804             ErasureCodingPolicyManager.getInstance().getPolicies();
805         for (ErasureCodingPolicyInfo pi : policyInfos) {
806           if (pi.getPolicy().equals(policy)) {
807             return pi.getState();
808           }
809         }
810         throw new IllegalArgumentException("Policy <" + policy + "> is not 
in"
811             + " system policies:" + Arrays.toString(policyInfos));
812       }
{code}

The above code is duplicated in multiple tests, could you consolidate them?

+1 pending the changes.



> ECAdmin -listPolicies will always show SystemErasureCodingPolicies state as 
> DISABLED
> ------------------------------------------------------------------------------------
>
>                 Key: HDFS-12682
>                 URL: https://issues.apache.org/jira/browse/HDFS-12682
>             Project: Hadoop HDFS
>          Issue Type: Bug
>          Components: erasure-coding
>            Reporter: Xiao Chen
>            Assignee: Xiao Chen
>            Priority: Blocker
>              Labels: hdfs-ec-3.0-must-do
>         Attachments: HDFS-12682.01.patch, HDFS-12682.02.patch, 
> HDFS-12682.03.patch, HDFS-12682.04.patch
>
>
> On a real cluster, {{hdfs ec -listPolicies}} will always show policy state as 
> DISABLED.
> {noformat}
> [hdfs@nightly6x-1 root]$ hdfs ec -listPolicies
> Erasure Coding Policies:
> ErasureCodingPolicy=[Name=RS-10-4-1024k, Schema=[ECSchema=[Codec=rs, 
> numDataUnits=10, numParityUnits=4]], CellSize=1048576, Id=5, State=DISABLED]
> ErasureCodingPolicy=[Name=RS-3-2-1024k, Schema=[ECSchema=[Codec=rs, 
> numDataUnits=3, numParityUnits=2]], CellSize=1048576, Id=2, State=DISABLED]
> ErasureCodingPolicy=[Name=RS-6-3-1024k, Schema=[ECSchema=[Codec=rs, 
> numDataUnits=6, numParityUnits=3]], CellSize=1048576, Id=1, State=DISABLED]
> ErasureCodingPolicy=[Name=RS-LEGACY-6-3-1024k, 
> Schema=[ECSchema=[Codec=rs-legacy, numDataUnits=6, numParityUnits=3]], 
> CellSize=1048576, Id=3, State=DISABLED]
> ErasureCodingPolicy=[Name=XOR-2-1-1024k, Schema=[ECSchema=[Codec=xor, 
> numDataUnits=2, numParityUnits=1]], CellSize=1048576, Id=4, State=DISABLED]
> [hdfs@nightly6x-1 root]$ hdfs ec -getPolicy -path /ecec
> XOR-2-1-1024k
> {noformat}
> This is because when [deserializing 
> protobuf|https://github.com/apache/hadoop/blob/branch-3.0.0-beta1/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/protocolPB/PBHelperClient.java#L2942],
>  the static instance of [SystemErasureCodingPolicies 
> class|https://github.com/apache/hadoop/blob/branch-3.0.0-beta1/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/protocol/SystemErasureCodingPolicies.java#L101]
>  is first checked, and always returns the cached policy objects, which are 
> created by default with state=DISABLED.
> All the existing unit tests pass, because that static instance that the 
> client (e.g. ECAdmin) reads in unit test is updated by NN. :)



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

---------------------------------------------------------------------
To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org

Reply via email to