diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/NetgroupCache.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/NetgroupCache.java index bd9c448da7..4495a66c43 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/NetgroupCache.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/NetgroupCache.java @@ -17,11 +17,11 @@ */ package org.apache.hadoop.security; +import java.util.Collections; +import java.util.HashSet; import java.util.LinkedList; import java.util.List; -import java.util.Map; import java.util.Set; -import java.util.HashSet; import java.util.concurrent.ConcurrentHashMap; import org.apache.hadoop.classification.InterfaceAudience; @@ -36,14 +36,9 @@ @InterfaceAudience.LimitedPrivate({"HDFS", "MapReduce"}) @InterfaceStability.Unstable public class NetgroupCache { - private static boolean netgroupToUsersMapUpdated = true; - private static Map> netgroupToUsersMap = + private static ConcurrentHashMap> userToNetgroupsMap = new ConcurrentHashMap>(); - private static Map> userToNetgroupsMap = - new ConcurrentHashMap>(); - - /** * Get netgroups for a given user * @@ -52,21 +47,11 @@ public class NetgroupCache { */ public static void getNetgroups(final String user, List groups) { - if(netgroupToUsersMapUpdated) { - netgroupToUsersMapUpdated = false; // at the beginning to avoid race - //update userToNetgroupsMap - for(String netgroup : netgroupToUsersMap.keySet()) { - for(String netuser : netgroupToUsersMap.get(netgroup)) { - // add to userToNetgroupsMap - if(!userToNetgroupsMap.containsKey(netuser)) { - userToNetgroupsMap.put(netuser, new HashSet()); - } - userToNetgroupsMap.get(netuser).add(netgroup); - } - } - } - if(userToNetgroupsMap.containsKey(user)) { - groups.addAll(userToNetgroupsMap.get(user)); + Set userGroups = userToNetgroupsMap.get(user); + //ConcurrentHashMap does not allow null values; + //So null value check can be used to check if the key exists + if (userGroups != null) { + groups.addAll(userGroups); } } @@ -76,7 +61,15 @@ public static void getNetgroups(final String user, * @return list of cached groups */ public static List getNetgroupNames() { - return new LinkedList(netgroupToUsersMap.keySet()); + return new LinkedList(getGroups()); + } + + private static Set getGroups() { + Set allGroups = new HashSet (); + for (Set userGroups : userToNetgroupsMap.values()) { + allGroups.addAll(userGroups); + } + return allGroups; } /** @@ -86,14 +79,13 @@ public static List getNetgroupNames() { * @return true if group is cached, false otherwise */ public static boolean isCached(String group) { - return netgroupToUsersMap.containsKey(group); + return getGroups().contains(group); } /** * Clear the cache */ public static void clear() { - netgroupToUsersMap.clear(); userToNetgroupsMap.clear(); } @@ -104,7 +96,20 @@ public static void clear() { * @param users list of users for a given group */ public static void add(String group, List users) { - netgroupToUsersMap.put(group, new HashSet(users)); - netgroupToUsersMapUpdated = true; // at the end to avoid race + for (String user : users) { + Set userGroups = userToNetgroupsMap.get(user); + // ConcurrentHashMap does not allow null values; + // So null value check can be used to check if the key exists + if (userGroups == null) { + //Generate a ConcurrentHashSet (backed by the keyset of the ConcurrentHashMap) + userGroups = + Collections.newSetFromMap(new ConcurrentHashMap()); + Set currentSet = userToNetgroupsMap.putIfAbsent(user, userGroups); + if (currentSet != null) { + userGroups = currentSet; + } + } + userGroups.add(group); + } } }