HDFS-10711. Optimize FSPermissionChecker group membership check. Cont…

…ributed by Daryn Sharp.
apache · Aug 19, 2016 · 2550371 · 2550371
1 parent 091dd19
commit 2550371
Show file tree

Hide file tree

Showing 2 changed files with 8 additions and 17 deletions.
diff --git a/...project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirAttrOp.java b/...project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirAttrOp.java
@@ -88,7 +88,7 @@ static HdfsFileStatus setOwner(
         if (username != null && !pc.getUser().equals(username)) {
           throw new AccessControlException("Non-super user cannot change owner");
         }
-        if (group != null && !pc.containsGroup(group)) {
+        if (group != null && !pc.isMemberOfGroup(group)) {
           throw new AccessControlException("User does not belong to " + group);
         }
       }

diff --git a/...hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSPermissionChecker.java b/...hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSPermissionChecker.java
@@ -17,10 +17,7 @@
  */
 package org.apache.hadoop.hdfs.server.namenode;
 
-import java.util.Arrays;
-import java.util.Collections;
-import java.util.HashSet;
-import java.util.Set;
+import java.util.Collection;
 import java.util.Stack;
 
 import org.apache.commons.logging.Log;
@@ -81,7 +78,7 @@ private String toAccessControlString(INodeAttributes inodeAttrib,
   private final UserGroupInformation callerUgi;
 
   private final String user;
-  private final Set<String> groups;
+  private final Collection<String> groups;
   private final boolean isSuper;
   private final INodeAttributeProvider attributeProvider;
 
@@ -92,26 +89,20 @@ private String toAccessControlString(INodeAttributes inodeAttrib,
     this.fsOwner = fsOwner;
     this.supergroup = supergroup;
     this.callerUgi = callerUgi;
-    HashSet<String> s =
-        new HashSet<String>(Arrays.asList(callerUgi.getGroupNames()));
-    groups = Collections.unmodifiableSet(s);
+    this.groups = callerUgi.getGroups();
     user = callerUgi.getShortUserName();
     isSuper = user.equals(fsOwner) || groups.contains(supergroup);
     this.attributeProvider = attributeProvider;
   }
 
-  public boolean containsGroup(String group) {
+  public boolean isMemberOfGroup(String group) {
     return groups.contains(group);
   }
 
   public String getUser() {
     return user;
   }
 
-  public Set<String> getGroups() {
-    return groups;
-  }
-
   public boolean isSuperUser() {
     return isSuper;
   }
@@ -337,7 +328,7 @@ private boolean hasPermission(INodeAttributes inode, FsAction access) {
     final FsAction checkAction;
     if (getUser().equals(inode.getUserName())) { //user class
       checkAction = mode.getUserAction();
-    } else if (getGroups().contains(inode.getGroupName())) { //group class
+    } else if (isMemberOfGroup(inode.getGroupName())) { //group class
       checkAction = mode.getGroupAction();
     } else { //other class
       checkAction = mode.getOtherAction();
@@ -407,7 +398,7 @@ private boolean hasAclPermission(INodeAttributes inode,
           // member of multiple groups that have entries that grant access, then
           // it doesn't matter which is chosen, so exit early after first match.
           String group = name == null ? inode.getGroupName() : name;
-          if (getGroups().contains(group)) {
+          if (isMemberOfGroup(group)) {
             FsAction masked = AclEntryStatusFormat.getPermission(entry).and(
                 mode.getGroupAction());
             if (masked.implies(access)) {
@@ -470,7 +461,7 @@ public void checkPermission(CachePool pool, FsAction access)
         && mode.getUserAction().implies(access)) {
       return;
     }
-    if (getGroups().contains(pool.getGroupName())
+    if (isMemberOfGroup(pool.getGroupName())
         && mode.getGroupAction().implies(access)) {
       return;
     }