[ 
https://issues.apache.org/jira/browse/HDFS-12336?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16142620#comment-16142620
 ] 

Xiao Chen commented on HDFS-12336:
----------------------------------

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

Reply via email to