[ https://issues.apache.org/jira/browse/HDFS-14172?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16756173#comment-16756173 ]
Adam Antal commented on HDFS-14172: ----------------------------------- Thanks for the patch, [~water]. I did not see the comparator is so broken. This could indeed be an incompatible change, since we are modifying the order of the section. I know that it is bad, but it is working consistently, and it'd be dangerous to touch that part. So I'd conclude not to modify {{FSImageFormatProtobuf.SectionName#fromString()}}. Otherwise I think it is not an incompatible change, since as it is in trunk currently - we can't handle unknown sections anyways, as the issue states: a NPE is thrown. But if we'd detect the null (=unknown section) and throw a descriptive IOException in {{FSImageFormatProtobuf.Loader#loadInternal()}}, everybody would be satisfied. The thing is that the code now throws an NPE, when an unknown section is coming across. We should not resolve that, because although the NPE itself is not intended, the error, and to abort the program is. > Return a default SectionName to avoid NPE > ----------------------------------------- > > Key: HDFS-14172 > URL: https://issues.apache.org/jira/browse/HDFS-14172 > Project: Hadoop HDFS > Issue Type: Improvement > Reporter: Xiang Li > Assignee: Xiang Li > Priority: Minor > Attachments: HADOOP-14172.000.patch, HADOOP-14172.001.patch > > > In FSImageFormatProtobuf.SectionName#fromString(), as follows: > {code:java} > public static SectionName fromString(String name) { > for (SectionName n : values) { > if (n.name.equals(name)) > return n; > } > return null; > } > {code} > When the code meets an unknown section from the fsimage, the function will > return null. Callers always operates the return value with a "switch" clause, > like FSImageFormatProtobuf.Loader#loadInternal(), as: > {code:java} > switch (SectionName.fromString(n)) > {code} > NPE will be thrown here. > For self-protection, shall we add a default section name in the enum of > SectionName, like "UNKNOWN", to steer clear of NPE? -- This message was sent by Atlassian JIRA (v7.6.3#76005) --------------------------------------------------------------------- To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org