hadoop-hdfs-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Chris Nauroth (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HDFS-5673) Implement logic for modification of ACLs.
Date Thu, 19 Dec 2013 00:40:08 GMT

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

Chris Nauroth commented on HDFS-5673:

Thanks for the review, Haohui.  I'm going to get to work on a v2 patch.

bq. Acl.Builder can initialize the entries field along with the declaration.

There are 2 constructors right now: one with specified capacity (used by product code, always
set to 32) and one without capacity (convenience for test code).  I'll change to a single
constructor that always uses 32 capacity.

bq. The constructor of Acl does not need to copy the list.

If we don't copy the list, then the immutability guarantee is weaker.  After building, a caller
could go back and add more entries through the builder, which would sneak in and mutate the
list.  I'd prefer to keep the copy.

bq. It seems to me that you can pass in a wrapper for Acl to indicate that the Acl list is

I'll investigate this.  If I understand correctly, we'd convert an untrusted ACL spec to an
instance of the well-formed object immediately, and that's what we would pass around between
the internal helper methods.  The type signatures would enforce that we're only passing around
properly sorted lists, so the precondition checks might not even be required at that point.

bq. I'm not sure that why AclTransformation needs to be a functor.

This was intended to avoid code duplication in all of the ACL modification methods in {{FSNamesystem}}.
 In the current HDFS-5658 patch, we're starting to see some of that code duplication related
to holding the namesystem lock and checking permissions.  With a functor, we'd be able to
have a single helper method in {{FSNameystem}} implementing that workflow, and each ACL modification
method can pass in a different functor instance.

bq. I think the code is probably overly optimized... Obviously it is turning an O ( n ) algorithm
to an O(nlogn) algorithm, but n is fairly small...

I believe that's actually O(N^2), or perhaps more accurately O(N*M) if you want to track size
of the existing ACL and size of the ACL spec separately as N and M respectively.  Still, I
understand your point.  I had an earlier version of the code that did multiple iterations.
 I found that it didn't really simplify things though.  It seems the complexity is driven
not by the iteration, but rather the rules for ACLs around automatic mask calculations, copying
of some of the default entries from the access entries if unspecified, and enforcing the validation.
 I'll play around with this a little more though and see if I can find a balance.

> Implement logic for modification of ACLs.
> -----------------------------------------
>                 Key: HDFS-5673
>                 URL: https://issues.apache.org/jira/browse/HDFS-5673
>             Project: Hadoop HDFS
>          Issue Type: Sub-task
>          Components: namenode
>    Affects Versions: HDFS ACLs (HDFS-4685)
>            Reporter: Chris Nauroth
>            Assignee: Chris Nauroth
>         Attachments: HDFS-5673.1.patch
> This patch will include the core logic for modification of ACLs.  This includes support
for all user-facing APIs that modify ACLs.  This will cover access ACLs, default ACLs, automatic
mask calculations, automatic inference of unprovided default ACL entries, and validation to
prevent creation of an invalid ACL.

This message was sent by Atlassian JIRA

View raw message