Revert "HADOOP-17669. Backport HADOOP-17079, HADOOP-17505 to branch-3.3 (#2959)"

This reverts commit 4ffe5eb1ddb4aa260134005335a003ed6d270685.

hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FileSystem.java

@@ -2747,7 +2747,7 @@ static void checkAccessPermissions(FileStatus stat, FsAction mode)
       if (perm.getUserAction().implies(mode)) {
         return;
       }
-    } else if (ugi.getGroupsSet().contains(stat.getGroup())) {
+    } else if (ugi.getGroups().contains(stat.getGroup())) {
       if (perm.getGroupAction().implies(mode)) {
         return;
       }

hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/SecureIOUtils.java

@@ -272,7 +272,7 @@ private static void checkStat(File f, String owner, String group,
             UserGroupInformation.createRemoteUser(expectedOwner);
         final String adminsGroupString = "Administrators";
         success = owner.equals(adminsGroupString)
-            && ugi.getGroupsSet().contains(adminsGroupString);
+            && ugi.getGroups().contains(adminsGroupString);
       } else {
         success = false;
       }

hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/CompositeGroupsMapping.java

@@ -19,7 +19,6 @@
 
 import java.io.IOException;
 import java.util.ArrayList;
-import java.util.HashSet;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
@@ -108,29 +107,6 @@ public void cacheGroupsAdd(List<String> groups) throws IOException {
     // does nothing in this provider of user to groups mapping
   }
 
-  @Override
-  public synchronized Set<String> getGroupsSet(String user) throws IOException {
-    Set<String> groupSet = new HashSet<String>();
-
-    Set<String> groups = null;
-    for (GroupMappingServiceProvider provider : providersList) {
-      try {
-        groups = provider.getGroupsSet(user);
-      } catch (Exception e) {
-        LOG.warn("Unable to get groups for user {} via {} because: {}",
-            user, provider.getClass().getSimpleName(), e.toString());
-        LOG.debug("Stacktrace: ", e);
-      }
-      if (groups != null && !groups.isEmpty()) {
-        groupSet.addAll(groups);
-        if (!combined) {
-          break;
-        }
-      }
-    }
-    return groupSet;
-  }
-
   @Override
   public synchronized Configuration getConf() {
     return conf;

hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/GroupMappingServiceProvider.java

@@ -18,9 +18,7 @@
 package org.apache.hadoop.security;
 
 import java.io.IOException;
-import java.util.LinkedHashSet;
 import java.util.List;
-import java.util.Set;
 
 import org.apache.hadoop.classification.InterfaceAudience;
 import org.apache.hadoop.classification.InterfaceStability;
@@ -54,16 +52,4 @@ public interface GroupMappingServiceProvider {
    * @throws IOException
    */
   public void cacheGroupsAdd(List<String> groups) throws IOException;
-
-  /**
-   * Get all various group memberships of a given user.
-   * Returns EMPTY set in case of non-existing user
-   * @param user User's name
-   * @return set of group memberships of user
-   * @throws IOException
-   */
-  default Set<String> getGroupsSet(String user) throws IOException {
-    //Override to form the set directly to avoid another conversion
-    return new LinkedHashSet<>(getGroups(user));
-  }
 }

hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/Groups.java

@@ -26,6 +26,7 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.concurrent.Callable;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.LinkedBlockingQueue;
 import java.util.concurrent.ThreadFactory;
@@ -77,8 +78,8 @@ public class Groups {
   
   private final GroupMappingServiceProvider impl;
 
-  private final LoadingCache<String, Set<String>> cache;
+  private final LoadingCache<String, List<String>> cache;
-  private final AtomicReference<Map<String, Set<String>>> staticMapRef =
+  private final AtomicReference<Map<String, List<String>>> staticMapRef =
       new AtomicReference<>();
   private final long cacheTimeout;
   private final long negativeCacheTimeout;
@@ -167,7 +168,8 @@ private void parseStaticMapping(Configuration conf) {
         CommonConfigurationKeys.HADOOP_USER_GROUP_STATIC_OVERRIDES_DEFAULT);
     Collection<String> mappings = StringUtils.getStringCollection(
         staticMapping, ";");
-    Map<String, Set<String>> staticUserToGroupsMap = new HashMap<>();
+    Map<String, List<String>> staticUserToGroupsMap =
+        new HashMap<String, List<String>>();
     for (String users : mappings) {
       Collection<String> userToGroups = StringUtils.getStringCollection(users,
           "=");
@@ -179,10 +181,10 @@ private void parseStaticMapping(Configuration conf) {
       String[] userToGroupsArray = userToGroups.toArray(new String[userToGroups
           .size()]);
       String user = userToGroupsArray[0];
-      Set<String> groups = Collections.emptySet();
+      List<String> groups = Collections.emptyList();
       if (userToGroupsArray.length == 2) {
-        groups = new LinkedHashSet(StringUtils
+        groups = (List<String>) StringUtils
-            .getStringCollection(userToGroupsArray[1]));
+            .getStringCollection(userToGroupsArray[1]);
       }
       staticUserToGroupsMap.put(user, groups);
     }
@@ -201,47 +203,15 @@ private IOException noGroupsForUser(String user) {
   /**
    * Get the group memberships of a given user.
    * If the user's group is not cached, this method may block.
-   * Note this method can be expensive as it involves Set->List conversion.
-   * For user with large group membership (i.e., > 1000 groups), we recommend
-   * using getGroupSet to avoid the conversion and fast membership look up via
-   * contains().
    * @param user User's name
-   * @return the group memberships of the user as list
+   * @return the group memberships of the user
    * @throws IOException if user does not exist
-   * @deprecated Use {@link #getGroupsSet(String user)} instead.
    */
-  @Deprecated
   public List<String> getGroups(final String user) throws IOException {
-    return Collections.unmodifiableList(new ArrayList<>(
-        getGroupInternal(user)));
-  }
-
-  /**
-   * Get the group memberships of a given user.
-   * If the user's group is not cached, this method may block.
-   * This provide better performance when user has large group membership via
-   * 1) avoid set->list->set conversion for the caller UGI/PermissionCheck
-   * 2) fast lookup using contains() via Set instead of List
-   * @param user User's name
-   * @return the group memberships of the user as set
-   * @throws IOException if user does not exist
-   */
-  public Set<String> getGroupsSet(final String user) throws IOException {
-    return Collections.unmodifiableSet(getGroupInternal(user));
-  }
-
-  /**
-   * Get the group memberships of a given user.
-   * If the user's group is not cached, this method may block.
-   * @param user User's name
-   * @return the group memberships of the user as Set
-   * @throws IOException if user does not exist
-   */
-  private Set<String> getGroupInternal(final String user) throws IOException {
     // No need to lookup for groups of static users
-    Map<String, Set<String>> staticUserToGroupsMap = staticMapRef.get();
+    Map<String, List<String>> staticUserToGroupsMap = staticMapRef.get();
     if (staticUserToGroupsMap != null) {
-      Set<String> staticMapping = staticUserToGroupsMap.get(user);
+      List<String> staticMapping = staticUserToGroupsMap.get(user);
       if (staticMapping != null) {
         return staticMapping;
       }
@@ -297,7 +267,7 @@ public long read() {
   /**
    * Deals with loading data into the cache.
    */
-  private class GroupCacheLoader extends CacheLoader<String, Set<String>> {
+  private class GroupCacheLoader extends CacheLoader<String, List<String>> {
 
     private ListeningExecutorService executorService;
 
@@ -338,7 +308,7 @@ private class GroupCacheLoader extends CacheLoader<String, Set<String>> {
      * @throws IOException to prevent caching negative entries
      */
     @Override
-    public Set<String> load(String user) throws Exception {
+    public List<String> load(String user) throws Exception {
       LOG.debug("GroupCacheLoader - load.");
       TraceScope scope = null;
       Tracer tracer = Tracer.curThreadTracer();
@@ -346,9 +316,9 @@ public Set<String> load(String user) throws Exception {
         scope = tracer.newScope("Groups#fetchGroupList");
         scope.addKVAnnotation("user", user);
       }
-      Set<String> groups = null;
+      List<String> groups = null;
       try {
-        groups = fetchGroupSet(user);
+        groups = fetchGroupList(user);
       } finally {
         if (scope != null) {
           scope.close();
@@ -364,7 +334,9 @@ public Set<String> load(String user) throws Exception {
         throw noGroupsForUser(user);
       }
 
-      return groups;
+      // return immutable de-duped list
+      return Collections.unmodifiableList(
+          new ArrayList<>(new LinkedHashSet<>(groups)));
     }
 
     /**
@@ -373,8 +345,8 @@ public Set<String> load(String user) throws Exception {
      * implementation, otherwise is arranges for the cache to be updated later
      */
     @Override
-    public ListenableFuture<Set<String>> reload(final String key,
+    public ListenableFuture<List<String>> reload(final String key,
-                                                 Set<String> oldValue)
+                                                 List<String> oldValue)
         throws Exception {
       LOG.debug("GroupCacheLoader - reload (async).");
       if (!reloadGroupsInBackground) {
@@ -382,16 +354,19 @@ public ListenableFuture<Set<String>> reload(final String key,
       }
 
       backgroundRefreshQueued.incrementAndGet();
-      ListenableFuture<Set<String>> listenableFuture =
+      ListenableFuture<List<String>> listenableFuture =
-          executorService.submit(() -> {
+          executorService.submit(new Callable<List<String>>() {
-            backgroundRefreshQueued.decrementAndGet();
+            @Override
-            backgroundRefreshRunning.incrementAndGet();
+            public List<String> call() throws Exception {
-            Set<String> results = load(key);
+              backgroundRefreshQueued.decrementAndGet();
-            return results;
+              backgroundRefreshRunning.incrementAndGet();
+              List<String> results = load(key);
+              return results;
+            }
           });
-      Futures.addCallback(listenableFuture, new FutureCallback<Set<String>>() {
+      Futures.addCallback(listenableFuture, new FutureCallback<List<String>>() {
         @Override
-        public void onSuccess(Set<String> result) {
+        public void onSuccess(List<String> result) {
           backgroundRefreshSuccess.incrementAndGet();
           backgroundRefreshRunning.decrementAndGet();
         }
@@ -405,12 +380,11 @@ public void onFailure(Throwable t) {
     }
 
     /**
-     * Queries impl for groups belonging to the user.
+     * Queries impl for groups belonging to the user. This could involve I/O and take awhile.
-     * This could involve I/O and take awhile.
      */
-    private Set<String> fetchGroupSet(String user) throws IOException {
+    private List<String> fetchGroupList(String user) throws IOException {
       long startMs = timer.monotonicNow();
-      Set<String> groups = impl.getGroupsSet(user);
+      List<String> groupList = impl.getGroups(user);
       long endMs = timer.monotonicNow();
       long deltaMs = endMs - startMs ;
       UserGroupInformation.metrics.addGetGroups(deltaMs);
@@ -418,7 +392,8 @@ private Set<String> fetchGroupSet(String user) throws IOException {
         LOG.warn("Potential performance problem: getGroups(user=" + user +") " +
           "took " + deltaMs + " milliseconds.");
       }
-      return groups;
+
+      return groupList;
     }
   }
 

hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/JniBasedUnixGroupsMapping.java

@@ -20,11 +20,8 @@
 
 import java.io.IOException;
 import java.util.Arrays;
-import java.util.LinkedHashSet;
 import java.util.List;
-import java.util.Set;
 
-import org.apache.commons.collections.CollectionUtils;
 import org.apache.hadoop.classification.InterfaceAudience;
 import org.apache.hadoop.classification.InterfaceStability;
 
@@ -78,18 +75,6 @@ static private void logError(int groupId, String error) {
 
   @Override
   public List<String> getGroups(String user) throws IOException {
-    return Arrays.asList(getGroupsInternal(user));
-  }
-
-  @Override
-  public Set<String> getGroupsSet(String user) throws IOException {
-    String[] groups = getGroupsInternal(user);
-    Set<String> result = new LinkedHashSet(groups.length);
-    CollectionUtils.addAll(result, groups);
-    return result;
-  }
-
-  private String[] getGroupsInternal(String user) throws IOException {
     String[] groups = new String[0];
     try {
       groups = getGroupsForUser(user);
@@ -100,7 +85,7 @@ private String[] getGroupsInternal(String user) throws IOException {
         LOG.info("Error getting groups for " + user + ": " + e.getMessage());
       }
     }
-    return groups;
+    return Arrays.asList(groups);
   }
 
   @Override

hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/JniBasedUnixGroupsMappingWithFallback.java

@@ -20,7 +20,6 @@
 
 import java.io.IOException;
 import java.util.List;
-import java.util.Set;
 
 import org.apache.hadoop.util.NativeCodeLoader;
 import org.apache.hadoop.util.PerformanceAdvisory;
@@ -62,9 +61,4 @@ public void cacheGroupsAdd(List<String> groups) throws IOException {
     impl.cacheGroupsAdd(groups);
   }
 
-  @Override
-  public Set<String> getGroupsSet(String user) throws IOException {
-    return impl.getGroupsSet(user);
-  }
-
 }

hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/JniBasedUnixGroupsNetgroupMappingWithFallback.java

@@ -20,7 +20,6 @@
 
 import java.io.IOException;
 import java.util.List;
-import java.util.Set;
 
 import org.apache.hadoop.util.NativeCodeLoader;
 import org.slf4j.Logger;
@@ -61,9 +60,4 @@ public void cacheGroupsAdd(List<String> groups) throws IOException {
     impl.cacheGroupsAdd(groups);
   }
 
-  @Override
-  public Set<String> getGroupsSet(String user) throws IOException {
-    return impl.getGroupsSet(user);
-  }
-
 }

hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/LdapGroupsMapping.java

@@ -33,7 +33,6 @@
 import java.util.Collections;
 import java.util.Hashtable;
 import java.util.Iterator;
-import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.HashSet;
 import java.util.Collection;
@@ -303,12 +302,12 @@ public class LdapGroupsMapping
   }
 
   private DirContext ctx;
-  private volatile Configuration conf;
+  private Configuration conf;
 
-  private volatile Iterator<String> ldapUrls;
+  private Iterator<String> ldapUrls;
   private String currentLdapUrl;
 
-  private volatile boolean useSsl;
+  private boolean useSsl;
   private String keystore;
   private String keystorePass;
   private String truststore;
@@ -321,21 +320,21 @@ public class LdapGroupsMapping
   private Iterator<BindUserInfo> bindUsers;
   private BindUserInfo currentBindUser;
 
-  private volatile String userbaseDN;
+  private String userbaseDN;
   private String groupbaseDN;
   private String groupSearchFilter;
-  private volatile String userSearchFilter;
+  private String userSearchFilter;
-  private volatile String memberOfAttr;
+  private String memberOfAttr;
   private String groupMemberAttr;
-  private volatile String groupNameAttr;
+  private String groupNameAttr;
-  private volatile int groupHierarchyLevels;
+  private int groupHierarchyLevels;
-  private volatile String posixUidAttr;
+  private String posixUidAttr;
-  private volatile String posixGidAttr;
+  private String posixGidAttr;
   private boolean isPosix;
-  private volatile boolean useOneQuery;
+  private boolean useOneQuery;
   private int numAttempts;
-  private volatile int numAttemptsBeforeFailover;
+  private int numAttemptsBeforeFailover;
-  private volatile String ldapCtxFactoryClassName;
+  private String ldapCtxFactoryClassName;
 
   /**
    * Returns list of groups for a user.
@@ -349,7 +348,38 @@ public class LdapGroupsMapping
    */
   @Override
   public synchronized List<String> getGroups(String user) {
-    return new ArrayList<>(getGroupsSet(user));
+    /*
+     * Normal garbage collection takes care of removing Context instances when
+     * they are no longer in use. Connections used by Context instances being
+     * garbage collected will be closed automatically. So in case connection is
+     * closed and gets CommunicationException, retry some times with new new
+     * DirContext/connection.
+     */
+
+    // Tracks the number of attempts made using the same LDAP server
+    int atemptsBeforeFailover = 1;
+
+    for (int attempt = 1; attempt <= numAttempts; attempt++,
+        atemptsBeforeFailover++) {
+      try {
+        return doGetGroups(user, groupHierarchyLevels);
+      } catch (AuthenticationException e) {
+        switchBindUser(e);
+      } catch (NamingException e) {
+        LOG.warn("Failed to get groups for user {} (attempt={}/{}) using {}. " +
+            "Exception: ", user, attempt, numAttempts, currentLdapUrl, e);
+        LOG.trace("TRACE", e);
+
+        if (failover(atemptsBeforeFailover, numAttemptsBeforeFailover)) {
+          atemptsBeforeFailover = 0;
+        }
+      }
+
+      // Reset ctx so that new DirContext can be created with new connection
+      this.ctx = null;
+    }
+    
+    return Collections.emptyList();
   }
 
   /**
@@ -428,10 +458,10 @@ private NamingEnumeration<SearchResult> lookupPosixGroup(SearchResult result,
    * @return a list of strings representing group names of the user.
    * @throws NamingException if unable to find group names
    */
-  private Set<String> lookupGroup(SearchResult result, DirContext c,
+  private List<String> lookupGroup(SearchResult result, DirContext c,
       int goUpHierarchy)
       throws NamingException {
-    Set<String> groups = new LinkedHashSet<>();
+    List<String> groups = new ArrayList<>();
     Set<String> groupDNs = new HashSet<>();
 
     NamingEnumeration<SearchResult> groupResults;
@@ -454,7 +484,11 @@ private Set<String> lookupGroup(SearchResult result, DirContext c,
         getGroupNames(groupResult, groups, groupDNs, goUpHierarchy > 0);
       }
       if (goUpHierarchy > 0 && !isPosix) {
-        goUpGroupHierarchy(groupDNs, goUpHierarchy, groups);
+        // convert groups to a set to ensure uniqueness
+        Set<String> groupset = new HashSet<>(groups);
+        goUpGroupHierarchy(groupDNs, goUpHierarchy, groupset);
+        // convert set back to list for compatibility
+        groups = new ArrayList<>(groupset);
       }
     }
     return groups;
@@ -473,7 +507,7 @@ private Set<String> lookupGroup(SearchResult result, DirContext c,
    * return an empty string array.
    * @throws NamingException if unable to get group names
    */
-  Set<String> doGetGroups(String user, int goUpHierarchy)
+  List<String> doGetGroups(String user, int goUpHierarchy)
       throws NamingException {
     DirContext c = getDirContext();
 
@@ -484,11 +518,11 @@ Set<String> doGetGroups(String user, int goUpHierarchy)
     if (!results.hasMoreElements()) {
       LOG.debug("doGetGroups({}) returned no groups because the " +
           "user is not found.", user);
-      return Collections.emptySet();
+      return new ArrayList<>();
     }
     SearchResult result = results.nextElement();
 
-    Set<String> groups = Collections.emptySet();
+    List<String> groups = null;
     if (useOneQuery) {
       try {
         /**
@@ -502,7 +536,7 @@ Set<String> doGetGroups(String user, int goUpHierarchy)
               memberOfAttr + "' attribute." +
               "Returned user object: " + result.toString());
         }
-        groups = new LinkedHashSet<>();
+        groups = new ArrayList<>();
         NamingEnumeration groupEnumeration = groupDNAttr.getAll();
         while (groupEnumeration.hasMore()) {
           String groupDN = groupEnumeration.next().toString();
@@ -688,42 +722,6 @@ public void cacheGroupsAdd(List<String> groups) {
     // does nothing in this provider of user to groups mapping
   }
 
-  @Override
-  public Set<String> getGroupsSet(String user) {
-    /*
-     * Normal garbage collection takes care of removing Context instances when
-     * they are no longer in use. Connections used by Context instances being
-     * garbage collected will be closed automatically. So in case connection is
-     * closed and gets CommunicationException, retry some times with new new
-     * DirContext/connection.
-     */
-
-    // Tracks the number of attempts made using the same LDAP server
-    int atemptsBeforeFailover = 1;
-
-    for (int attempt = 1; attempt <= numAttempts; attempt++,
-        atemptsBeforeFailover++) {
-      try {
-        return doGetGroups(user, groupHierarchyLevels);
-      } catch (AuthenticationException e) {
-        switchBindUser(e);
-      } catch (NamingException e) {
-        LOG.warn("Failed to get groups for user {} (attempt={}/{}) using {}. " +
-            "Exception: ", user, attempt, numAttempts, currentLdapUrl, e);
-        LOG.trace("TRACE", e);
-
-        if (failover(atemptsBeforeFailover, numAttemptsBeforeFailover)) {
-          atemptsBeforeFailover = 0;
-        }
-      }
-
-      // Reset ctx so that new DirContext can be created with new connection
-      this.ctx = null;
-    }
-
-    return Collections.emptySet();
-  }
-
   @Override
   public synchronized Configuration getConf() {
     return conf;

hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/NullGroupsMapping.java

@@ -15,10 +15,8 @@
  */
 package org.apache.hadoop.security;
 
-import java.io.IOException;
 import java.util.Collections;
 import java.util.List;
-import java.util.Set;
 
 /**
  * This class provides groups mapping for {@link UserGroupInformation} when the
@@ -33,19 +31,6 @@ public class NullGroupsMapping implements GroupMappingServiceProvider {
   public void cacheGroupsAdd(List<String> groups) {
   }
 
-  /**
-   * Get all various group memberships of a given user.
-   * Returns EMPTY set in case of non-existing user
-   *
-   * @param user User's name
-   * @return set of group memberships of user
-   * @throws IOException
-   */
-  @Override
-  public Set<String> getGroupsSet(String user) throws IOException {
-    return Collections.emptySet();
-  }
-
   /**
    * Returns an empty list.
    * @param user ignored

hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/RuleBasedLdapGroupsMapping.java

@@ -17,6 +17,7 @@
  */
 package org.apache.hadoop.security;
 
+import org.apache.hadoop.thirdparty.com.google.common.annotations.VisibleForTesting;
 import org.apache.hadoop.classification.InterfaceAudience;
 import org.apache.hadoop.classification.InterfaceStability;
 import org.apache.hadoop.conf.Configuration;
@@ -24,9 +25,7 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import java.util.LinkedHashSet;
 import java.util.List;
-import java.util.Set;
 import java.util.stream.Collectors;
 
 /**
@@ -89,18 +88,4 @@ public synchronized List<String> getGroups(String user) {
     }
   }
 
-  public synchronized Set<String> getGroupsSet(String user) {
-    Set<String> groups = super.getGroupsSet(user);
-    switch (rule) {
-    case TO_UPPER:
-      return groups.stream().map(StringUtils::toUpperCase).collect(
-          Collectors.toCollection(LinkedHashSet::new));
-    case TO_LOWER:
-      return groups.stream().map(StringUtils::toLowerCase).collect(
-          Collectors.toCollection(LinkedHashSet::new));
-    case NONE:
-    default:
-      return groups;
-    }
-  }
 }

hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/ShellBasedUnixGroupsMapping.java

@@ -18,11 +18,8 @@
 package org.apache.hadoop.security;
 
 import java.io.IOException;
-import java.util.ArrayList;
+import java.util.LinkedList;
-import java.util.Collections;
-import java.util.LinkedHashSet;
 import java.util.List;
-import java.util.Set;
 import java.util.StringTokenizer;
 import java.util.concurrent.TimeUnit;
 
@@ -56,7 +53,7 @@ public class ShellBasedUnixGroupsMapping extends Configured
 
   private long timeout = CommonConfigurationKeys.
       HADOOP_SECURITY_GROUP_SHELL_COMMAND_TIMEOUT_DEFAULT;
-  private static final Set<String> EMPTY_GROUPS_SET = Collections.emptySet();
+  private static final List<String> EMPTY_GROUPS = new LinkedList<>();
 
   @Override
   public void setConf(Configuration conf) {
@@ -97,7 +94,7 @@ public String toString() {
    */
   @Override
   public List<String> getGroups(String userName) throws IOException {
-    return new ArrayList(getUnixGroups(userName));
+    return getUnixGroups(userName);
   }
 
   /**
@@ -118,11 +115,6 @@ public void cacheGroupsAdd(List<String> groups) throws IOException {
     // does nothing in this provider of user to groups mapping
   }
 
-  @Override
-  public Set<String> getGroupsSet(String userName) throws IOException {
-    return getUnixGroups(userName);
-  }
-
   /**
    * Create a ShellCommandExecutor object using the user's name.
    *
@@ -200,33 +192,44 @@ private boolean handleExecutorTimeout(
    *         group is returned first.
    * @throws IOException if encounter any error when running the command
    */
-  private Set<String> getUnixGroups(String user) throws IOException {
+  private List<String> getUnixGroups(String user) throws IOException {
     ShellCommandExecutor executor = createGroupExecutor(user);
 
-    Set<String> groups;
+    List<String> groups;
     try {
       executor.execute();
       groups = resolveFullGroupNames(executor.getOutput());
     } catch (ExitCodeException e) {
       if (handleExecutorTimeout(executor, user)) {
-        return EMPTY_GROUPS_SET;
+        return EMPTY_GROUPS;
       } else {
         try {
           groups = resolvePartialGroupNames(user, e.getMessage(),
               executor.getOutput());
         } catch (PartialGroupNameException pge) {
           LOG.warn("unable to return groups for user {}", user, pge);
-          return EMPTY_GROUPS_SET;
+          return EMPTY_GROUPS;
         }
       }
     } catch (IOException ioe) {
       if (handleExecutorTimeout(executor, user)) {
-        return EMPTY_GROUPS_SET;
+        return EMPTY_GROUPS;
       } else {
         // If its not an executor timeout, we should let the caller handle it
         throw ioe;
       }
     }
+
+    // remove duplicated primary group
+    if (!Shell.WINDOWS) {
+      for (int i = 1; i < groups.size(); i++) {
+        if (groups.get(i).equals(groups.get(0))) {
+          groups.remove(i);
+          break;
+        }
+      }
+    }
+
     return groups;
   }
 
@@ -239,13 +242,13 @@ private Set<String> getUnixGroups(String user) throws IOException {
    * @return a linked list of group names
    * @throws PartialGroupNameException
    */
-  private Set<String> parsePartialGroupNames(String groupNames,
+  private List<String> parsePartialGroupNames(String groupNames,
       String groupIDs) throws PartialGroupNameException {
     StringTokenizer nameTokenizer =
         new StringTokenizer(groupNames, Shell.TOKEN_SEPARATOR_REGEX);
     StringTokenizer idTokenizer =
         new StringTokenizer(groupIDs, Shell.TOKEN_SEPARATOR_REGEX);
-    Set<String> groups = new LinkedHashSet<>();
+    List<String> groups = new LinkedList<String>();
     while (nameTokenizer.hasMoreTokens()) {
       // check for unresolvable group names.
       if (!idTokenizer.hasMoreTokens()) {
@@ -274,10 +277,10 @@ private Set<String> parsePartialGroupNames(String groupNames,
    * @param userName the user's name
    * @param errMessage error message from the shell command
    * @param groupNames the incomplete list of group names
-   * @return a set of resolved group names
+   * @return a list of resolved group names
    * @throws PartialGroupNameException if the resolution fails or times out
    */
-  private Set<String> resolvePartialGroupNames(String userName,
+  private List<String> resolvePartialGroupNames(String userName,
       String errMessage, String groupNames) throws PartialGroupNameException {
     // Exception may indicate that some group names are not resolvable.
     // Shell-based implementation should tolerate unresolvable groups names,
@@ -319,16 +322,16 @@ private Set<String> resolvePartialGroupNames(String userName,
   }
 
   /**
-   * Split group names into a set.
+   * Split group names into a linked list.
    *
    * @param groupNames a string representing the user's group names
-   * @return a set of group names
+   * @return a linked list of group names
    */
   @VisibleForTesting
-  protected Set<String> resolveFullGroupNames(String groupNames) {
+  protected List<String> resolveFullGroupNames(String groupNames) {
     StringTokenizer tokenizer =
         new StringTokenizer(groupNames, Shell.TOKEN_SEPARATOR_REGEX);
-    Set<String> groups = new LinkedHashSet<>();
+    List<String> groups = new LinkedList<String>();
     while (tokenizer.hasMoreTokens()) {
       groups.add(tokenizer.nextToken());
     }

hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/UserGroupInformation.java

@@ -40,6 +40,7 @@
 import java.security.PrivilegedActionException;
 import java.security.PrivilegedExceptionAction;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.EnumMap;
@@ -1515,8 +1516,8 @@ public UserGroupInformation getRealUser() {
    * map that has the translation of usernames to groups.
    */
   private static class TestingGroups extends Groups {
-    private final Map<String, Set<String>> userToGroupsMapping =
+    private final Map<String, List<String>> userToGroupsMapping = 
-        new HashMap<>();
+      new HashMap<String,List<String>>();
     private Groups underlyingImplementation;
     
     private TestingGroups(Groups underlyingImplementation) {
@@ -1526,22 +1527,17 @@ private TestingGroups(Groups underlyingImplementation) {
     
     @Override
     public List<String> getGroups(String user) throws IOException {
-      return new ArrayList<>(getGroupsSet(user));
+      List<String> result = userToGroupsMapping.get(user);
-    }
+      
-
-    @Override
-    public Set<String> getGroupsSet(String user) throws IOException {
-      Set<String> result = userToGroupsMapping.get(user);
       if (result == null) {
-        result = underlyingImplementation.getGroupsSet(user);
+        result = underlyingImplementation.getGroups(user);
       }
+
       return result;
     }
 
     private void setUserGroups(String user, String[] groups) {
-      Set<String> groupsSet = new LinkedHashSet<>();
+      userToGroupsMapping.put(user, Arrays.asList(groups));
-      Collections.addAll(groupsSet, groups);
-      userToGroupsMapping.put(user, groupsSet);
     }
   }
 
@@ -1600,11 +1596,11 @@ public String getShortUserName() {
   }
 
   public String getPrimaryGroupName() throws IOException {
-    Set<String> groupsSet = getGroupsSet();
+    List<String> groups = getGroups();
-    if (groupsSet.isEmpty()) {
+    if (groups.isEmpty()) {
       throw new IOException("There is no primary group for UGI " + this);
     }
-    return groupsSet.iterator().next();
+    return groups.get(0);
   }
 
   /**
@@ -1717,24 +1713,21 @@ private synchronized Credentials getCredentialsInternal() {
   }
 
   /**
-   * Get the group names for this user. {@link #getGroupsSet()} is less
+   * Get the group names for this user. {@link #getGroups()} is less
    * expensive alternative when checking for a contained element.
    * @return the list of users with the primary group first. If the command
    *    fails, it returns an empty list.
    */
   public String[] getGroupNames() {
-    Collection<String> groupsSet = getGroupsSet();
+    List<String> groups = getGroups();
-    return groupsSet.toArray(new String[groupsSet.size()]);
+    return groups.toArray(new String[groups.size()]);
   }
 
   /**
-   * Get the group names for this user. {@link #getGroupsSet()} is less
+   * Get the group names for this user.
-   * expensive alternative when checking for a contained element.
    * @return the list of users with the primary group first. If the command
    *    fails, it returns an empty list.
-   * @deprecated Use {@link #getGroupsSet()} instead.
    */
-  @Deprecated
   public List<String> getGroups() {
     ensureInitialized();
     try {
@@ -1745,21 +1738,6 @@ public List<String> getGroups() {
     }
   }
 
-  /**
-   * Get the groups names for the user as a Set.
-   * @return the set of users with the primary group first. If the command
-   *     fails, it returns an empty set.
-   */
-  public Set<String> getGroupsSet() {
-    ensureInitialized();
-    try {
-      return groups.getGroupsSet(getShortUserName());
-    } catch (IOException ie) {
-      LOG.debug("Failed to get groups for user {}", getShortUserName(), ie);
-      return Collections.emptySet();
-    }
-  }
-
   /**
    * Return the username.
    */

hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/authorize/AccessControlList.java

@@ -24,7 +24,6 @@
 import java.util.HashSet;
 import java.util.LinkedList;
 import java.util.List;
-import java.util.Set;
 
 import org.apache.hadoop.classification.InterfaceAudience;
 import org.apache.hadoop.classification.InterfaceStability;
@@ -232,9 +231,8 @@ public final boolean isUserInList(UserGroupInformation ugi) {
     if (allAllowed || users.contains(ugi.getShortUserName())) {
       return true;
     } else if (!groups.isEmpty()) {
-      Set<String> ugiGroups = ugi.getGroupsSet();
+      for (String group : ugi.getGroups()) {
-      for (String group : groups) {
+        if (groups.contains(group)) {
-        if (ugiGroups.contains(group)) {
           return true;
         }
       }

hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/http/TestHttpServer.java

@@ -62,10 +62,8 @@
 import java.util.Arrays;
 import java.util.Enumeration;
 import java.util.HashMap;
-import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
-import java.util.Set;
 import java.util.SortedSet;
 import java.util.TreeSet;
 import java.util.concurrent.CountDownLatch;
@@ -412,13 +410,6 @@ static void clearMapping() {
     public List<String> getGroups(String user) throws IOException {
       return mapping.get(user);
     }
-
-    @Override
-    public Set<String> getGroupsSet(String user) throws IOException {
-      Set<String> result = new HashSet();
-      result.addAll(mapping.get(user));
-      return result;
-    }
   }
 
   /**

hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestCompositeGroupMapping.java

@@ -22,9 +22,7 @@
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Arrays;
-import java.util.HashSet;
 import java.util.List;
-import java.util.Set;
 
 import org.apache.hadoop.conf.Configurable;
 import org.apache.hadoop.conf.Configuration;
@@ -89,22 +87,13 @@ public void cacheGroupsRefresh() throws IOException {
     public void cacheGroupsAdd(List<String> groups) throws IOException {
       
     }
-
+    
     protected List<String> toList(String group) {
       if (group != null) {
         return Arrays.asList(new String[] {group});
       }
       return new ArrayList<String>();
     }
-
-    protected Set<String> toSet(String group) {
-      if (group != null) {
-        Set<String> result = new HashSet<>();
-        result.add(group);
-        return result;
-      }
-      return new HashSet<String>();
-    }
     
     protected void checkTestConf(String expectedValue) {
       String configValue = getConf().get(PROVIDER_SPECIFIC_CONF_KEY);
@@ -117,49 +106,32 @@ protected void checkTestConf(String expectedValue) {
   private static class UserProvider extends GroupMappingProviderBase {
     @Override
     public List<String> getGroups(String user) throws IOException {
-      return toList(getGroupInternal(user));
-    }
-
-    @Override
-    public Set<String> getGroupsSet(String user) throws IOException {
-      return toSet(getGroupInternal(user));
-    }
-
-    private String getGroupInternal(String user) throws IOException {
       checkTestConf(PROVIDER_SPECIFIC_CONF_VALUE_FOR_USER);
-
+      
       String group = null;
       if (user.equals(john.name)) {
         group = john.group;
       } else if (user.equals(jack.name)) {
         group = jack.group;
       }
-      return group;
+      
+      return toList(group);
     }
   }
   
   private static class ClusterProvider extends GroupMappingProviderBase {    
     @Override
     public List<String> getGroups(String user) throws IOException {
-      return toList(getGroupsInternal(user));
-    }
-
-    @Override
-    public Set<String> getGroupsSet(String user) throws IOException {
-      return toSet(getGroupsInternal(user));
-    }
-
-    private String getGroupsInternal(String user) throws IOException {
       checkTestConf(PROVIDER_SPECIFIC_CONF_VALUE_FOR_CLUSTER);
-
+      
       String group = null;
       if (user.equals(hdfs.name)) {
         group = hdfs.group;
       } else if (user.equals(jack.name)) { // jack has another group from clusterProvider
         group = jack.group2;
       }
-      return group;
+      
-
+      return toList(group);
     }
   }
   

hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestGroupsCaching.java

@@ -21,9 +21,9 @@
 
 import java.util.ArrayList;
 import java.util.Arrays;
-import java.util.Collections;
 import java.util.HashSet;
 import java.util.LinkedHashSet;
+import java.util.LinkedList;
 import java.util.List;
 import java.util.Set;
 import java.util.concurrent.CountDownLatch;
@@ -75,7 +75,7 @@ public static class FakeGroupMapping extends ShellBasedUnixGroupsMapping {
     private static volatile CountDownLatch latch = null;
 
     @Override
-    public Set<String> getGroupsSet(String user) throws IOException {
+    public List<String> getGroups(String user) throws IOException {
       TESTLOG.info("Getting groups for " + user);
       delayIfNecessary();
 
@@ -86,14 +86,9 @@ public Set<String> getGroupsSet(String user) throws IOException {
       }
 
       if (blackList.contains(user)) {
-        return Collections.emptySet();
+        return new LinkedList<String>();
       }
-      return new LinkedHashSet<>(allGroups);
+      return new LinkedList<String>(allGroups);
-    }
-
-    @Override
-    public List<String> getGroups(String user) throws IOException {
-      return new ArrayList<>(getGroupsSet(user));
     }
 
     /**
@@ -134,7 +129,7 @@ public static void clearAll() throws IOException {
       TESTLOG.info("Resetting FakeGroupMapping");
       blackList.clear();
       allGroups.clear();
-      resetRequestCount();
+      requestCount = 0;
       getGroupsDelayMs = 0;
       throwException = false;
       latch = null;
@@ -202,12 +197,6 @@ public List<String> getGroups(String user) throws IOException {
       throw new IOException("For test");
     }
 
-    @Override
-    public Set<String> getGroupsSet(String user) throws IOException {
-      requestCount++;
-      throw new IOException("For test");
-    }
-
     public static int getRequestCount() {
       return requestCount;
     }
@@ -561,7 +550,7 @@ public void testExceptionOnBackgroundRefreshHandled() throws Exception {
     FakeGroupMapping.clearBlackList();
 
     // We make an initial request to populate the cache
-    List<String> g1 = groups.getGroups("me");
+    groups.getGroups("me");
 
     // add another group
     groups.cacheGroupsAdd(Arrays.asList("grp3"));

hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestRuleBasedLdapGroupsMapping.java

@@ -24,9 +24,7 @@
 
 import javax.naming.NamingException;
 import java.util.ArrayList;
-import java.util.LinkedHashSet;
 import java.util.List;
-import java.util.Set;
 
 import static org.apache.hadoop.security.RuleBasedLdapGroupsMapping
     .CONVERSION_RULE_KEY;
@@ -42,7 +40,7 @@ public class TestRuleBasedLdapGroupsMapping  {
   public void testGetGroupsToUpper() throws NamingException {
     RuleBasedLdapGroupsMapping groupsMapping = Mockito.spy(
         new RuleBasedLdapGroupsMapping());
-    Set<String> groups = new LinkedHashSet<>();
+    List<String> groups = new ArrayList<>();
     groups.add("group1");
     groups.add("group2");
     Mockito.doReturn(groups).when((LdapGroupsMapping) groupsMapping)
@@ -63,7 +61,7 @@ public void testGetGroupsToUpper() throws NamingException {
   public void testGetGroupsToLower() throws NamingException {
     RuleBasedLdapGroupsMapping groupsMapping = Mockito.spy(
         new RuleBasedLdapGroupsMapping());
-    Set<String> groups = new LinkedHashSet<>();
+    List<String> groups = new ArrayList<>();
     groups.add("GROUP1");
     groups.add("GROUP2");
     Mockito.doReturn(groups).when((LdapGroupsMapping) groupsMapping)
@@ -84,7 +82,7 @@ public void testGetGroupsToLower() throws NamingException {
   public void testGetGroupsInvalidRule() throws NamingException {
     RuleBasedLdapGroupsMapping groupsMapping = Mockito.spy(
         new RuleBasedLdapGroupsMapping());
-    Set<String> groups = new LinkedHashSet<>();
+    List<String> groups = new ArrayList<>();
     groups.add("group1");
     groups.add("GROUP2");
     Mockito.doReturn(groups).when((LdapGroupsMapping) groupsMapping)
@@ -95,7 +93,7 @@ public void testGetGroupsInvalidRule() throws NamingException {
     conf.set(CONVERSION_RULE_KEY, "none");
     groupsMapping.setConf(conf);
 
-    Assert.assertEquals(groups, groupsMapping.getGroupsSet("admin"));
+    Assert.assertEquals(groups, groupsMapping.getGroups("admin"));
   }
 
 }

hadoop-hdfs-project/hadoop-hdfs-httpfs/src/main/java/org/apache/hadoop/fs/http/server/HttpFSServer.java

@@ -96,7 +96,6 @@
 import java.util.EnumSet;
 import java.util.List;
 import java.util.Map;
-import java.util.Set;
 
 /**
  * Main class of HttpFSServer server.
@@ -325,7 +324,7 @@ public InputStream run() throws Exception {
     case INSTRUMENTATION: {
       enforceRootPath(op.value(), path);
       Groups groups = HttpFSServerWebApp.get().get(Groups.class);
-      Set<String> userGroups = groups.getGroupsSet(user.getShortUserName());
+      List<String> userGroups = groups.getGroups(user.getShortUserName());
       if (!userGroups.contains(HttpFSServerWebApp.get().getAdminGroup())) {
         throw new AccessControlException(
             "User not in HttpFSServer admin group");

hadoop-hdfs-project/hadoop-hdfs-httpfs/src/main/java/org/apache/hadoop/lib/service/Groups.java

@@ -22,13 +22,10 @@
 
 import java.io.IOException;
 import java.util.List;
-import java.util.Set;
 
 @InterfaceAudience.Private
 public interface Groups {
 
   public List<String> getGroups(String user) throws IOException;
 
-  Set<String> getGroupsSet(String user) throws IOException;
-
 }

hadoop-hdfs-project/hadoop-hdfs-httpfs/src/main/java/org/apache/hadoop/lib/service/security/GroupsService.java

@@ -27,7 +27,6 @@
 
 import java.io.IOException;
 import java.util.List;
-import java.util.Set;
 
 @InterfaceAudience.Private
 public class GroupsService extends BaseService implements Groups {
@@ -51,18 +50,9 @@ public Class getInterface() {
     return Groups.class;
   }
 
-  /**
-   * @deprecated use {@link #getGroupsSet(String user)}
-   */
-  @Deprecated
   @Override
   public List<String> getGroups(String user) throws IOException {
     return hGroups.getGroups(user);
   }
 
-  @Override
-  public Set<String> getGroupsSet(String user) throws IOException {
-    return hGroups.getGroupsSet(user);
-  }
-
 }

hadoop-hdfs-project/hadoop-hdfs-httpfs/src/test/java/org/apache/hadoop/fs/http/server/TestHttpFSServer.java

@@ -61,11 +61,9 @@
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.HashMap;
-import java.util.HashSet;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
-import java.util.Set;
 
 import org.apache.commons.io.IOUtils;
 import org.apache.hadoop.conf.Configuration;
@@ -191,11 +189,6 @@ public List<String> getGroups(String user) throws IOException {
       return Arrays.asList(HadoopUsersConfTestHelper.getHadoopUserGroups(user));
     }
 
-    @Override
-    public Set<String> getGroupsSet(String user) throws IOException {
-      return new HashSet<>(getGroups(user));
-    }
-
   }
 
   private Configuration createHttpFSConf(boolean addDelegationTokenAuthHandler,

hadoop-hdfs-project/hadoop-hdfs-httpfs/src/test/java/org/apache/hadoop/lib/service/security/DummyGroupMapping.java

@@ -21,9 +21,7 @@
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
-import java.util.Set;
 
-import org.apache.hadoop.thirdparty.com.google.common.collect.Sets;
 import org.apache.hadoop.security.GroupMappingServiceProvider;
 import org.apache.hadoop.test.HadoopUsersConfTestHelper;
 
@@ -49,17 +47,4 @@ public void cacheGroupsRefresh() throws IOException {
   @Override
   public void cacheGroupsAdd(List<String> groups) throws IOException {
   }
-
-  @Override
-  public Set<String> getGroupsSet(String user) throws IOException {
-    if (user.equals("root")) {
-      return Sets.newHashSet("admin");
-    } else if (user.equals("nobody")) {
-      return Sets.newHashSet("nobody");
-    } else {
-      String[] groups = HadoopUsersConfTestHelper.getHadoopUserGroups(user);
-      return (groups != null) ? Sets.newHashSet(groups) :
-          Collections.emptySet();
-    }
-  }
 }

hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/RouterPermissionChecker.java

@@ -18,6 +18,8 @@
 package org.apache.hadoop.hdfs.server.federation.router;
 
 import java.io.IOException;
+import java.util.Arrays;
+import java.util.List;
 
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -124,7 +126,8 @@ public void checkSuperuserPrivilege() throws  AccessControlException {
     }
 
     // Is the user a member of the super group?
-    if (ugi.getGroupsSet().contains(superGroup)) {
+    List<String> groups = Arrays.asList(ugi.getGroupNames());
+    if (groups.contains(superGroup)) {
       return;
     }
 

hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/store/records/MountTable.java

@@ -149,7 +149,7 @@ public static MountTable newInstance(final String src,
     // Set permission fields
     UserGroupInformation ugi = NameNode.getRemoteUser();
     record.setOwnerName(ugi.getShortUserName());
-    String group = ugi.getGroupsSet().isEmpty() ? ugi.getShortUserName()
+    String group = ugi.getGroups().isEmpty() ? ugi.getShortUserName()
         : ugi.getPrimaryGroupName();
     record.setGroupName(group);
     record.setMode(new FsPermission(

hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/router/TestRouterRefreshSuperUserGroupsConfiguration.java

@@ -45,7 +45,6 @@
 import java.net.URLDecoder;
 import java.util.ArrayList;
 import java.util.Arrays;
-import java.util.LinkedHashSet;
 
 import static org.junit.Assert.assertEquals;
 import static org.mockito.Mockito.mock;
@@ -136,8 +135,6 @@ private void testRefreshSuperUserGroupsConfigurationInternal(
     when(ugi.getRealUser()).thenReturn(impersonator);
     when(ugi.getUserName()).thenReturn("victim");
     when(ugi.getGroups()).thenReturn(Arrays.asList("groupVictim"));
-    when(ugi.getGroupsSet()).thenReturn(new LinkedHashSet<>(Arrays.asList(
-        "groupVictim")));
 
     // Exception should be thrown before applying config
     LambdaTestUtils.intercept(

hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/router/TestRouterUserMappings.java

@@ -18,7 +18,6 @@
 
 package org.apache.hadoop.hdfs.server.federation.router;
 
-import org.apache.hadoop.thirdparty.com.google.common.collect.Sets;
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.fs.CommonConfigurationKeys;
 import org.apache.hadoop.fs.FileSystem;
@@ -57,9 +56,7 @@
 import java.net.URL;
 import java.net.URLDecoder;
 import java.util.ArrayList;
-import java.util.LinkedHashSet;
 import java.util.List;
-import java.util.Set;
 import java.util.concurrent.TimeUnit;
 
 import static org.junit.Assert.assertArrayEquals;
@@ -114,16 +111,6 @@ public void cacheGroupsRefresh() throws IOException {
     @Override
     public void cacheGroupsAdd(List<String> groups) throws IOException {
     }
-
-    @Override
-    public Set<String> getGroupsSet(String user) throws IOException {
-      LOG.info("Getting groups in MockUnixGroupsMapping");
-      String g1 = user + (10 * i + 1);
-      String g2 = user + (10 * i + 2);
-      Set<String> s = Sets.newHashSet(g1, g2);
-      i++;
-      return s;
-    }
   }
 
   @Before
@@ -204,10 +191,6 @@ private void testRefreshSuperUserGroupsConfigurationInternal(
     final List<String> groupNames2 = new ArrayList<>();
     groupNames2.add("gr3");
     groupNames2.add("gr4");
-    final Set<String> groupNamesSet1 = new LinkedHashSet<>();
-    groupNamesSet1.addAll(groupNames1);
-    final Set<String> groupNamesSet2 = new LinkedHashSet<>();
-    groupNamesSet2.addAll(groupNames2);
 
     //keys in conf
     String userKeyGroups = DefaultImpersonationProvider.getTestProvider().
@@ -239,8 +222,6 @@ private void testRefreshSuperUserGroupsConfigurationInternal(
     // set groups for users
     when(ugi1.getGroups()).thenReturn(groupNames1);
     when(ugi2.getGroups()).thenReturn(groupNames2);
-    when(ugi1.getGroupsSet()).thenReturn(groupNamesSet1);
-    when(ugi2.getGroupsSet()).thenReturn(groupNamesSet2);
 
     // check before refresh
     LambdaTestUtils.intercept(AuthorizationException.class,

hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataNode.java

@@ -1082,7 +1082,8 @@ private void checkSuperuserPrivilege() throws IOException, AccessControlExceptio
     }
 
     // Is the user a member of the super group?
-    if (callerUgi.getGroupsSet().contains(supergroup)) {
+    List<String> groups = Arrays.asList(callerUgi.getGroupNames());
+    if (groups.contains(supergroup)) {
       return;
     }
     // Not a superuser.

hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSPermissionChecker.java

@@ -103,7 +103,7 @@ protected FSPermissionChecker(String fsOwner, String supergroup,
     this.fsOwner = fsOwner;
     this.supergroup = supergroup;
     this.callerUgi = callerUgi;
-    this.groups = callerUgi.getGroupsSet();
+    this.groups = callerUgi.getGroups();
     user = callerUgi.getShortUserName();
     isSuper = user.equals(fsOwner) || groups.contains(supergroup);
     this.attributeProvider = attributeProvider;

hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/security/TestRefreshUserMappings.java

@@ -34,11 +34,8 @@
 import java.net.URL;
 import java.net.URLDecoder;
 import java.util.ArrayList;
-import java.util.LinkedHashSet;
 import java.util.List;
-import java.util.Set;
 
-import org.apache.hadoop.thirdparty.com.google.common.collect.Sets;
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.fs.FileSystem;
 import org.apache.hadoop.fs.Path;
@@ -87,16 +84,6 @@ public void cacheGroupsRefresh() throws IOException {
     @Override
     public void cacheGroupsAdd(List<String> groups) throws IOException {
     }
-
-    @Override
-    public Set<String> getGroupsSet(String user) {
-      LOG.info("Getting groups in MockUnixGroupsMapping");
-      String g1 = user + (10 * i + 1);
-      String g2 = user + (10 * i + 2);
-      Set<String> s = Sets.newHashSet(g1, g2);
-      i++;
-      return s;
-    }
   }
   
   @Before
@@ -209,8 +196,6 @@ public void testRefreshSuperUserGroupsConfiguration() throws Exception {
     // set groups for users
     when(ugi1.getGroups()).thenReturn(groupNames1);
     when(ugi2.getGroups()).thenReturn(groupNames2);
-    when(ugi1.getGroupsSet()).thenReturn(new LinkedHashSet<>(groupNames1));
-    when(ugi2.getGroupsSet()).thenReturn(new LinkedHashSet<>(groupNames2));
 
 
     // check before

hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-hs/src/test/java/org/apache/hadoop/mapreduce/v2/hs/server/TestHSAdminServer.java

@@ -26,9 +26,7 @@
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
-import java.util.LinkedHashSet;
 import java.util.List;
-import java.util.Set;
 
 import org.apache.hadoop.HadoopIllegalArgumentException;
 import org.apache.hadoop.conf.Configuration;
@@ -58,7 +56,6 @@
 
 import org.apache.hadoop.security.authorize.AuthorizationException;
 import org.apache.hadoop.yarn.logaggregation.AggregatedLogDeletionService;
-import org.mockito.internal.util.collections.Sets;
 
 @RunWith(Parameterized.class)
 public class TestHSAdminServer {
@@ -94,15 +91,6 @@ public void cacheGroupsRefresh() throws IOException {
     @Override
     public void cacheGroupsAdd(List<String> groups) throws IOException {
     }
-
-    @Override
-    public Set<String> getGroupsSet(String user) throws IOException {
-      Set<String> result = new LinkedHashSet<>();
-      result.add(user + (10 * i + 1));
-      result.add(user + (10 * i +2));
-      i++;
-      return result;
-    }
   }
 
   @Parameters
@@ -201,9 +189,6 @@ public void testRefreshSuperUserGroups() throws Exception {
     when(superUser.getUserName()).thenReturn("superuser");
     when(ugi.getGroups())
         .thenReturn(Arrays.asList(new String[] { "group3" }));
-    when(ugi.getGroupsSet())
-        .thenReturn(Sets.newSet("group3"));
-
     when(ugi.getUserName()).thenReturn("regularUser");
 
     // Set super user groups not to include groups of regularUser

hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-hs/src/test/java/org/apache/hadoop/mapreduce/v2/hs/webapp/TestHsWebServicesAcls.java

@@ -28,7 +28,6 @@
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
-import java.util.Set;
 
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
@@ -277,11 +276,6 @@ public void cacheGroupsRefresh() throws IOException {
     @Override
     public void cacheGroupsAdd(List<String> groups) throws IOException {
     }
-
-    @Override
-    public Set<String> getGroupsSet(String user) throws IOException {
-      return Collections.emptySet();
-    }
   }
 
   private static class MockJobForAcls implements Job {

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/linux/resources/NetworkTagMappingJsonManager.java

@@ -86,7 +86,7 @@ public String getNetworkTagHexID(Container container) {
         container.getUser());
     List<Group> groups = this.networkTagMapping.getGroups();
     for(Group group : groups) {
-      if (userUGI.getGroupsSet().contains(group.getGroupName())) {
+      if (userUGI.getGroups().contains(group.getGroupName())) {
         return group.getNetworkTagID();
       }
     }

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/linux/runtime/JavaSandboxLinuxContainerRuntime.java

@@ -303,9 +303,9 @@ public boolean isRuntimeRequested(Map<String, String> env) {
   private static List<String> getGroupPolicyFiles(Configuration conf,
       String user) throws ContainerExecutionException {
     Groups groups = Groups.getUserToGroupsMappingService(conf);
-    Set<String> userGroups;
+    List<String> userGroups;
     try {
-      userGroups = groups.getGroupsSet(user);
+      userGroups = groups.getGroups(user);
     } catch (IOException e) {
       throw new ContainerExecutionException("Container user does not exist");
     }
@@ -330,11 +330,11 @@ private boolean isSandboxContainerWhitelisted(String username,
     String whitelistGroup = configuration.get(
         YarnConfiguration.YARN_CONTAINER_SANDBOX_WHITELIST_GROUP);
     Groups groups = Groups.getUserToGroupsMappingService(configuration);
-    Set<String> userGroups;
+    List<String> userGroups;
     boolean isWhitelisted = false;
 
     try {
-      userGroups = groups.getGroupsSet(username);
+      userGroups = groups.getGroups(username);
     } catch (IOException e) {
       throw new ContainerExecutionException("Container user does not exist");
     }

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/placement/PrimaryGroupPlacementRule.java

@@ -30,7 +30,7 @@
 import org.slf4j.LoggerFactory;
 
 import java.io.IOException;
-import java.util.Set;
+import java.util.List;
 
 import static org.apache.hadoop.yarn.server.resourcemanager.placement.FairQueuePlacementUtils.DOT;
 import static org.apache.hadoop.yarn.server.resourcemanager.placement.FairQueuePlacementUtils.assureRoot;
@@ -62,19 +62,19 @@ public ApplicationPlacementContext getPlacementForApp(
 
     // All users should have at least one group the primary group. If no groups
     // are returned then there is a real issue.
-    final Set<String> groupSet;
+    final List<String> groupList;
     try {
-      groupSet = groupProvider.getGroupsSet(user);
+      groupList = groupProvider.getGroups(user);
     } catch (IOException ioe) {
       throw new YarnException("Group resolution failed", ioe);
     }
-    if (groupSet.isEmpty()) {
+    if (groupList.isEmpty()) {
       LOG.error("Group placement rule failed: No groups returned for user {}",
           user);
       throw new YarnException("No groups returned for user " + user);
     }
 
-    String cleanGroup = cleanName(groupSet.iterator().next());
+    String cleanGroup = cleanName(groupList.get(0));
     String queueName;
     PlacementRule parentRule = getParentRule();
 

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/placement/SecondaryGroupExistingPlacementRule.java

@@ -30,8 +30,7 @@
 import org.slf4j.LoggerFactory;
 
 import java.io.IOException;
-import java.util.Iterator;
+import java.util.List;
-import java.util.Set;
 
 import static org.apache.hadoop.yarn.server.resourcemanager.placement.FairQueuePlacementUtils.DOT;
 import static org.apache.hadoop.yarn.server.resourcemanager.placement.FairQueuePlacementUtils.assureRoot;
@@ -66,9 +65,9 @@ public ApplicationPlacementContext getPlacementForApp(
 
     // All users should have at least one group the primary group. If no groups
     // are returned then there is a real issue.
-    final Set<String> groupSet;
+    final List<String> groupList;
     try {
-      groupSet = groupProvider.getGroupsSet(user);
+      groupList = groupProvider.getGroups(user);
     } catch (IOException ioe) {
       throw new YarnException("Group resolution failed", ioe);
     }
@@ -91,9 +90,8 @@ public ApplicationPlacementContext getPlacementForApp(
           parentQueue);
     }
     // now check the groups inside the parent
-    Iterator<String> it = groupSet.iterator();
+    for (int i = 1; i < groupList.size(); i++) {
-    while (it.hasNext()) {
+      String group = cleanName(groupList.get(i));
-      String group = cleanName(it.next());
       String queueName =
           parentQueue == null ? assureRoot(group) : parentQueue + DOT + group;
       if (configuredQueue(queueName)) {

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/placement/UserGroupMappingPlacementRule.java

@@ -20,9 +20,7 @@
 
 import java.io.IOException;
 import java.util.ArrayList;
-import java.util.Iterator;
 import java.util.List;
-import java.util.Set;
 
 import org.apache.commons.lang3.StringUtils;
 import org.apache.hadoop.classification.InterfaceAudience.Private;
@@ -76,21 +74,18 @@ public UserGroupMappingPlacementRule(){
   }
 
   private String getPrimaryGroup(String user) throws IOException {
-    return groups.getGroupsSet(user).iterator().next();
+    return groups.getGroups(user).get(0);
   }
 
   private String getSecondaryGroup(String user) throws IOException {
-    Set<String> groupsSet = groups.getGroupsSet(user);
+    List<String> groupsList = groups.getGroups(user);
     String secondaryGroup = null;
     // Traverse all secondary groups (as there could be more than one
     // and position is not guaranteed) and ensure there is queue with
     // the same name
-    Iterator<String> it = groupsSet.iterator();
+    for (int i = 1; i < groupsList.size(); i++) {
-    it.next();
+      if (this.queueManager.getQueue(groupsList.get(i)) != null) {
-    while (it.hasNext()) {
+        secondaryGroup = groupsList.get(i);
-      String group = it.next();
-      if (this.queueManager.getQueue(group) != null) {
-        secondaryGroup = group;
         break;
       }
     }
@@ -185,7 +180,7 @@ private ApplicationPlacementContext getPlacementForUser(String user)
         }
       }
       if (mapping.getType().equals(MappingType.GROUP)) {
-        for (String userGroups : groups.getGroupsSet(user)) {
+        for (String userGroups : groups.getGroups(user)) {
           if (userGroups.equals(mapping.getSource())) {
             if (mapping.getQueue().equals(CURRENT_USER_MAPPING)) {
               if (LOG.isDebugEnabled()) {

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/TestRMAdminService.java

@@ -1459,11 +1459,6 @@ public void cacheGroupsAdd(List<String> groups) throws IOException {
       // Do nothing
     }
 
-    @Override
-    public Set<String> getGroupsSet(String user) throws IOException {
-      return ImmutableSet.copyOf(group);
-    }
-
     public static void updateGroups() {
       group.clear();
       group.add("test_group_D");

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/PeriodGroupsMapping.java

@@ -18,20 +18,17 @@
 
 package org.apache.hadoop.yarn.server.resourcemanager.scheduler.fair;
 
-import org.apache.hadoop.thirdparty.com.google.common.collect.ImmutableSet;
 import org.apache.hadoop.security.GroupMappingServiceProvider;
 
 import java.io.IOException;
 import java.util.Arrays;
 import java.util.List;
-import java.util.Set;
 
 public class PeriodGroupsMapping implements GroupMappingServiceProvider {
   
   @Override
   public List<String> getGroups(String user) {
-    return Arrays.asList(user + ".group", user + "subgroup1",
+    return Arrays.asList(user + ".group", user + "subgroup1", user + "subgroup2");
-        user + "subgroup2");
   }
 
   @Override
@@ -44,9 +41,4 @@ public void cacheGroupsAdd(List<String> groups) throws IOException {
     throw new UnsupportedOperationException();
   }
 
-  @Override
-  public Set<String> getGroupsSet(String user) throws IOException {
-    return ImmutableSet.of(user + ".group", user + "subgroup1",
-        user + "subgroup2");
-  }
 }

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/PrimaryGroupMapping.java

@@ -22,9 +22,7 @@
 
 import java.io.IOException;
 import java.util.Arrays;
-import java.util.Collections;
 import java.util.List;
-import java.util.Set;
 
 /**
  * Group Mapping class used for test cases. Returns only primary group of the
@@ -46,9 +44,4 @@ public void cacheGroupsRefresh() throws IOException {
   public void cacheGroupsAdd(List<String> groups) throws IOException {
     throw new UnsupportedOperationException();
   }
-
-  @Override
-  public Set<String> getGroupsSet(String user) throws IOException {
-    return Collections.singleton(user + "group");
-  }
 }

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/SimpleGroupsMapping.java

@@ -21,9 +21,7 @@
 import java.io.IOException;
 import java.util.Arrays;
 import java.util.List;
-import java.util.Set;
 
-import org.apache.hadoop.thirdparty.com.google.common.collect.ImmutableSet;
 import org.apache.hadoop.security.GroupMappingServiceProvider;
 
 public class SimpleGroupsMapping implements GroupMappingServiceProvider {
@@ -47,10 +45,4 @@ public void cacheGroupsRefresh() throws IOException {
   @Override
   public void cacheGroupsAdd(List<String> groups) throws IOException {
   }
-
-  @Override
-  public Set<String> getGroupsSet(String user) throws IOException {
-    return ImmutableSet.of(user + "group", user + "subgroup1",
-        user + "subgroup2");
-  }
 }