Do not propagate ldap_groups cache evictions
When Gerrit is using LDAP authentication, the group membership
is continuously loaded because of the expiry of existing entries
and the group membership being loaded during the authentication
phase.
Propagating the ldap_groups cache events would generate a
continuous flooding of evictions causing the second node to be
always bombarded by events and also having its ldap_groups cache
completely empty, which is unfortunate for the condition when the
node needs to become active and thus would benefit from having some
entries already in cache.
Additionally, when nodes are receiving traffic alternatively because
of workload balancing flipping, the generation of cache evictions
upon authentication would result in a cross-traffic of evictions
that would make both ldap_groups caches highly inefficient and
provoke also, in extreme cases, an overload of the LDAP server
and potentially a global outage of both nodes.
For all those reasons, it is not always useful and potentially
dangerous to propagate the ldap_groups cache evictions and thus
should still be configurable but not enabled by default.
Bug: Issue 11294
Change-Id: Ib96bd241da27ee6d3a1868fb3e5f11b27481dd91
diff --git a/src/main/java/com/ericsson/gerrit/plugins/highavailability/cache/CachePatternMatcher.java b/src/main/java/com/ericsson/gerrit/plugins/highavailability/cache/CachePatternMatcher.java
index 41a2b13..cab8f58 100644
--- a/src/main/java/com/ericsson/gerrit/plugins/highavailability/cache/CachePatternMatcher.java
+++ b/src/main/java/com/ericsson/gerrit/plugins/highavailability/cache/CachePatternMatcher.java
@@ -27,13 +27,7 @@
class CachePatternMatcher {
private static final List<String> DEFAULT_PATTERNS =
ImmutableList.of(
- "^accounts.*",
- "^groups.*",
- "ldap_groups",
- "ldap_usernames",
- "projects",
- "sshkeys",
- "web_sessions");
+ "^accounts.*", "^groups.*", "ldap_usernames", "projects", "sshkeys", "web_sessions");
private final Pattern pattern;
diff --git a/src/test/java/com/ericsson/gerrit/plugins/highavailability/cache/CacheEvictionIT.java b/src/test/java/com/ericsson/gerrit/plugins/highavailability/cache/CacheEvictionIT.java
index af2d685..7299238 100644
--- a/src/test/java/com/ericsson/gerrit/plugins/highavailability/cache/CacheEvictionIT.java
+++ b/src/test/java/com/ericsson/gerrit/plugins/highavailability/cache/CacheEvictionIT.java
@@ -25,12 +25,19 @@
import static com.google.common.truth.Truth.assertThat;
import com.github.tomakehurst.wiremock.junit.WireMockRule;
+import com.google.common.cache.LoadingCache;
+import com.google.gerrit.acceptance.GerritConfig;
import com.google.gerrit.acceptance.GlobalPluginConfig;
import com.google.gerrit.acceptance.LightweightPluginDaemonTest;
import com.google.gerrit.acceptance.NoHttpd;
import com.google.gerrit.acceptance.TestPlugin;
import com.google.gerrit.acceptance.UseLocalDisk;
import com.google.gerrit.acceptance.UseSsh;
+import com.google.gerrit.reviewdb.client.AccountGroup;
+import com.google.inject.Inject;
+import com.google.inject.name.Named;
+import java.util.Collections;
+import java.util.Set;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;
import org.apache.http.HttpStatus;
@@ -46,9 +53,16 @@
public class CacheEvictionIT extends LightweightPluginDaemonTest {
private static final int PORT = 18888;
private static final String URL = "http://localhost:" + PORT;
+ private static final String GROUP_CACHE = "ldap_groups";
@Rule public WireMockRule wireMockRule = new WireMockRule(options().port(PORT));
+ @Inject
+ @Named(GROUP_CACHE)
+ private LoadingCache<String, Set<AccountGroup.UUID>> membershipCache;
+
+ private final CountDownLatch expectedRequestLatch = new CountDownLatch(1);
+
@Override
public void setUp() throws Exception {
givenThat(any(anyUrl()).willReturn(aResponse().withStatus(HttpStatus.SC_NO_CONTENT)));
@@ -59,18 +73,49 @@
@UseLocalDisk
@GlobalPluginConfig(pluginName = "high-availability", name = "peerInfo.static.url", value = URL)
@GlobalPluginConfig(pluginName = "high-availability", name = "http.retryInterval", value = "100")
+ @GerritConfig(name = "auth.type", value = "ldap")
public void flushProjectsCacheShouldSendPostForEvictingRemoteCache() throws Exception {
final String flushRequest = "/plugins/high-availability/cache/" + Constants.PROJECTS;
- final CountDownLatch expectedRequestLatch = new CountDownLatch(1);
+
+ expectRestApiCall(flushRequest);
+
+ adminSshSession.exec("gerrit flush-caches --cache " + Constants.PROJECTS);
+ assertThat(waitForEvictionEvents()).isTrue();
+ verify(postRequestedFor(urlEqualTo(flushRequest)));
+ }
+
+ @Test
+ @UseLocalDisk
+ @GlobalPluginConfig(pluginName = "high-availability", name = "peerInfo.static.url", value = URL)
+ @GlobalPluginConfig(pluginName = "high-availability", name = "http.retryInterval", value = "100")
+ @GerritConfig(name = "auth.type", value = "ldap")
+ public void ldapCacheLoadShouldNotSendAnyPostEvictionForLdapGroups() throws Exception {
+ final String flushRequest = "/plugins/high-availability/cache/" + GROUP_CACHE;
+ String username = "username";
+ Set<AccountGroup.UUID> groups = Collections.emptySet();
+
+ expectRestApiCall(flushRequest);
+
+ loadLdapGroupMembers(username, groups);
+ loadLdapGroupMembers(username, groups); // For triggering an eviction
+ assertThat(waitForEvictionEvents()).isFalse();
+ verify(0, postRequestedFor(urlEqualTo(flushRequest)));
+ }
+
+ private void expectRestApiCall(final String flushRequest) {
wireMockRule.addMockServiceRequestListener(
(request, response) -> {
if (request.getAbsoluteUrl().contains(flushRequest)) {
expectedRequestLatch.countDown();
}
});
+ }
- adminSshSession.exec("gerrit flush-caches --cache " + Constants.PROJECTS);
- assertThat(expectedRequestLatch.await(5, TimeUnit.SECONDS)).isTrue();
- verify(postRequestedFor(urlEqualTo(flushRequest)));
+ private boolean waitForEvictionEvents() throws InterruptedException {
+ return expectedRequestLatch.await(5, TimeUnit.SECONDS);
+ }
+
+ private void loadLdapGroupMembers(String username, Set<AccountGroup.UUID> groups) {
+ membershipCache.put(username, groups);
}
}
diff --git a/src/test/java/com/ericsson/gerrit/plugins/highavailability/cache/CachePattenMatcherTest.java b/src/test/java/com/ericsson/gerrit/plugins/highavailability/cache/CachePattenMatcherTest.java
index 35a6ba3..f90a735 100644
--- a/src/test/java/com/ericsson/gerrit/plugins/highavailability/cache/CachePattenMatcherTest.java
+++ b/src/test/java/com/ericsson/gerrit/plugins/highavailability/cache/CachePattenMatcherTest.java
@@ -46,7 +46,6 @@
"groups_byuuid",
"groups_external",
"groups_members",
- "ldap_groups",
"ldap_usernames",
"projects",
"sshkeys",
@@ -67,6 +66,7 @@
"diff_summary",
"git_tags",
"ldap_group_existence",
+ "ldap_groups",
"ldap_groups_byinclude",
"mergeability",
"oauth_tokens",
