[ https://issues.apache.org/jira/browse/HDFS-12918?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16288521#comment-16288521 ]
Manoj Govindassamy commented on HDFS-12918: ------------------------------------------- A new check added in the convert seems to be not backward compatible. It is going to break the upgrade from previous image format where the ErasureCodingPolicyProto didn't have state field. It is suppose to be an optional field and the below check need to be relaxed as well. [~xiaochen] your thoughts please? {noformat} /** * Convert the protobuf to a {@link ErasureCodingPolicyInfo}. This should only * be needed when the caller is interested in the state of the policy. */ public static ErasureCodingPolicyInfo convertErasureCodingPolicyInfo( ErasureCodingPolicyProto proto) { ErasureCodingPolicy policy = convertErasureCodingPolicy(proto); ErasureCodingPolicyInfo info = new ErasureCodingPolicyInfo(policy); Preconditions.checkArgument(proto.hasState(), <====== "Missing state field in ErasureCodingPolicy proto"); info.setState(convertECState(proto.getState())); return info; } {noformat} > EC Policy defaults incorrectly to enabled in protobufs > ------------------------------------------------------ > > Key: HDFS-12918 > URL: https://issues.apache.org/jira/browse/HDFS-12918 > Project: Hadoop HDFS > Issue Type: Bug > Reporter: Zach Amsden > Assignee: Manoj Govindassamy > Priority: Critical > > According to documentation and code comments, the default setting for erasure > coding policy is disabled: > /** Policy is disabled. It's policy default state. */ > DISABLED(1), > However, HDFS-12258 appears to have incorrectly set the policy state in the > protobuf to enabled: > {code:java} > message ErasureCodingPolicyProto { > ooptional string name = 1; > optional ECSchemaProto schema = 2; > optional uint32 cellSize = 3; > required uint32 id = 4; // Actually a byte - only 8 bits used > + optional ErasureCodingPolicyState state = 5 [default = ENABLED]; > } > {code} > This means the parameter can't actually be optional, it must always be > included, and existing serialized data without this optional field will be > incorrectly interpreted as having erasure coding enabled. > This unnecessarily breaks compatibility and will require existing HDFS > installations that store metadata in protobufs to require reformatting. > It looks like a simple mistake that was overlooked in code review. -- 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