[
https://issues.apache.org/jira/browse/HDFS-12336?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16141247#comment-16141247
]
Xiao Chen commented on HDFS-12336:
----------------------------------
Thanks for revving and providing more details Wellington!
It all makes sense to me, a few comments:
- for the {{isValidEZPath}} check, can we check {{pathName}} for absolute path,
instead of walking through the inode all the way to the root? INode.java
currently has {{checkAbsolutePath}} which is exactly where the exception is
thrown now. Perhaps we can extract the if condition to a helper method, and
reuse it in our {{isValidEZPath}} check?
- Suggest to use javadoc on the {{isValidEZPath}} method comment. Replacing the
'/*' with '/**' should do.
- The added test looks good. I think we should call out why move to trash then
delete is necessary (instead of fsWrapper.delete): to create a inodereference
for the test.
- Instead of 2 {{assertZonePresent}}, it feels to me we could list the zones
once, then check things while going through the iterator.
- Similarly for the last assert, maybe we can verify from listing directly,
rather than the current way of catching assertion error on a method named
{{assertZonePresent}} and assert the message. :)
> 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
>
>
> 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
