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 =