[jira] [Commented] (HDFS-6301) NameNode: persist XAttrs in fsimage and record XAttrs modifications to edit log.
[
https://issues.apache.org/jira/browse/HDFS-6301?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15868107#comment-15868107
]
Xiao Chen commented on HDFS-6301:
-
FYI - created HDFS-11410 for the above and Andrew +1'ed, plan to commit today.
> 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.1.patch, 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.3.15#6346)
-
To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org
[jira] [Commented] (HDFS-6301) NameNode: persist XAttrs in fsimage and record XAttrs modifications to edit log.
[
https://issues.apache.org/jira/browse/HDFS-6301?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15862115#comment-15862115
]
Xiao Chen commented on HDFS-6301:
-
Hi, sorry for pinging on this 3yo jira...
[~andrew.wang] recently had a comment on HDFS-10899, which I traced back to
this jira.
{quote}
Looks like we aren't using the op cache in FSEditLog SetXAttrOp /
RemoveXAttrOp. I think this is accidental, could you do some research?
Particularly since we'll be doing a lot of SetXAttrOps, avoiding all that
object allocation would be nice. This could be a separate JIRA.
{quote}
Checking on latest trunk, only {{SetXAttrOp}}, {{RemoveXAttrOp}} and
{{SetAclOp}} doesn't use the cache. I can't think of why a cache can't be used
in this case, and would be happy to fix this in a new jira if this is a bug.
Could you please confirm?
i.e.
{code}
static SetXAttrOp getInstance() {
return new SetXAttrOp();
}
{code}
v.s.
{code}
static AddOp getInstance(OpInstanceCache cache) {
return (AddOp) cache.get(OP_ADD);
}
{code}
Thanks.
> 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.1.patch, 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.3.15#6346)
-
To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org
[jira] [Commented] (HDFS-6301) NameNode: persist XAttrs in fsimage and record XAttrs modifications to edit log.
[
https://issues.apache.org/jira/browse/HDFS-6301?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13989520#comment-13989520
]
Uma Maheswara Rao G commented on HDFS-6301:
---
+1 on the latest patch. Thanks for the reviews Andrew and Charles. I will file
separate JIRA for OP_SET_XATTRS optimization above mentioned.
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.1.patch, 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)
[jira] [Commented] (HDFS-6301) NameNode: persist XAttrs in fsimage and record XAttrs modifications to edit log.
[
https://issues.apache.org/jira/browse/HDFS-6301?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13988644#comment-13988644
]
Uma Maheswara Rao G commented on HDFS-6301:
---
{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}
Yes, you are right that we will not have any xattrs on file/dir creation time.
{quote}
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}
+1 to do on followup JIRAs
Thinking on OP_SET_XATTRS op code,
When a new xattrs set on an Inode, it may add this with OP_SET_XATTRS and
let's say [USER.name1:value1]
On a next call if we set another xattrs, then it may store along with older
existing xattrs again. It may be like [USER.name1:value1, USER.name2:value2]
So, on adding more xattrs on same Inode, that list may grow and we keep store
the entries of already existing name, value fairs.
Right now we defaulted the max Xattrs on an Inode to 32 and configured. If
user modified it to much larger value and start setting xattrs, then edits
loading may create issue like my above example.
But I didn't refer any usecase of having large number of xattrs, this is just
the assumption to consider a case.
So, here is a thought on having OP_SET_XATTR for each setXAttr operation to be
logged, When we replay them we need to consolidate. This is some initial
thought we can think more if others also feel we need to consider this case to
handle.
But we don't need to block the current JIRA, we can do it in followup JIRA as
well.
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)
[jira] [Commented] (HDFS-6301) NameNode: persist XAttrs in fsimage and record XAttrs modifications to edit log.
[
https://issues.apache.org/jira/browse/HDFS-6301?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=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)
[jira] [Commented] (HDFS-6301) NameNode: persist XAttrs in fsimage and record XAttrs modifications to edit log.
[
https://issues.apache.org/jira/browse/HDFS-6301?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13988463#comment-13988463
]
Andrew Wang commented on HDFS-6301:
---
This patch needs a small rebase, but overall looks good. A few review comments:
* 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.
* buildXattrs, should we mask the getNameSpace() for paranoia?
* 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.
Thanks Yi.
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)
[jira] [Commented] (HDFS-6301) NameNode: persist XAttrs in fsimage and record XAttrs modifications to edit log.
[
https://issues.apache.org/jira/browse/HDFS-6301?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13985924#comment-13985924
]
Charles Lamb commented on HDFS-6301:
Hi Yi,
In general this looks fine.
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.
FSEditLog.java:
s/src =src/src = src/
final could be added to the decl ofSetXAttrsOp.
FSEditLogOp.java:
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?
In general things like boolean hasName could benefit from final decls.
if (this.opCode == OP_ADD) {
AclEditLogUtil.write(aclEntries, out);
+XAttrsEditLogUtil.write(xAttrs, out);
FSImageSerialization.writeString(clientName,out);
FSImageSerialization.writeString(clientMachine,out);
// write clientId and callId
@@ -542,6 +617,7 @@
// clientname, clientMachine and block locations of last block.
if (this.opCode == OP_ADD) {
aclEntries = AclEditLogUtil.read(in, logVersion);
+xAttrs = XAttrsEditLogUtil.read(in, logVersion);
this.clientName = FSImageSerialization.readString(in);
this.clientMachine = FSImageSerialization.readString(in);
// read clientId and callId
Is it possible for xAttrs (and aclEntries for that matter) to be uninit'd if
opCode != OP_ADD? I'm looking at this.clientName and this.clientMachine and
wondering why there isn't an equivalent for xAttrs (and aclEntries) in the else?
void readFields(DataInputStream in, int logVersion) throws IOException {
XAttrEditLogProto p = XAttrEditLogProto.parseDelimitedFrom(
(DataInputStream)in);
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 } closing the class def.
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)
