[
https://issues.apache.org/jira/browse/HDFS-10673?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15394332#comment-15394332
]
Jing Zhao commented on HDFS-10673:
----------------------------------
Thanks for the work, Daryn. The patch looks good to me. Only one comment: in
the following code, when checking subaccess, the {{inodeAttr}} is no longer
retrieved from the attribute provider, but directly got from the default
{{d.getSnapshotINode(snapshotId)}}. In case we have an external attribute
provider (which may still use FSPermissonChecker as its enforcer), this changes
the semantic.
{code}
- private void checkSubAccess(byte[][] pathByNameArr, int pathIdx, INode inode,
+ private void checkSubAccess(INode inode,
int snapshotId, FsAction access, boolean ignoreEmptyDir)
throws AccessControlException {
if (inode == null || !inode.isDirectory()) {
@@ -299,8 +291,11 @@ private void checkSubAccess(byte[][] pathByNameArr, int
pathIdx, INode inode,
ReadOnlyList<INode> cList = d.getChildrenList(snapshotId);
if (!(cList.isEmpty() && ignoreEmptyDir)) {
//TODO have to figure this out with inodeattribute provider
- check(getINodeAttrs(pathByNameArr, pathIdx, d, snapshotId),
- inode.getFullPathName(), access);
+ INodeAttributes inodeAttr = d.getSnapshotINode(snapshotId);
+ if (!hasPermission(inodeAttr, access)) {
+ throw new AccessControlException(
+ toAccessControlString(inodeAttr, d.getFullPathName(), access));
+ }
}
{code}
+1 after addressing the comment.
> Optimize FSPermissionChecker's internal path usage
> --------------------------------------------------
>
> Key: HDFS-10673
> URL: https://issues.apache.org/jira/browse/HDFS-10673
> Project: Hadoop HDFS
> Issue Type: Sub-task
> Components: hdfs
> Reporter: Daryn Sharp
> Assignee: Daryn Sharp
> Attachments: HDFS-10673.patch
>
>
> The INodeAttributeProvider and AccessControlEnforcer features degrade
> performance and generate excessive garbage even when neither is used. Main
> issues:
> # A byte[][] of components is unnecessarily created. Each path component
> lookup converts a subrange of the byte[][] to a new String[] - then not used
> by default attribute provider.
> # Subaccess checks are insanely expensive. The full path of every subdir is
> created by walking up the inode tree, creating a INode[], building a string
> by converting each inode's byte[] name to a string, etc. Which will only be
> used if there's an exception.
> The expensive of #1 should only be incurred when using the provider/enforcer
> feature. For #2, paths should be created on-demand for exceptions.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
