[ https://issues.apache.org/jira/browse/HDFS-12336?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16142620#comment-16142620 ]
Xiao Chen edited comment on HDFS-12336 at 8/26/17 5:12 AM: ----------------------------------------------------------- Thanks for the new patch Wellington, looks pretty good to me. I think we're really close. (Yea, HDFS-10899 was me, sorry for the rebase :) ) Comments, mostly nits: - I think we should pass in {{zonePath}}, instead of {{inode.getFullPathName}} to {{isValidAbsolutePath}}. The latter is more expensive, and later {{getINodesInPath}} is resolving the former. - Nit: I think we can leave out the outer () in {{return (path != null && path.startsWith(Path.SEPARATOR));}} - Nit: Test could use {{assertNotEquals}} instead of {{assertFalse(x.equals(y))}}. - Please fix checkstyle while you're at it. (I can't explain why, but we really love the '.' at the end of the first sentence) was (Author: xiaochen): Thanks for the new patch Wellington, looks pretty good to me. I think we're really close. Comments, mostly nits: - I think we should pass in {{zonePath}}, instead of {{inode.getFullPathName}} to {{isValidAbsolutePath}}. The latter is more expensive, and later {{getINodesInPath}} is resolving the former. - Nit: I think we can leave out the outer () in {{return (path != null && path.startsWith(Path.SEPARATOR));}} - Nit: Test could use {{assertNotEquals}} instead of {{assertFalse(x.equals(y))}}. - Please fix checkstyle while you're at it. (I can't explain why, but we really love the '.' at the end of the first sentence) > Listing encryption zones still fails when deleted EZ is not a direct child of > snapshottable directory > ----------------------------------------------------------------------------------------------------- > > Key: HDFS-12336 > URL: https://issues.apache.org/jira/browse/HDFS-12336 > Project: Hadoop HDFS > Issue Type: Bug > Components: hdfs > Affects Versions: 3.0.0-alpha4 > Reporter: Wellington Chevreuil > Assignee: Wellington Chevreuil > Priority: Minor > Attachments: HDFS-12336.001.patch, HDFS-12336.002.patch, > HDFS-12336.003.patch > > > The fix proposed on HDFS-11197 didn't cover the scenario where the EZ deleted > but still under a snapshot is not a direct child of the snapshottable > directory. > Here the code snippet proposed on HDFS-11197 that would avoid the error > reported by *hdfs crypto -listZones* when a deleted EZ is still under a given > snapshot: > {noformat} > INode lastINode = null; > if (inode.getParent() != null || inode.isRoot()) { > INodesInPath iip = dir.getINodesInPath(pathName, DirOp.READ_LINK); > lastINode = iip.getLastINode(); > } > if (lastINode == null || lastINode.getId() != ezi.getINodeId()) { > continue; > } > {noformat} > It will ignore EZs when it's a direct child of a snapshot, because its parent > inode will be null, and it isn't the root inode. However, if the EZ is not > directly under snapshottable directory, its parent will not be null, and it > will pass this check, so it will fail further due *absolute path required* > validation error. > I would like to work on a fix that would also cover this scenario. -- This message was sent by Atlassian JIRA (v6.4.14#64029) --------------------------------------------------------------------- To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org