[
https://issues.apache.org/jira/browse/HDFS-6301?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13988649#comment-13988649
]
Yi Liu commented on HDFS-6301:
------------------------------
Thanks Andrew and Charles for review.
I will update the patch for your comments.
{quote}
I noticed that you never say anywhere whether namespaces and xattr names are
case sensitive or insensitive. That should be spelled out somewhere in the
javadoc
{quote}
xattr namespace and name are case sensitive, it's in line with Linux. we have
this description in {{XAttrHelper#buildXAttr}}. If you think we should specify
this information in interface javadoc, we can add it when touching that file.
{quote}
s/src =src/src = src/
"final" could be added to the decl ofSetXAttrsOp.
{quote}
Right, let's update.
{quote}
XAttrsEditLogUtil: when would an xattr not have a name? I see why
HAS_VALUE_OFFSET is necessary (some xattrs are name only), but why do you need
a flag for whether a name is present? Don't all xattrs have at least a name?
{quote}
Right {{hasName}} is not necessary. And we decide to remove xattr for OP_ADD,
so this issue will not exist.
{quote}
I don't understand why you need the (DataInputStream) cast above. Isn't it
already known to be a DIS from the formal arg decl? TestFSImageWithXAttr.java:
There's an extra newline just before the last }
{quote}
Right, the cast is not necessary, let's remove it. And extra newline in
{{TestFSImageWithXAttr}} will be removed.
{quote}
This patch needs a small rebase
{quote}
Right, new patch will be rebased.
{quote}
Why do we need to modify OP_ADD, and have xattrs be an argument to
unprotectedAddFile? AFAIK new files don't have any xattrs until they're set.
The other use of OP_ADD is for append, which also does not involve setting
xattrs. This looks like c+p from the ACL code, which needs it because of
default ACLs.
{quote}
OK, let's remove xattrs for OP_ADD. My original thought was we may extend
create dir and add file interfaces to have initial xattrs.
{quote}
buildXattrs, should we mask the getNameSpace() for paranoia?
{quote}
Right, we should mask.
{quote}
Have you done a full test run? I know for one that DFSTestUtil#runOperations
needs to be updated with the new op, and the test resources related to
TestOfflineEditsViewer will need to be updated. These are fairly mechanical and
we can do it in a follow-on if you like.
{quote}
I ran some relevant tests and missed {{TestOfflineEditsViewer}}, let's track
them in follow-on JIRA.
> NameNode: persist XAttrs in fsimage and record XAttrs modifications to edit
> log.
> --------------------------------------------------------------------------------
>
> Key: HDFS-6301
> URL: https://issues.apache.org/jira/browse/HDFS-6301
> Project: Hadoop HDFS
> Issue Type: Sub-task
> Components: namenode
> Affects Versions: HDFS XAttrs (HDFS-2006)
> Reporter: Yi Liu
> Assignee: Yi Liu
> Fix For: HDFS XAttrs (HDFS-2006)
>
> Attachments: HDFS-6301.patch
>
>
> Store XAttrs in fsimage so that XAttrs are retained across NameNode restarts.
> Implement a new edit log opcode, {{OP_SET_XATTRS}}.
--
This message was sent by Atlassian JIRA
(v6.2#6252)
