Make index synchronization configurable per index type
Allow configuring which index types should be synchronized across sites
in a multi-primary setup. Supported types are: change, account, group,
and project.
The `index.synchronize` setting accepts a list of types:
- `change-index`
- `account-index`
- `group-index`
- `project-index`
To disable synchronization entirely set to `false` and
to enable all set to `true`.
This enables finer-grained control on pull-replication and
multi-site indexing mechanism.
For example it will be possible to disable redundant change
indexing retries on fetch-ref-replicated when using pull-replication
in a multi-primary Gerrit setup.
Bug: Issue 425113249
Change-Id: I15fbf51e9814364cf624f251544164ec340ac225
diff --git a/src/main/java/com/googlesource/gerrit/plugins/multisite/Configuration.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/Configuration.java
index 7d76807..846192f 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/multisite/Configuration.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/multisite/Configuration.java
@@ -29,14 +29,27 @@
import com.google.inject.Inject;
import com.google.inject.Singleton;
import com.google.inject.spi.Message;
+import com.googlesource.gerrit.plugins.multisite.forwarder.ForwardedIndexAccountHandler;
+import com.googlesource.gerrit.plugins.multisite.forwarder.ForwardedIndexChangeHandler;
+import com.googlesource.gerrit.plugins.multisite.forwarder.ForwardedIndexGroupHandler;
+import com.googlesource.gerrit.plugins.multisite.forwarder.ForwardedIndexProjectHandler;
+import com.googlesource.gerrit.plugins.multisite.forwarder.ForwardedIndexingHandler;
+import com.googlesource.gerrit.plugins.multisite.forwarder.events.AccountIndexEvent;
+import com.googlesource.gerrit.plugins.multisite.forwarder.events.ChangeIndexEvent;
+import com.googlesource.gerrit.plugins.multisite.forwarder.events.GroupIndexEvent;
+import com.googlesource.gerrit.plugins.multisite.forwarder.events.IndexEvent;
+import com.googlesource.gerrit.plugins.multisite.forwarder.events.ProjectIndexEvent;
import java.io.IOException;
import java.time.Duration;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.List;
+import java.util.Map;
import java.util.Random;
+import java.util.Set;
import java.util.concurrent.TimeUnit;
+import java.util.stream.Collectors;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.storage.file.FileBasedConfig;
@@ -366,11 +379,12 @@
}
}
- public static class Index extends Forwarding {
+ public static class Index {
static final String INDEX_SECTION = "index";
static final String MAX_TRIES_KEY = "maxTries";
static final String RETRY_INTERVAL_KEY = "retryInterval";
static final String SYNCHRONIZE_FORCED_KEY = "synchronizeForced";
+ static final String SYNCHRONIZE_KEY = "synchronize";
static final boolean DEFAULT_SYNCHRONIZE_FORCED = true;
private final int threadPoolSize;
@@ -378,10 +392,39 @@
private final int maxTries;
private final int numStripedLocks;
+ private final Map<String, Class<? extends ForwardedIndexingHandler<?, ? extends IndexEvent>>>
+ synchronize;
private final boolean synchronizeForced;
+ private static final Map<
+ String, Class<? extends ForwardedIndexingHandler<?, ? extends IndexEvent>>>
+ SYNCHRONIZE_ALL_EVENTS_HANDLERS =
+ Map.of(
+ ChangeIndexEvent.TYPE, ForwardedIndexChangeHandler.class,
+ GroupIndexEvent.TYPE, ForwardedIndexGroupHandler.class,
+ AccountIndexEvent.TYPE, ForwardedIndexAccountHandler.class,
+ ProjectIndexEvent.TYPE, ForwardedIndexProjectHandler.class);
+
+ public Map<String, Class<? extends ForwardedIndexingHandler<?, ? extends IndexEvent>>>
+ getSynchronizeIndex(Supplier<Config> cfg) {
+ Set<String> syncIndexTypes =
+ Arrays.stream(cfg.get().getStringList(INDEX_SECTION, null, SYNCHRONIZE_KEY))
+ .map(String::toLowerCase)
+ .collect(Collectors.toSet());
+
+ if (syncIndexTypes.contains("false")) {
+ return Map.of();
+ }
+
+ Map<String, Class<? extends ForwardedIndexingHandler<?, ? extends IndexEvent>>> synchIndex =
+ SYNCHRONIZE_ALL_EVENTS_HANDLERS.entrySet().stream()
+ .filter(entry -> syncIndexTypes.contains(entry.getKey()))
+ .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));
+
+ return synchIndex.isEmpty() ? SYNCHRONIZE_ALL_EVENTS_HANDLERS : synchIndex;
+ }
+
private Index(Supplier<Config> cfg) {
- super(cfg, INDEX_SECTION);
threadPoolSize =
getInt(cfg, INDEX_SECTION, null, THREAD_POOL_SIZE_KEY, DEFAULT_THREAD_POOL_SIZE);
retryInterval =
@@ -391,6 +434,7 @@
getInt(cfg, INDEX_SECTION, null, NUM_STRIPED_LOCKS, DEFAULT_NUM_STRIPED_LOCKS);
synchronizeForced =
getBoolean(cfg, INDEX_SECTION, null, SYNCHRONIZE_FORCED_KEY, DEFAULT_SYNCHRONIZE_FORCED);
+ synchronize = getSynchronizeIndex(cfg);
}
public int threadPoolSize() {
@@ -412,6 +456,11 @@
public boolean synchronizeForced() {
return synchronizeForced;
}
+
+ public Map<String, Class<? extends ForwardedIndexingHandler<?, ? extends IndexEvent>>>
+ synchronize() {
+ return synchronize;
+ }
}
public static class Broker {
diff --git a/src/main/java/com/googlesource/gerrit/plugins/multisite/Module.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/Module.java
index c161a6d..0e5ae15 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/multisite/Module.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/multisite/Module.java
@@ -54,14 +54,14 @@
if (config.event().synchronize()) {
brokerRouterNeeded = true;
}
- if (config.index().synchronize()) {
+ if (!config.index().synchronize().isEmpty()) {
install(new IndexModule());
brokerRouterNeeded = true;
}
if (brokerRouterNeeded) {
install(new BrokerModule());
- install(new RouterModule());
+ install(new RouterModule(config.index()));
}
}
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/multisite/PluginModule.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/PluginModule.java
index c77b057..dd0b98d 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/multisite/PluginModule.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/multisite/PluginModule.java
@@ -53,7 +53,7 @@
@Override
protected void configure() {
- if (config.index().synchronize()
+ if (!config.index().synchronize().isEmpty()
|| config.cache().synchronize()
|| config.event().synchronize()) {
install(new EventModule(config));
diff --git a/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/router/RouterModule.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/router/RouterModule.java
index 1353e30..b5af428 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/router/RouterModule.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/router/RouterModule.java
@@ -19,21 +19,22 @@
import com.google.gerrit.extensions.registration.DynamicSet;
import com.google.gerrit.lifecycle.LifecycleModule;
import com.google.gerrit.server.events.EventListener;
+import com.google.inject.Inject;
import com.google.inject.Scopes;
import com.google.inject.TypeLiteral;
-import com.googlesource.gerrit.plugins.multisite.forwarder.ForwardedIndexAccountHandler;
-import com.googlesource.gerrit.plugins.multisite.forwarder.ForwardedIndexChangeHandler;
-import com.googlesource.gerrit.plugins.multisite.forwarder.ForwardedIndexGroupHandler;
-import com.googlesource.gerrit.plugins.multisite.forwarder.ForwardedIndexProjectHandler;
+import com.googlesource.gerrit.plugins.multisite.Configuration;
import com.googlesource.gerrit.plugins.multisite.forwarder.ForwardedIndexingHandler;
-import com.googlesource.gerrit.plugins.multisite.forwarder.events.AccountIndexEvent;
-import com.googlesource.gerrit.plugins.multisite.forwarder.events.ChangeIndexEvent;
-import com.googlesource.gerrit.plugins.multisite.forwarder.events.GroupIndexEvent;
import com.googlesource.gerrit.plugins.multisite.forwarder.events.IndexEvent;
-import com.googlesource.gerrit.plugins.multisite.forwarder.events.ProjectIndexEvent;
public class RouterModule extends LifecycleModule {
+ private final Configuration.Index indexConfig;
+
+ @Inject
+ public RouterModule(Configuration.Index indexConfig) {
+ this.indexConfig = indexConfig;
+ }
+
public static final TypeLiteral<ForwardedIndexingHandler<?, ? extends IndexEvent>> INDEX_HANDLER =
new TypeLiteral<>() {};
@@ -44,18 +45,10 @@
DynamicSet.bind(binder(), EventListener.class).to(IndexEventRouter.class);
DynamicMap.mapOf(binder(), INDEX_HANDLER);
- bind(INDEX_HANDLER)
- .annotatedWith(Exports.named(ChangeIndexEvent.TYPE))
- .to(ForwardedIndexChangeHandler.class);
- bind(INDEX_HANDLER)
- .annotatedWith(Exports.named(GroupIndexEvent.TYPE))
- .to(ForwardedIndexGroupHandler.class);
- bind(INDEX_HANDLER)
- .annotatedWith(Exports.named(ProjectIndexEvent.TYPE))
- .to(ForwardedIndexProjectHandler.class);
- bind(INDEX_HANDLER)
- .annotatedWith(Exports.named(AccountIndexEvent.TYPE))
- .to(ForwardedIndexAccountHandler.class);
+ indexConfig
+ .synchronize()
+ .forEach(
+ (type, handler) -> bind(INDEX_HANDLER).annotatedWith(Exports.named(type)).to(handler));
bind(CacheEvictionEventRouter.class).in(Scopes.SINGLETON);
bind(ProjectListUpdateRouter.class).in(Scopes.SINGLETON);
diff --git a/src/main/resources/Documentation/config.md b/src/main/resources/Documentation/config.md
index 75fc55b..95cb2ca 100644
--- a/src/main/resources/Documentation/config.md
+++ b/src/main/resources/Documentation/config.md
@@ -42,10 +42,27 @@
Defaults to 10
```index.synchronize```
-: Whether to synchronize secondary indexes. Set to false when using multi-site
- on Gerrit replicas that do not have an index, or when using an external
- service such as ElasticSearch.
- Defaults to true.
+: Controls which types of index events should be synchronized across sites.
+ It supports the following values:
+
+ - `true`: shortcut for including all index types
+ - `false`: disable index synchronization
+ - A list of specific index types to synchronize:
+ - `change-index`
+ - `account-index`
+ - `group-index`
+ - `project-index`
+
+ Example to synchronize only change and project indexes:
+ ```
+ [index]
+ synchronize = change-index
+ synchronize = project-index
+ ```
+
+> NOTE: All unknown index types are ignored.
+
+Defaults: all index types are enabled.
```index.synchronizeForced```
: Whether to synchronize forced index events. E.g. on-line reindex
diff --git a/src/test/java/com/googlesource/gerrit/plugins/multisite/ConfigurationTest.java b/src/test/java/com/googlesource/gerrit/plugins/multisite/ConfigurationTest.java
index 3a24773..4f0d373 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/multisite/ConfigurationTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/multisite/ConfigurationTest.java
@@ -65,16 +65,16 @@
@Test
public void testGetIndexSynchronize() throws Exception {
- assertThat(getConfiguration().index().synchronize()).isEqualTo(DEFAULT_SYNCHRONIZE);
+ assertThat(getConfiguration().index().synchronize()).isNotEmpty();
globalPluginConfig.setBoolean(INDEX_SECTION, null, SYNCHRONIZE_KEY, false);
- assertThat(getConfiguration().index().synchronize()).isFalse();
+ assertThat(getConfiguration().index().synchronize()).isEmpty();
globalPluginConfig.setBoolean(INDEX_SECTION, null, SYNCHRONIZE_KEY, true);
- assertThat(getConfiguration().index().synchronize()).isTrue();
+ assertThat(getConfiguration().index().synchronize()).isNotEmpty();
globalPluginConfig.setString(INDEX_SECTION, null, SYNCHRONIZE_KEY, INVALID_BOOLEAN);
- assertThat(getConfiguration().index().synchronize()).isTrue();
+ assertThat(getConfiguration().index().synchronize()).isNotEmpty();
}
@Test
diff --git a/src/test/java/com/googlesource/gerrit/plugins/multisite/forwarder/ForwardedCacheEvictionHandlerIT.java b/src/test/java/com/googlesource/gerrit/plugins/multisite/forwarder/ForwardedCacheEvictionHandlerIT.java
index 6ca1a7a..3233baf 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/multisite/forwarder/ForwardedCacheEvictionHandlerIT.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/multisite/forwarder/ForwardedCacheEvictionHandlerIT.java
@@ -35,6 +35,7 @@
import com.google.inject.AbstractModule;
import com.google.inject.Inject;
import com.google.inject.Singleton;
+import com.googlesource.gerrit.plugins.multisite.Configuration;
import com.googlesource.gerrit.plugins.multisite.ExecutorProvider;
import com.googlesource.gerrit.plugins.multisite.cache.CacheModule;
import com.googlesource.gerrit.plugins.multisite.forwarder.events.CacheEvictionEvent;
@@ -72,11 +73,13 @@
private RegistrationHandle cacheEvictionRegistrationHandle;
public static class TestModule extends AbstractModule {
+ @Inject Configuration config;
+
@Override
protected void configure() {
install(new ForwarderModule());
install(new CacheModule(TestForwardingExecutorProvider.class));
- install(new RouterModule());
+ install(new RouterModule(config.index()));
install(new IndexModule());
SharedRefDbConfiguration sharedRefDbConfig =
new SharedRefDbConfiguration(new Config(), "multi-site");
diff --git a/src/test/java/com/googlesource/gerrit/plugins/multisite/http/ReplicationStatusServletIT.java b/src/test/java/com/googlesource/gerrit/plugins/multisite/http/ReplicationStatusServletIT.java
index 80c3d8a..f11ee26 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/multisite/http/ReplicationStatusServletIT.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/multisite/http/ReplicationStatusServletIT.java
@@ -63,12 +63,13 @@
public static class TestModule extends AbstractModule {
@Inject WorkQueue workQueue;
+ @Inject Configuration config;
@Override
protected void configure() {
install(new ForwarderModule());
install(new CacheModule());
- install(new RouterModule());
+ install(new RouterModule(config.index()));
install(new IndexModule());
install(new ReplicationStatusModule(workQueue));
SharedRefDbConfiguration sharedRefDbConfig =
