You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-commits@hadoop.apache.org by cn...@apache.org on 2015/01/13 18:50:03 UTC
[02/50] [abbrv] hadoop git commit: HADOOP-11402. Negative
user-to-group cache entries are never cleared for never-again-accessed users.
Contributed by Varun Saxena.
HADOOP-11402. Negative user-to-group cache entries are never cleared for never-again-accessed users. Contributed by Varun Saxena.
Project: http://git-wip-us.apache.org/repos/asf/hadoop/repo
Commit: http://git-wip-us.apache.org/repos/asf/hadoop/commit/53caeaa1
Tree: http://git-wip-us.apache.org/repos/asf/hadoop/tree/53caeaa1
Diff: http://git-wip-us.apache.org/repos/asf/hadoop/diff/53caeaa1
Branch: refs/heads/HDFS-6994
Commit: 53caeaa16b1450b54e994c77f5d0c8a767b88d57
Parents: 94d342e
Author: Benoy Antony <be...@apache.org>
Authored: Mon Jan 5 15:06:46 2015 -0800
Committer: Benoy Antony <be...@apache.org>
Committed: Mon Jan 5 15:06:46 2015 -0800
----------------------------------------------------------------------
.../java/org/apache/hadoop/security/Groups.java | 36 +++++++++------
.../hadoop/security/TestGroupsCaching.java | 48 ++++++++++++++++++++
2 files changed, 71 insertions(+), 13 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/hadoop/blob/53caeaa1/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/Groups.java
----------------------------------------------------------------------
diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/Groups.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/Groups.java
index f3c5094..9fd39b0 100644
--- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/Groups.java
+++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/Groups.java
@@ -23,12 +23,14 @@ import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
-import java.util.concurrent.ConcurrentHashMap;
+import java.util.Set;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.TimeUnit;
+import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Ticker;
import com.google.common.cache.CacheBuilder;
+import com.google.common.cache.Cache;
import com.google.common.cache.CacheLoader;
import com.google.common.cache.LoadingCache;
import org.apache.hadoop.HadoopIllegalArgumentException;
@@ -60,14 +62,13 @@ public class Groups {
private final GroupMappingServiceProvider impl;
private final LoadingCache<String, List<String>> cache;
- private final ConcurrentHashMap<String, Long> negativeCacheMask =
- new ConcurrentHashMap<String, Long>();
private final Map<String, List<String>> staticUserToGroupsMap =
new HashMap<String, List<String>>();
private final long cacheTimeout;
private final long negativeCacheTimeout;
private final long warningDeltaMs;
private final Timer timer;
+ private Set<String> negativeCache;
public Groups(Configuration conf) {
this(conf, new Timer());
@@ -99,11 +100,24 @@ public class Groups {
.expireAfterWrite(10 * cacheTimeout, TimeUnit.MILLISECONDS)
.build(new GroupCacheLoader());
+ if(negativeCacheTimeout > 0) {
+ Cache<String, Boolean> tempMap = CacheBuilder.newBuilder()
+ .expireAfterWrite(negativeCacheTimeout, TimeUnit.MILLISECONDS)
+ .ticker(new TimerToTickerAdapter(timer))
+ .build();
+ negativeCache = Collections.newSetFromMap(tempMap.asMap());
+ }
+
if(LOG.isDebugEnabled())
LOG.debug("Group mapping impl=" + impl.getClass().getName() +
"; cacheTimeout=" + cacheTimeout + "; warningDeltaMs=" +
warningDeltaMs);
}
+
+ @VisibleForTesting
+ Set<String> getNegativeCache() {
+ return negativeCache;
+ }
/*
* Parse the hadoop.user.group.static.mapping.overrides configuration to
@@ -159,13 +173,8 @@ public class Groups {
// Check the negative cache first
if (isNegativeCacheEnabled()) {
- Long expirationTime = negativeCacheMask.get(user);
- if (expirationTime != null) {
- if (timer.monotonicNow() < expirationTime) {
- throw noGroupsForUser(user);
- } else {
- negativeCacheMask.remove(user, expirationTime);
- }
+ if (negativeCache.contains(user)) {
+ throw noGroupsForUser(user);
}
}
@@ -212,8 +221,7 @@ public class Groups {
if (groups.isEmpty()) {
if (isNegativeCacheEnabled()) {
- long expirationTime = timer.monotonicNow() + negativeCacheTimeout;
- negativeCacheMask.put(user, expirationTime);
+ negativeCache.add(user);
}
// We throw here to prevent Cache from retaining an empty group
@@ -252,7 +260,9 @@ public class Groups {
LOG.warn("Error refreshing groups cache", e);
}
cache.invalidateAll();
- negativeCacheMask.clear();
+ if(isNegativeCacheEnabled()) {
+ negativeCache.clear();
+ }
}
/**
http://git-wip-us.apache.org/repos/asf/hadoop/blob/53caeaa1/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestGroupsCaching.java
----------------------------------------------------------------------
diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestGroupsCaching.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestGroupsCaching.java
index 89e5b2d..3686694 100644
--- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestGroupsCaching.java
+++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestGroupsCaching.java
@@ -454,5 +454,53 @@ public class TestGroupsCaching {
}
assertEquals(startingRequestCount + 1, FakeGroupMapping.getRequestCount());
+ }
+
+ @Test
+ public void testNegativeCacheEntriesExpire() throws Exception {
+ conf.setLong(
+ CommonConfigurationKeys.HADOOP_SECURITY_GROUPS_NEGATIVE_CACHE_SECS, 2);
+ FakeTimer timer = new FakeTimer();
+ // Ensure that stale entries are removed from negative cache every 2 seconds
+ Groups groups = new Groups(conf, timer);
+ groups.cacheGroupsAdd(Arrays.asList(myGroups));
+ groups.refresh();
+ // Add both these users to blacklist so that they
+ // can be added to negative cache
+ FakeGroupMapping.addToBlackList("user1");
+ FakeGroupMapping.addToBlackList("user2");
+
+ // Put user1 in negative cache.
+ try {
+ groups.getGroups("user1");
+ fail("Did not throw IOException : Failed to obtain groups" +
+ " from FakeGroupMapping.");
+ } catch (IOException e) {
+ GenericTestUtils.assertExceptionContains("No groups found for user", e);
+ }
+ // Check if user1 exists in negative cache
+ assertTrue(groups.getNegativeCache().contains("user1"));
+
+ // Advance fake timer
+ timer.advance(1000);
+ // Put user2 in negative cache
+ try {
+ groups.getGroups("user2");
+ fail("Did not throw IOException : Failed to obtain groups" +
+ " from FakeGroupMapping.");
+ } catch (IOException e) {
+ GenericTestUtils.assertExceptionContains("No groups found for user", e);
+ }
+ // Check if user2 exists in negative cache
+ assertTrue(groups.getNegativeCache().contains("user2"));
+
+ // Advance timer. Only user2 should be present in negative cache.
+ timer.advance(1100);
+ assertFalse(groups.getNegativeCache().contains("user1"));
+ assertTrue(groups.getNegativeCache().contains("user2"));
+
+ // Advance timer. Even user2 should not be present in negative cache.
+ timer.advance(1000);
+ assertFalse(groups.getNegativeCache().contains("user2"));
}
}