[
https://issues.apache.org/jira/browse/HDFS-8900?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14705583#comment-14705583
]
Andrew Wang commented on HDFS-8900:
-----------------------------------
Thanks for working on this Yi. Change looks great, seems like a nice simple
dictionary encoding scheme.
The hard limit changes are incompatible, but I see why it's required to limit
the length to a 2B field. Other filesystems have a limit which isn't even
configurable, so seems okay. Let's flag as such and add a release note. Maybe
this should only go into trunk as a result.
Hard limit related:
* Could we exit on setting an xattr size bigger than the hard limit, rather
than doing a silent min? We should mention this new hard limit somewhere as
well.
* Any comment on the max size supported by other filesystems like ext4 or
btrfs, for reference as to what's reasonable here?
* Typo in FSDirectory: "should" -> "should be". Also the line below it says
"unlimited" but that'll never get triggered now.
* Regarding configuration, we could simplify by just using the hard limit.
Admins would still have the option of disabling xattrs entirely; is there
really any value in being able to set something smaller than 32KB? This would
definitely make it a trunk change.
More review comments:
* FSDirectory#addToInodeMap, do we need that new return?
* A bit out of scope and so optional, but I think everywhere we say
"prefixName" we really want to say "prefixedName" because "prefixName" sounds
more like the name of the prefix rather than a name with a prefix.
* Some unrelated import changes in FSNamesystemLock and INodeAttributeProvider
* XAttrFeature has an extra import
* What's the reason for switching from ImmutableList to List in some places?
The switch is also not complete, since I still see some usages of
ImmutableList. I remember we liked ImmutableList originally since it made the
need to set very explicit.
* Mind adding Javadoc for SerialNumberMap, and an interface audience private
annotation?
* XAttrsFormat's class javadoc goes over 80 chars, could use interface audience
private also.
* Can we add an IllegalStateException to SerialNumberMap#get(T) for Integer
overflow? Also there's the case that the int from the map doesn't fit in the 29
bits in XAttrFormat; check that in XAttrsFormattoBytes?
* Consider also renaming XAttrsFormat to XAttrFormat, so it's named like
XAttrStorage
* Is it worthwhile to do the same dictionary encoding for the FSImage as well?
If the # xattrs is large enough to affect memory footprint, it'd also affect
loading times which can already be minutes. Can be a follow-on JIRA for sure.
> Optimize XAttr memory footprint.
> --------------------------------
>
> Key: HDFS-8900
> URL: https://issues.apache.org/jira/browse/HDFS-8900
> Project: Hadoop HDFS
> Issue Type: Improvement
> Components: namenode
> Reporter: Yi Liu
> Assignee: Yi Liu
> Attachments: HDFS-8900.001.patch
>
>
> {code}
> private final ImmutableList<XAttr> xAttrs;
> {code}
> Currently we use above in XAttrFeature, it's not efficient from memory point
> of view, since {{ImmutableList}} and {{XAttr}} have object memory overhead,
> and each object has memory alignment.
> We can use a {{byte[]}} in XAttrFeature and do some compact in {{XAttr}}.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
