Merge branch 'stable-3.12' * stable-3.12: Throw lockFailureException when RefDBLock occurs Add 3rd site in the local test environment Use fanout replication config for the local test environment Sort ref-names before locking them in filterAndLock Manage change deletion performed by pull-replication Change-Id: Id8ac6c552a56e79638a29185f35cd7bd54ca3599
diff --git a/setup_local_env/setup.sh b/setup_local_env/setup.sh index 06fd7c7..3005275 100755 --- a/setup_local_env/setup.sh +++ b/setup_local_env/setup.sh
@@ -16,7 +16,7 @@ SCRIPT_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null 2>&1 && pwd )" -GERRIT_BRANCH=stable-3.12 +GERRIT_BRANCH=master GERRIT_CI=https://gerrit-ci.gerritforge.com/view/Plugins-$GERRIT_BRANCH/job LAST_BUILD=lastSuccessfulBuild/artifact/bazel-bin/plugins
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 1be09aa..846192f 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/multisite/Configuration.java +++ b/src/main/java/com/googlesource/gerrit/plugins/multisite/Configuration.java
@@ -22,20 +22,34 @@ import com.google.common.base.MoreObjects; import com.google.common.base.Strings; import com.google.common.base.Supplier; +import com.google.common.base.Suppliers; import com.google.common.collect.ImmutableList; import com.google.gerrit.server.config.ConfigUtil; import com.google.gerrit.server.config.SitePaths; 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; @@ -83,8 +97,8 @@ private final Config multiSiteConfig; private final Supplier<Duration> replicationLagRefreshInterval; private final Supplier<Boolean> replicationLagEnabled; - private final Supplier<Boolean> pushReplicationFilterEnabled; - private final Supplier<Boolean> pullReplicationFilterEnabled; + private Supplier<Boolean> pushReplicationFilterEnabled; + private Supplier<Boolean> pullReplicationFilterEnabled; private final Supplier<Long> localRefLockTimeoutMsec; @Inject @@ -122,10 +136,10 @@ replicationLagEnabled = memoize( () -> - lazyMultiSiteCfg - .get() - .getBoolean(REF_DATABASE, null, REPLICATION_LAG_ENABLED, true)); - + !pullReplicationFilterEnabled.get() + && lazyMultiSiteCfg + .get() + .getBoolean(REF_DATABASE, null, REPLICATION_LAG_ENABLED, true)); pushReplicationFilterEnabled = memoize( () -> @@ -194,10 +208,18 @@ return pushReplicationFilterEnabled.get(); } - public boolean pullRepllicationFilterEnabled() { + public void disablePushReplicationFilter() { + pushReplicationFilterEnabled = Suppliers.ofInstance(false); + } + + public boolean pullReplicationFilterEnabled() { return pullReplicationFilterEnabled.get(); } + public void disablePullReplicationFilter() { + pullReplicationFilterEnabled = Suppliers.ofInstance(false); + } + public long localRefLockTimeoutMsec() { return localRefLockTimeoutMsec.get(); } @@ -357,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; @@ -369,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 = @@ -382,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() { @@ -403,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 3ced8c6..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)); @@ -86,16 +86,20 @@ ImmutableList.Builder<AbstractModule> filterModulesBuilder = ImmutableList.builder(); if (config.pushReplicationFilterEnabled()) { - bindReplicationFilterClass(PUSH_REPLICATION_FILTER_MODULE, filterModulesBuilder); + if (!bindReplicationFilterClass(PUSH_REPLICATION_FILTER_MODULE, filterModulesBuilder)) { + config.disablePushReplicationFilter(); + } } - if (config.pullRepllicationFilterEnabled()) { - bindReplicationFilterClass(PULL_REPLICATION_FILTER_MODULE, filterModulesBuilder); + if (config.pullReplicationFilterEnabled()) { + if (!bindReplicationFilterClass(PULL_REPLICATION_FILTER_MODULE, filterModulesBuilder)) { + config.disablePullReplicationFilter(); + } } return filterModulesBuilder.build(); } - private void bindReplicationFilterClass( + private boolean bindReplicationFilterClass( String filterClassName, ImmutableList.Builder<AbstractModule> filterModulesBuilder) { try { @SuppressWarnings("unchecked") @@ -106,9 +110,11 @@ parentInjector.createChildInjector(filterModule); filterModulesBuilder.add(filterModule); + return true; } catch (NoClassDefFoundError | ClassNotFoundException e) { log.atWarning().withCause(e).log( "Not loading %s because of missing the associated replication plugin", filterClassName); + return false; } catch (Exception e) { throw new ProvisionException( "Unable to instantiate replication filter " + filterClassName, e);
diff --git a/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/ForwardedIndexAccountHandler.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/ForwardedIndexAccountHandler.java index 5212aa4..6e0aa11 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/ForwardedIndexAccountHandler.java +++ b/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/ForwardedIndexAccountHandler.java
@@ -14,12 +14,15 @@ package com.googlesource.gerrit.plugins.multisite.forwarder; +import static com.googlesource.gerrit.plugins.multisite.forwarder.ForwardedIndexingHandler.Operation.INDEX; + import com.google.gerrit.entities.Account; import com.google.gerrit.server.index.account.AccountIndexer; import com.google.inject.Inject; import com.google.inject.Singleton; import com.googlesource.gerrit.plugins.multisite.Configuration; import com.googlesource.gerrit.plugins.multisite.forwarder.events.AccountIndexEvent; +import com.googlesource.gerrit.plugins.multisite.forwarder.events.IndexEvent; import java.io.IOException; import java.util.HashMap; import java.util.Map; @@ -48,6 +51,14 @@ } @Override + public void handle(IndexEvent sourceEvent) throws IOException { + if (sourceEvent instanceof AccountIndexEvent) { + AccountIndexEvent accountIndexEvent = (AccountIndexEvent) sourceEvent; + indexAsync(Account.id(accountIndexEvent.accountId), INDEX); + } + } + + @Override protected void doIndex(Account.Id id, Optional<AccountIndexEvent> event) { indexer.index(id); log.debug("Account {} successfully indexed", id);
diff --git a/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/ForwardedIndexChangeHandler.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/ForwardedIndexChangeHandler.java index 15f7c13..1f3fd93 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/ForwardedIndexChangeHandler.java +++ b/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/ForwardedIndexChangeHandler.java
@@ -14,6 +14,9 @@ package com.googlesource.gerrit.plugins.multisite.forwarder; +import static com.googlesource.gerrit.plugins.multisite.forwarder.ForwardedIndexingHandler.Operation.DELETE; +import static com.googlesource.gerrit.plugins.multisite.forwarder.ForwardedIndexingHandler.Operation.INDEX; + import com.google.common.base.Splitter; import com.google.gerrit.entities.Change; import com.google.gerrit.server.index.change.ChangeIndexer; @@ -24,9 +27,11 @@ import com.google.inject.Singleton; import com.googlesource.gerrit.plugins.multisite.Configuration; import com.googlesource.gerrit.plugins.multisite.forwarder.events.ChangeIndexEvent; +import com.googlesource.gerrit.plugins.multisite.forwarder.events.IndexEvent; import com.googlesource.gerrit.plugins.multisite.index.ChangeChecker; import com.googlesource.gerrit.plugins.multisite.index.ChangeCheckerImpl; import com.googlesource.gerrit.plugins.multisite.index.ForwardedIndexExecutor; +import java.io.IOException; import java.util.Optional; import java.util.concurrent.ScheduledExecutorService; @@ -55,6 +60,18 @@ } @Override + public void handle(IndexEvent sourceEvent) throws IOException { + if (sourceEvent instanceof ChangeIndexEvent) { + ChangeIndexEvent changeIndexEvent = (ChangeIndexEvent) sourceEvent; + ForwardedIndexingHandler.Operation operation = changeIndexEvent.deleted ? DELETE : INDEX; + index( + changeIndexEvent.projectName + "~" + changeIndexEvent.changeId, + operation, + Optional.of(changeIndexEvent)); + } + } + + @Override protected void doIndex(String id, Optional<ChangeIndexEvent> indexEvent) { scheduleIndexing(id, indexEvent, this::indexIfConsistent); }
diff --git a/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/ForwardedIndexGroupHandler.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/ForwardedIndexGroupHandler.java index d05405f..f1141c9 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/ForwardedIndexGroupHandler.java +++ b/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/ForwardedIndexGroupHandler.java
@@ -14,6 +14,8 @@ package com.googlesource.gerrit.plugins.multisite.forwarder; +import static com.googlesource.gerrit.plugins.multisite.forwarder.ForwardedIndexingHandler.Operation.INDEX; + import com.google.gerrit.entities.AccountGroup; import com.google.gerrit.server.index.group.GroupIndexer; import com.google.gerrit.server.util.OneOffRequestContext; @@ -21,8 +23,10 @@ import com.google.inject.Singleton; import com.googlesource.gerrit.plugins.multisite.Configuration; import com.googlesource.gerrit.plugins.multisite.forwarder.events.GroupIndexEvent; +import com.googlesource.gerrit.plugins.multisite.forwarder.events.IndexEvent; import com.googlesource.gerrit.plugins.multisite.index.ForwardedIndexExecutor; import com.googlesource.gerrit.plugins.multisite.index.GroupChecker; +import java.io.IOException; import java.util.Optional; import java.util.concurrent.ScheduledExecutorService; @@ -51,6 +55,14 @@ } @Override + public void handle(IndexEvent sourceEvent) throws IOException { + if (sourceEvent instanceof GroupIndexEvent) { + GroupIndexEvent groupIndexEvent = (GroupIndexEvent) sourceEvent; + index(groupIndexEvent.groupUUID, INDEX, Optional.of(groupIndexEvent)); + } + } + + @Override protected void doIndex(String uuid, Optional<GroupIndexEvent> event) { scheduleIndexing(uuid, event, this::reindex); }
diff --git a/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/ForwardedIndexProjectHandler.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/ForwardedIndexProjectHandler.java index 2054f10..453d961 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/ForwardedIndexProjectHandler.java +++ b/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/ForwardedIndexProjectHandler.java
@@ -14,15 +14,19 @@ package com.googlesource.gerrit.plugins.multisite.forwarder; +import static com.googlesource.gerrit.plugins.multisite.forwarder.ForwardedIndexingHandler.Operation.INDEX; + import com.google.gerrit.entities.Project; import com.google.gerrit.index.project.ProjectIndexer; import com.google.gerrit.server.util.OneOffRequestContext; import com.google.inject.Inject; import com.google.inject.Singleton; import com.googlesource.gerrit.plugins.multisite.Configuration; +import com.googlesource.gerrit.plugins.multisite.forwarder.events.IndexEvent; import com.googlesource.gerrit.plugins.multisite.forwarder.events.ProjectIndexEvent; import com.googlesource.gerrit.plugins.multisite.index.ForwardedIndexExecutor; import com.googlesource.gerrit.plugins.multisite.index.ProjectChecker; +import java.io.IOException; import java.util.Optional; import java.util.concurrent.ScheduledExecutorService; @@ -51,6 +55,14 @@ } @Override + public void handle(IndexEvent sourceEvent) throws IOException { + if (sourceEvent instanceof ProjectIndexEvent) { + ProjectIndexEvent projectIndexEvent = (ProjectIndexEvent) sourceEvent; + index(projectIndexEvent.projectName, INDEX, Optional.of(projectIndexEvent)); + } + } + + @Override protected void doIndex(String projectName, Optional<ProjectIndexEvent> event) { scheduleIndexing(projectName, event, this::reindex); }
diff --git a/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/ForwardedIndexingHandler.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/ForwardedIndexingHandler.java index de6b836..a6440e4 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/ForwardedIndexingHandler.java +++ b/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/ForwardedIndexingHandler.java
@@ -15,6 +15,7 @@ package com.googlesource.gerrit.plugins.multisite.forwarder; import com.google.common.util.concurrent.Striped; +import com.googlesource.gerrit.plugins.multisite.forwarder.events.IndexEvent; import java.io.IOException; import java.util.Optional; import java.util.concurrent.locks.Lock; @@ -40,6 +41,8 @@ } } + public abstract void handle(IndexEvent sourceEvent) throws IOException; + private final Striped<Lock> idLocks; protected abstract void doIndex(T id, Optional<E> indexEvent);
diff --git a/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/events/AccountIndexEvent.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/events/AccountIndexEvent.java index 065ef7e..31333e7 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/events/AccountIndexEvent.java +++ b/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/events/AccountIndexEvent.java
@@ -17,7 +17,7 @@ import com.google.common.base.Objects; public class AccountIndexEvent extends IndexEvent { - static final String TYPE = "account-index"; + public static final String TYPE = "account-index"; public int accountId;
diff --git a/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/events/ChangeIndexEvent.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/events/ChangeIndexEvent.java index 64fbdfb..7fd3b6c 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/events/ChangeIndexEvent.java +++ b/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/events/ChangeIndexEvent.java
@@ -21,7 +21,7 @@ import java.time.format.DateTimeFormatter; public class ChangeIndexEvent extends IndexEvent { - static final String TYPE = "change-index"; + public static final String TYPE = "change-index"; public String projectName; public int changeId;
diff --git a/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/events/GroupIndexEvent.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/events/GroupIndexEvent.java index 05ac198..63adeb6 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/events/GroupIndexEvent.java +++ b/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/events/GroupIndexEvent.java
@@ -19,7 +19,7 @@ import org.eclipse.jgit.lib.ObjectId; public class GroupIndexEvent extends IndexEvent { - static final String TYPE = "group-index"; + public static final String TYPE = "group-index"; public final String groupUUID; public final ObjectId sha1;
diff --git a/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/events/ProjectIndexEvent.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/events/ProjectIndexEvent.java index 954befb..00cca2e 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/events/ProjectIndexEvent.java +++ b/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/events/ProjectIndexEvent.java
@@ -17,7 +17,7 @@ import com.google.common.base.Objects; public class ProjectIndexEvent extends IndexEvent { - static final String TYPE = "project-index"; + public static final String TYPE = "project-index"; public String projectName;
diff --git a/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/router/IndexEventRouter.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/router/IndexEventRouter.java index a647bce..238db51 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/router/IndexEventRouter.java +++ b/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/router/IndexEventRouter.java
@@ -14,12 +14,13 @@ package com.googlesource.gerrit.plugins.multisite.forwarder.router; -import static com.googlesource.gerrit.plugins.multisite.forwarder.ForwardedIndexingHandler.Operation.DELETE; +import static com.google.gerrit.extensions.registration.PluginName.GERRIT; import static com.googlesource.gerrit.plugins.multisite.forwarder.ForwardedIndexingHandler.Operation.INDEX; import com.google.common.flogger.FluentLogger; import com.google.gerrit.entities.Account; import com.google.gerrit.extensions.events.LifecycleListener; +import com.google.gerrit.extensions.registration.DynamicMap; import com.google.gerrit.server.config.AllUsersName; import com.google.gerrit.server.config.GerritInstanceId; import com.google.gerrit.server.events.Event; @@ -27,15 +28,8 @@ import com.google.gerrit.server.events.RefEvent; import com.google.inject.Inject; 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.util.Optional; import java.util.Set; @@ -45,50 +39,30 @@ private static final FluentLogger logger = FluentLogger.forEnclosingClass(); private final ForwardedIndexAccountHandler indexAccountHandler; - private final ForwardedIndexChangeHandler indexChangeHandler; - private final ForwardedIndexGroupHandler indexGroupHandler; - private final ForwardedIndexProjectHandler indexProjectHandler; private final AllUsersName allUsersName; private final String gerritInstanceId; + private final DynamicMap<ForwardedIndexingHandler<?, ? extends IndexEvent>> indexHandlers; @Inject public IndexEventRouter( ForwardedIndexAccountHandler indexAccountHandler, - ForwardedIndexChangeHandler indexChangeHandler, - ForwardedIndexGroupHandler indexGroupHandler, - ForwardedIndexProjectHandler indexProjectHandler, + DynamicMap<ForwardedIndexingHandler<?, ? extends IndexEvent>> indexHandlers, AllUsersName allUsersName, @GerritInstanceId String gerritInstanceId) { this.indexAccountHandler = indexAccountHandler; - this.indexChangeHandler = indexChangeHandler; - this.indexGroupHandler = indexGroupHandler; - this.indexProjectHandler = indexProjectHandler; + this.indexHandlers = indexHandlers; this.allUsersName = allUsersName; this.gerritInstanceId = gerritInstanceId; } @Override public void route(IndexEvent sourceEvent) throws IOException { - if (sourceEvent instanceof ChangeIndexEvent) { - ChangeIndexEvent changeIndexEvent = (ChangeIndexEvent) sourceEvent; - ForwardedIndexingHandler.Operation operation = changeIndexEvent.deleted ? DELETE : INDEX; - indexChangeHandler.index( - changeIndexEvent.projectName + "~" + changeIndexEvent.changeId, - operation, - Optional.of(changeIndexEvent)); - } else if (sourceEvent instanceof AccountIndexEvent) { - AccountIndexEvent accountIndexEvent = (AccountIndexEvent) sourceEvent; - indexAccountHandler.indexAsync(Account.id(accountIndexEvent.accountId), INDEX); - } else if (sourceEvent instanceof GroupIndexEvent) { - GroupIndexEvent groupIndexEvent = (GroupIndexEvent) sourceEvent; - indexGroupHandler.index(groupIndexEvent.groupUUID, INDEX, Optional.of(groupIndexEvent)); - } else if (sourceEvent instanceof ProjectIndexEvent) { - ProjectIndexEvent projectIndexEvent = (ProjectIndexEvent) sourceEvent; - indexProjectHandler.index( - projectIndexEvent.projectName, INDEX, Optional.of(projectIndexEvent)); + ForwardedIndexingHandler<?, ? extends IndexEvent> handler = + indexHandlers.get(GERRIT, sourceEvent.getType()); + if (handler != null) { + handler.handle(sourceEvent); } else { - throw new UnsupportedOperationException( - String.format("Cannot route event %s", sourceEvent.getType())); + logger.atInfo().log("No registered handlers to route event %s", sourceEvent.getType()); } }
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 2193034..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
@@ -14,17 +14,41 @@ package com.googlesource.gerrit.plugins.multisite.forwarder.router; +import com.google.gerrit.extensions.annotations.Exports; +import com.google.gerrit.extensions.registration.DynamicMap; 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.Configuration; +import com.googlesource.gerrit.plugins.multisite.forwarder.ForwardedIndexingHandler; +import com.googlesource.gerrit.plugins.multisite.forwarder.events.IndexEvent; 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<>() {}; + @Override protected void configure() { bind(IndexEventRouter.class).in(Scopes.SINGLETON); listener().to(IndexEventRouter.class).in(Scopes.SINGLETON); DynamicSet.bind(binder(), EventListener.class).to(IndexEventRouter.class); + DynamicMap.mapOf(binder(), INDEX_HANDLER); + + 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/java/com/googlesource/gerrit/plugins/multisite/validation/ValidationModule.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/ValidationModule.java index 958f265..1971c35 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/ValidationModule.java +++ b/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/ValidationModule.java
@@ -26,9 +26,6 @@ import com.gerritforge.gerrit.globalrefdb.validation.SharedRefDbRefUpdate; import com.gerritforge.gerrit.globalrefdb.validation.SharedRefDbRepository; import com.gerritforge.gerrit.globalrefdb.validation.SharedRefLogger; -import com.gerritforge.gerrit.globalrefdb.validation.dfsrefdb.CustomSharedRefEnforcementByProject; -import com.gerritforge.gerrit.globalrefdb.validation.dfsrefdb.DefaultSharedRefEnforcement; -import com.gerritforge.gerrit.globalrefdb.validation.dfsrefdb.SharedRefEnforcement; import com.google.common.collect.ImmutableSet; import com.google.gerrit.extensions.config.FactoryModule; import com.google.gerrit.extensions.registration.DynamicSet; @@ -69,14 +66,6 @@ ProjectVersionRefUpdate.MULTI_SITE_VERSIONING_VALUE_REF)); install(new RepositoryManagerModule(repoConfig)); - if (cfg.getSharedRefDbConfiguration().getSharedRefDb().getEnforcementRules().isEmpty()) { - bind(SharedRefEnforcement.class).to(DefaultSharedRefEnforcement.class).in(Scopes.SINGLETON); - } else { - bind(SharedRefEnforcement.class) - .to(CustomSharedRefEnforcementByProject.class) - .in(Scopes.SINGLETON); - } - DynamicSet.bind(binder(), ExceptionHook.class).to(SharedRefDbExceptionHook.class); } }
diff --git a/src/main/resources/Documentation/config.md b/src/main/resources/Documentation/config.md index ed72405..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 @@ -95,12 +112,12 @@ : Enable the use of a shared ref-database Defaults: true -```ref-database.pushReplicationFilterClassEnabled``` +```ref-database.pushReplicationFilterEnabled``` : Enable the filtering of push replication events checking their up-to-date status with the global-refdb. Defaults: true -```ref-database.pullReplicationFilterClassEnabled``` +```ref-database.pullReplicationFilterEnabled``` : Enable the filtering of pull replication events checking their up-to-date status with the global-refdb. Defaults: true @@ -115,7 +132,8 @@ updating the `refs/multi-site/version/*` to the _epoch_ timestamp in milliseconds. Please note that the `replication-lag` REST-API is also disabled if this setting is false. - Defaults: true + Defaults: true unless `pullReplicationFilterEnabled` is enabled and the pull-replication plugin + is available, in which case replicationLag is always disabled. ```ref-database.replicationLagRefreshInterval``` : Enable the auto-refresh of the metrics to trace the auto-replication
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/consumer/SubscriberMetricsTest.java b/src/test/java/com/googlesource/gerrit/plugins/multisite/consumer/SubscriberMetricsTest.java index 6a03a6c..191d7fd 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/multisite/consumer/SubscriberMetricsTest.java +++ b/src/test/java/com/googlesource/gerrit/plugins/multisite/consumer/SubscriberMetricsTest.java
@@ -57,6 +57,8 @@ @Before public void setup() throws Exception { + Config multiSiteConfig = new Config(); + multiSiteConfig.setBoolean("ref-database", null, "pullReplicationFilterEnabled", false); replicationStatus = new ReplicationStatus( CacheBuilder.newBuilder().build(), @@ -64,7 +66,8 @@ verLogger, projectCache, Executors.newScheduledThreadPool(1), - new com.googlesource.gerrit.plugins.multisite.Configuration(new Config(), new Config()), + new com.googlesource.gerrit.plugins.multisite.Configuration( + multiSiteConfig, new Config()), new DisabledMetricMaker()); metrics = new SubscriberMetrics(metricMaker, replicationStatus); }
diff --git a/src/test/java/com/googlesource/gerrit/plugins/multisite/event/IndexEventRouterTest.java b/src/test/java/com/googlesource/gerrit/plugins/multisite/event/IndexEventRouterTest.java index 4ffdbbc..11600f9 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/multisite/event/IndexEventRouterTest.java +++ b/src/test/java/com/googlesource/gerrit/plugins/multisite/event/IndexEventRouterTest.java
@@ -14,12 +14,14 @@ package com.googlesource.gerrit.plugins.multisite.event; -import static com.google.gerrit.testing.GerritJUnit.assertThrows; +import static com.google.gerrit.extensions.registration.PluginName.GERRIT; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoInteractions; -import com.google.gerrit.entities.Account; +import com.google.gerrit.extensions.registration.DynamicMap; +import com.google.gerrit.extensions.registration.PrivateInternals_DynamicMapImpl; import com.google.gerrit.server.config.AllUsersName; +import com.google.inject.util.Providers; import com.googlesource.gerrit.plugins.multisite.forwarder.ForwardedEventHandler; import com.googlesource.gerrit.plugins.multisite.forwarder.ForwardedIndexAccountHandler; import com.googlesource.gerrit.plugins.multisite.forwarder.ForwardedIndexChangeHandler; @@ -34,7 +36,6 @@ import com.googlesource.gerrit.plugins.multisite.forwarder.router.IndexEventRouter; import com.googlesource.gerrit.plugins.multisite.forwarder.router.StreamEventRouter; import com.googlesource.gerrit.plugins.replication.events.RefReplicationDoneEvent; -import java.util.Optional; import org.eclipse.jgit.lib.ObjectId; import org.junit.Before; import org.junit.Test; @@ -52,17 +53,16 @@ @Mock private ForwardedIndexProjectHandler indexProjectHandler; @Mock private ForwardedEventHandler forwardedEventHandler; private AllUsersName allUsersName = new AllUsersName("All-Users"); + PrivateInternals_DynamicMapImpl<ForwardedIndexingHandler<?, ? extends IndexEvent>> indexHandlers; @Before public void setUp() { - router = - new IndexEventRouter( - indexAccountHandler, - indexChangeHandler, - indexGroupHandler, - indexProjectHandler, - allUsersName, - INSTANCE_ID); + indexHandlers = (PrivateInternals_DynamicMapImpl) DynamicMap.emptyMap(); + indexHandlers.put(GERRIT, AccountIndexEvent.TYPE, Providers.of(indexAccountHandler)); + indexHandlers.put(GERRIT, GroupIndexEvent.TYPE, Providers.of(indexGroupHandler)); + indexHandlers.put(GERRIT, ChangeIndexEvent.TYPE, Providers.of(indexChangeHandler)); + indexHandlers.put(GERRIT, ProjectIndexEvent.TYPE, Providers.of(indexProjectHandler)); + router = new IndexEventRouter(indexAccountHandler, indexHandlers, allUsersName, INSTANCE_ID); } @Test @@ -70,22 +70,19 @@ final AccountIndexEvent event = new AccountIndexEvent(1, INSTANCE_ID); router.route(event); - verify(indexAccountHandler) - .indexAsync(Account.id(event.accountId), ForwardedIndexingHandler.Operation.INDEX); + verify(indexAccountHandler).handle(event); verifyNoInteractions(indexChangeHandler, indexGroupHandler, indexProjectHandler); } @Test public void streamEventRouterShouldTriggerAccountIndexFlush() throws Exception { - StreamEventRouter streamEventRouter = new StreamEventRouter(forwardedEventHandler, router); final AccountIndexEvent event = new AccountIndexEvent(1, INSTANCE_ID); router.route(event); - verify(indexAccountHandler) - .indexAsync(Account.id(event.accountId), ForwardedIndexingHandler.Operation.INDEX); + verify(indexAccountHandler).handle(event); verifyNoInteractions(indexChangeHandler, indexGroupHandler, indexProjectHandler); @@ -100,8 +97,7 @@ final GroupIndexEvent event = new GroupIndexEvent(groupId, ObjectId.zeroId(), INSTANCE_ID); router.route(event); - verify(indexGroupHandler) - .index(groupId, ForwardedIndexingHandler.Operation.INDEX, Optional.of(event)); + verify(indexGroupHandler).handle(event); verifyNoInteractions(indexAccountHandler, indexChangeHandler, indexProjectHandler); } @@ -112,8 +108,7 @@ final ProjectIndexEvent event = new ProjectIndexEvent(projectName, INSTANCE_ID); router.route(event); - verify(indexProjectHandler) - .index(projectName, ForwardedIndexingHandler.Operation.INDEX, Optional.of(event)); + verify(indexProjectHandler).handle(event); verifyNoInteractions(indexAccountHandler, indexChangeHandler, indexGroupHandler); } @@ -123,11 +118,7 @@ final ChangeIndexEvent event = new ChangeIndexEvent("projectName", 3, false, INSTANCE_ID); router.route(event); - verify(indexChangeHandler) - .index( - event.projectName + "~" + event.changeId, - ForwardedIndexingHandler.Operation.INDEX, - Optional.of(event)); + verify(indexChangeHandler).handle(event); verifyNoInteractions(indexAccountHandler, indexGroupHandler, indexProjectHandler); } @@ -137,20 +128,16 @@ final ChangeIndexEvent event = new ChangeIndexEvent("projectName", 3, true, INSTANCE_ID); router.route(event); - verify(indexChangeHandler) - .index( - event.projectName + "~" + event.changeId, - ForwardedIndexingHandler.Operation.DELETE, - Optional.of(event)); + verify(indexChangeHandler).handle(event); verifyNoInteractions(indexAccountHandler, indexGroupHandler, indexProjectHandler); } @Test - public void routerShouldFailForNotRecognisedEvents() throws Exception { + public void routerShouldIgnoreNotRecognisedEvents() throws Exception { final IndexEvent newEventType = new IndexEvent("new-type", INSTANCE_ID) {}; - assertThrows(UnsupportedOperationException.class, () -> router.route(newEventType)); + router.route(newEventType); verifyNoInteractions( indexAccountHandler, indexChangeHandler, indexGroupHandler, indexProjectHandler); }
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 f940fc4..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
@@ -17,6 +17,7 @@ import static com.google.common.net.HttpHeaders.CONTENT_TYPE; import static com.google.common.truth.Truth.assertThat; import static com.googlesource.gerrit.plugins.multisite.http.HttpModule.LAG_ENDPOINT_SEGMENT; +import static org.junit.Assume.assumeTrue; import com.gerritforge.gerrit.globalrefdb.validation.Log4jSharedRefLogger; import com.gerritforge.gerrit.globalrefdb.validation.SharedRefDbConfiguration; @@ -32,6 +33,7 @@ import com.google.inject.Inject; import com.google.inject.Scopes; import com.google.inject.multibindings.OptionalBinder; +import com.googlesource.gerrit.plugins.multisite.Configuration; import com.googlesource.gerrit.plugins.multisite.Log4jProjectVersionLogger; import com.googlesource.gerrit.plugins.multisite.ProjectVersionLogger; import com.googlesource.gerrit.plugins.multisite.cache.CacheModule; @@ -57,15 +59,17 @@ private static final String LAG_ENDPOINT = String.format("/plugins/multi-site/%s", LAG_ENDPOINT_SEGMENT); private ReplicationStatus replicationStatus; + private Configuration multiSiteConfig; 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 = @@ -83,11 +87,13 @@ @Before public void setUp() throws IOException { replicationStatus = plugin.getSysInjector().getInstance(ReplicationStatus.class); + multiSiteConfig = plugin.getSysInjector().getInstance(Configuration.class); } @Test @GerritConfig(name = "gerrit.instanceId", value = "testInstanceId") public void shouldSucceedForAdminUsers() throws Exception { + assumeReplicationLagEnabled(); RestResponse result = adminRestSession.get(LAG_ENDPOINT); result.assertOK(); @@ -97,6 +103,7 @@ @Test @GerritConfig(name = "gerrit.instanceId", value = "testInstanceId") public void shouldFailWhenUserHasNoAdminServerCapability() throws Exception { + assumeReplicationLagEnabled(); RestResponse result = userRestSession.get(LAG_ENDPOINT); result.assertForbidden(); assertThat(result.getEntityContent()).contains("not permitted"); @@ -105,6 +112,7 @@ @Test @GerritConfig(name = "gerrit.instanceId", value = "testInstanceId") public void shouldReturnCurrentProjectLag() throws Exception { + assumeReplicationLagEnabled(); replicationStatus.doUpdateLag(Project.nameKey("foo"), 123L); RestResponse result = adminRestSession.get(LAG_ENDPOINT); @@ -116,6 +124,7 @@ @Test @GerritConfig(name = "gerrit.instanceId", value = "testInstanceId") public void shouldReturnProjectsOrderedDescendinglyByLag() throws Exception { + assumeReplicationLagEnabled(); replicationStatus.doUpdateLag(Project.nameKey("bar"), 123L); replicationStatus.doUpdateLag(Project.nameKey("foo"), 3L); replicationStatus.doUpdateLag(Project.nameKey("baz"), 52300L); @@ -129,6 +138,7 @@ @Test @GerritConfig(name = "gerrit.instanceId", value = "testInstanceId") public void shouldHonourTheLimitParameter() throws Exception { + assumeReplicationLagEnabled(); replicationStatus.doUpdateLag(Project.nameKey("bar"), 1L); replicationStatus.doUpdateLag(Project.nameKey("foo"), 2L); replicationStatus.doUpdateLag(Project.nameKey("baz"), 3L); @@ -142,4 +152,10 @@ private String contentWithoutMagicJson(RestResponse response) throws IOException { return response.getEntityContent().substring(RestApiServlet.JSON_MAGIC.length); } + + private void assumeReplicationLagEnabled() { + assumeTrue( + "Replication status skipped as replication lag is disabled", + multiSiteConfig.replicationLagEnabled()); + } }