[
https://issues.apache.org/jira/browse/HDFS-14172?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16740242#comment-16740242
]
Adam Antal commented on HDFS-14172:
-----------------------------------
I support the idea of modifying this part of the code as well to make sure we
won't bump into this in the future when we add new parts to the FSImage.
Previously in {{FSImageFormatProtobuf.SectionName#fromString()}}:
{code:java}
Collections.sort(sections, new Comparator<FileSummary.Section>() {
@Override
public int compare(FileSummary.Section s1, FileSummary.Section s2) {
SectionName n1 = SectionName.fromString(s1.getName());
SectionName n2 = SectionName.fromString(s2.getName());
if (n1 == null) {
return n2 == null ? 0 : -1;
} else if (n2 == null) {
return -1;
} else {
return n1.ordinal() - n2.ordinal();
}
}
});
{code}
In this function the sections are ordered this way, and they're aware that the
SectionName can be null. I think we should follow the same pattern later.
So the switch should be something like this:
{code:java}
SectionName sectionName = SectionName.fromString(n);
if (sectionName == null) {
LOG.warn("Unrecognized section {}", n);
} else {
switch(sectionName) {
case NS_INFO:
loadNameSystemSection(in);
break;
...
}
}
{code}
Maybe at the end if there was a section which is not null, but wasn't handled
in the switch, we would throw an IOException. (Someone in the future will have
added a new section to the FSImage without handling it in {{loadInternal}},
which is a serious issue.)
If HDFS-7076 caused this issue, it is important that they should be careful
about this (not forgetting to add the STORAGE_POLICIES to
{{FSImageFormatProtobuf.SectionName}}), though that issue seems to be inactive
for a while.
> 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
>
> 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
