From 2550371f66c49fe0e40aadaa68744311270084ce Mon Sep 17 00:00:00 2001
From: Kihwal Lee <kihwal@apache.org>
Date: Fri, 19 Aug 2016 09:12:17 -0500
Subject: [PATCH] HDFS-10711. Optimize FSPermissionChecker group membership
 check. Contributed by Daryn Sharp.

---
 .../hdfs/server/namenode/FSDirAttrOp.java     |  2 +-
 .../server/namenode/FSPermissionChecker.java  | 23 ++++++-------------
 2 files changed, 8 insertions(+), 17 deletions(-)

diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirAttrOp.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirAttrOp.java
index e6d36b8fda..e19341cdca 100644
--- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirAttrOp.java
+++ b/hadoop-hdfs-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-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSPermissionChecker.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSPermissionChecker.java
index 726319fa70..c9b1c76b36 100644
--- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSPermissionChecker.java
+++ b/hadoop-hdfs-project/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,15 +89,13 @@ 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);
   }
 
@@ -108,10 +103,6 @@ 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;
     }