Merge branch 'stable-3.5' into stable-3.6 * stable-3.5: DRY out shouldNotBeTrackedAnymoreOnGlobalRefDb Ignore refs/multi-site/version on global-refdb in push replication filter Change-Id: I2ef34f2bad8d0c63124cbb5ea42ec1d129b0663e
diff --git a/DESIGN.md b/DESIGN.md index 1bbe78f..b114eb3 100644 --- a/DESIGN.md +++ b/DESIGN.md
@@ -2,7 +2,7 @@ This document collects and organizes thoughts about the design of the Gerrit multi-site plugin, supporting the definition of the -[implementation roadmap](#next-steps-in-the-road-map). +[implementation roadmap](#next-steps-in-the-roadmap). It first presents background for the problems the plugin will address and the tools currently available in the Gerrit ecosystem that support the
diff --git a/e2e-tests/test.sh b/e2e-tests/test.sh index 12aca41..b069565 100755 --- a/e2e-tests/test.sh +++ b/e2e-tests/test.sh
@@ -16,8 +16,8 @@ LOCATION="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null 2>&1 && pwd )" LOCAL_ENV="$( cd "${LOCATION}/../setup_local_env" >/dev/null 2>&1 && pwd )" -GERRIT_BRANCH=stable-3.5 -GERRIT_CI=https://archive-ci.gerritforge.com/view/Plugins-$GERRIT_BRANCH/job +GERRIT_BRANCH=stable-3.6 +GERRIT_CI=https://gerrit-ci.gerritforge.com/view/Plugins-$GERRIT_BRANCH/job LAST_BUILD=lastSuccessfulBuild/artifact/bazel-bin/plugins DEF_MULTISITE_LOCATION=${LOCATION}/../../../bazel-bin/plugins/multi-site/multi-site.jar DEF_GERRIT_IMAGE=3.4.0-centos8 @@ -25,6 +25,7 @@ DEF_GERRIT_HEALTHCHECK_INTERVAL=5s DEF_GERRIT_HEALTHCHECK_TIMEOUT=5s DEF_GERRIT_HEALTHCHECK_RETRIES=5 +COMMON_PLUGINS_LIST="websession-broker healthcheck zookeeper-refdb" function check_application_requirements { type java >/dev/null 2>&1 || { echo >&2 "Require java but it's not installed. Aborting."; exit 1; } @@ -112,6 +113,19 @@ return 0 } +function download_plugin { + local PLUGIN_NAME=$1 + + echo "Downloading $PLUGIN_NAME plugin $GERRIT_BRANCH onto $TARGET_DIR" + wget $GERRIT_CI/plugin-$PLUGIN_NAME-bazel-$GERRIT_BRANCH/$LAST_BUILD/$PLUGIN_NAME/$PLUGIN_NAME.jar \ + -O $COMMON_PLUGINS/$PLUGIN_NAME.jar || \ + wget $GERRIT_CI/plugin-$PLUGIN_NAME-bazel-master-$GERRIT_BRANCH/$LAST_BUILD/$PLUGIN_NAME/$PLUGIN_NAME.jar \ + -O $COMMON_PLUGINS/$PLUGIN_NAME.jar || \ + { echo >&2 "Cannot download $PLUGIN_NAME plugin: Check internet connection. Aborting"; exit 1; } + + return 0 +} + # Check application requirements check_application_requirements @@ -220,27 +234,14 @@ echo "Downloading common plugins" COMMON_PLUGINS=${DEPLOYMENT_LOCATION}/common_plugins mkdir -p ${COMMON_PLUGINS} +for plugin in $COMMON_PLUGINS_LIST; do download_plugin $plugin; done echo "plugin location[${MULTISITE_LIB_LOCATION}]" cp -f $MULTISITE_LIB_LOCATION $COMMON_PLUGINS/multi-site.jar >/dev/null 2>&1 || \ { echo >&2 "$MULTISITE_LIB_LOCATION: Not able to copy the file. Aborting"; exit 1; } -echo "Downloading websession-broker plugin $GERRIT_BRANCH" -wget $GERRIT_CI/plugin-websession-broker-bazel-$GERRIT_BRANCH/$LAST_BUILD/websession-broker/websession-broker.jar \ - -O $COMMON_PLUGINS/websession-broker.jar || { echo >&2 "Cannot download websession-broker plugin: Check internet connection. Aborting"; exit 1; } - -echo "Downloading healthcheck plugin $GERRIT_BRANCH" -wget $GERRIT_CI/plugin-healthcheck-bazel-$GERRIT_BRANCH/$LAST_BUILD/healthcheck/healthcheck.jar \ - -O $COMMON_PLUGINS/healthcheck.jar || { echo >&2 "Cannot download healthcheck plugin: Check internet connection. Aborting"; exit 1; } - -echo "Downloading zookeeper plugin $GERRIT_BRANCH" -wget $GERRIT_CI/plugin-zookeeper-refdb-bazel-$GERRIT_BRANCH/$LAST_BUILD/zookeeper-refdb/zookeeper-refdb.jar \ - -O $COMMON_PLUGINS/zookeeper-refdb.jar || { echo >&2 "Cannot download zookeeper plugin: Check internet connection. Aborting"; exit 1; } - if [ "$BROKER_TYPE" = "kafka" ]; then - echo "Downloading events-kafka plugin $GERRIT_BRANCH" - wget $GERRIT_CI/plugin-events-kafka-bazel-$GERRIT_BRANCH/$LAST_BUILD/events-kafka/events-kafka.jar \ - -O $COMMON_PLUGINS/events-kafka.jar || { echo >&2 "Cannot download events-kafka plugin: Check internet connection. Aborting"; exit 1; } + download_plugin events-kafka $COMMON_PLUGINS BROKER_PORT=9092 BROKER_HOST=kafka BROKER_PLUGIN=events-kafka
diff --git a/setup_local_env/setup.sh b/setup_local_env/setup.sh index 5180473..7697f24 100755 --- a/setup_local_env/setup.sh +++ b/setup_local_env/setup.sh
@@ -16,8 +16,8 @@ SCRIPT_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null 2>&1 && pwd )" -GERRIT_BRANCH=stable-3.5 -GERRIT_CI=https://archive-ci.gerritforge.com/view/Plugins-$GERRIT_BRANCH/job +GERRIT_BRANCH=stable-3.6 +GERRIT_CI=https://gerrit-ci.gerritforge.com/view/Plugins-$GERRIT_BRANCH/job LAST_BUILD=lastSuccessfulBuild/artifact/bazel-bin/plugins function check_application_requirements { @@ -224,6 +224,16 @@ fi } +function download_artifact_from_ci { + local artifact_name=$1 + local prefix=${2:-plugin} + wget $GERRIT_CI/$prefix-$artifact_name-bazel-$GERRIT_BRANCH/$LAST_BUILD/$artifact_name/$artifact_name.jar \ + -O $DEPLOYMENT_LOCATION/$artifact_name.jar || \ + wget $GERRIT_CI/$prefix-$artifact_name-bazel-master-$GERRIT_BRANCH/$LAST_BUILD/$artifact_name/$artifact_name.jar \ + -O $DEPLOYMENT_LOCATION/$artifact_name.jar || \ + { echo >&2 "Cannot download $artifact_name $prefix: Check internet connection. Aborting"; exit 1; } +} + while [ $# -ne 0 ] do case "$1" in @@ -429,70 +439,37 @@ if [ $DOWNLOAD_WEBSESSION_PLUGIN = "true" ];then echo "Downloading websession-broker plugin $GERRIT_BRANCH" - wget $GERRIT_CI/plugin-websession-broker-bazel-$GERRIT_BRANCH/$LAST_BUILD/websession-broker/websession-broker.jar \ - -O $DEPLOYMENT_LOCATION/websession-broker.jar || \ - wget $GERRIT_CI/plugin-websession-broker-bazel-master-$GERRIT_BRANCH/$LAST_BUILD/websession-broker/websession-broker.jar \ - -O $DEPLOYMENT_LOCATION/websession-broker.jar || \ - { echo >&2 "Cannot download websession-broker plugin: Check internet connection. Abort\ -ing"; exit 1; } - wget $GERRIT_CI/plugin-healthcheck-bazel-$GERRIT_BRANCH/$LAST_BUILD/healthcheck/healthcheck.jar \ - -O $DEPLOYMENT_LOCATION/healthcheck.jar || { echo >&2 "Cannot download healthcheck plugin: Check internet connection. Abort\ -ing"; exit 1; } + download_artifact_from_ci websession-broker + download_artifact_from_ci healthcheck + else echo "Without the websession-broker; user login via haproxy will fail." fi echo "Downloading zookeeper plugin $GERRIT_BRANCH" - wget $GERRIT_CI/plugin-zookeeper-refdb-bazel-$GERRIT_BRANCH/$LAST_BUILD/zookeeper-refdb/zookeeper-refdb.jar \ - -O $DEPLOYMENT_LOCATION/zookeeper-refdb.jar || \ - wget $GERRIT_CI/plugin-zookeeper-refdb-bazel-master-$GERRIT_BRANCH/$LAST_BUILD/zookeeper-refdb/zookeeper-refdb.jar \ - -O $DEPLOYMENT_LOCATION/zookeeper-refdb.jar || \ - { echo >&2 "Cannot download zookeeper plugin: Check internet connection. Abort\ -ing"; exit 1; } + download_artifact_from_ci zookeeper-refdb if [ "$BROKER_TYPE" = "kafka" ]; then echo "Downloading events-kafka plugin $GERRIT_BRANCH" - wget $GERRIT_CI/plugin-events-kafka-bazel-$GERRIT_BRANCH/$LAST_BUILD/events-kafka/events-kafka.jar \ - -O $DEPLOYMENT_LOCATION/events-kafka.jar || \ - wget $GERRIT_CI/plugin-events-kafka-bazel-master-$GERRIT_BRANCH/$LAST_BUILD/events-kafka/events-kafka.jar \ - -O $DEPLOYMENT_LOCATION/events-kafka.jar || \ - { echo >&2 "Cannot download events-kafka plugin: Check internet connection. Abort\ -ing"; exit 1; } + download_artifact_from_ci events-kafka fi if [ "$BROKER_TYPE" = "kinesis" ]; then echo "Downloading events-aws-kinesis plugin $GERRIT_BRANCH" - wget $GERRIT_CI/plugin-events-aws-kinesis-bazel-$GERRIT_BRANCH/$LAST_BUILD/events-aws-kinesis/events-aws-kinesis.jar \ - -O $DEPLOYMENT_LOCATION/events-aws-kinesis.jar || \ - wget $GERRIT_CI/plugin-events-aws-kinesis-bazel-master-$GERRIT_BRANCH/$LAST_BUILD/events-aws-kinesis/events-aws-kinesis.jar \ - -O $DEPLOYMENT_LOCATION/events-aws-kinesis.jar || \ - { echo >&2 "Cannot download events-aws-kinesis plugin: Check internet connection. Abort\ -ing"; exit 1; } + download_artifact_from_ci events-aws-kinesis fi if [ "$BROKER_TYPE" = "gcloud-pubsub" ]; then echo "Downloading events-gcloud-pubsub plugin $GERRIT_BRANCH" - wget $GERRIT_CI/plugin-events-gcloud-pubsub-bazel-$GERRIT_BRANCH/$LAST_BUILD/events-gcloud-pubsub/events-gcloud-pubsub.jar \ - -O $DEPLOYMENT_LOCATION/events-gcloud-pubsub.jar || \ - wget $GERRIT_CI/plugin-events-gcloud-pubsub-bazel-master-$GERRIT_BRANCH/$LAST_BUILD/events-gcloud-pubsub/events-gcloud-pubsub.jar \ - -O $DEPLOYMENT_LOCATION/events-gcloud-pubsub.jar || \ - { echo >&2 "Cannot download events-gcloud-pubsub plugin: Check internet connection. Abort\ -ing"; exit 1; } + download_artifact_from_ci events-gcloud-pubsub fi echo "Downloading metrics-reporter-prometheus plugin $GERRIT_BRANCH" - wget $GERRIT_CI/plugin-metrics-reporter-prometheus-bazel-$GERRIT_BRANCH/$LAST_BUILD/metrics-reporter-prometheus/metrics-reporter-prometheus.jar \ - -O $DEPLOYMENT_LOCATION/metrics-reporter-prometheus.jar || \ - wget $GERRIT_CI/plugin-metrics-reporter-prometheus-bazel-master-$GERRIT_BRANCH/$LAST_BUILD/metrics-reporter-prometheus/metrics-reporter-prometheus.jar \ - -O $DEPLOYMENT_LOCATION/metrics-reporter-prometheus.jar || \ - { echo >&2 "Cannot download metrics-reporter-prometheus plugin: Check internet connection. Abort\ -ing"; exit 1; } + download_artifact_from_ci metrics-reporter-prometheus echo "Downloading pull-replication plugin $GERRIT_BRANCH" - wget $GERRIT_CI/plugin-pull-replication-bazel-$GERRIT_BRANCH/$LAST_BUILD/pull-replication/pull-replication.jar \ - -O $DEPLOYMENT_LOCATION/pull-replication.jar || { echo >&2 "Cannot download pull-replication plugin: Check internet connection. Abort\ -ing"; exit 1; } + download_artifact_from_ci pull-replication if [ "$HTTPS_ENABLED" = "true" ];then export HTTP_PROTOCOL="https"
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 5c443e7..e3f53ab 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/multisite/Configuration.java +++ b/src/main/java/com/googlesource/gerrit/plugins/multisite/Configuration.java
@@ -306,12 +306,15 @@ 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 boolean DEFAULT_SYNCHRONIZE_FORCED = true; private final int threadPoolSize; private final int retryInterval; private final int maxTries; private final int numStripedLocks; + private final boolean synchronizeForced; private Index(Supplier<Config> cfg) { super(cfg, INDEX_SECTION); @@ -322,6 +325,8 @@ maxTries = getInt(cfg, INDEX_SECTION, null, MAX_TRIES_KEY, DEFAULT_INDEX_MAX_TRIES); numStripedLocks = getInt(cfg, INDEX_SECTION, null, NUM_STRIPED_LOCKS, DEFAULT_NUM_STRIPED_LOCKS); + synchronizeForced = + getBoolean(cfg, INDEX_SECTION, null, SYNCHRONIZE_FORCED_KEY, DEFAULT_SYNCHRONIZE_FORCED); } public int threadPoolSize() { @@ -339,6 +344,10 @@ public int numStripedLocks() { return numStripedLocks; } + + public boolean synchronizeForced() { + return synchronizeForced; + } } public static class Broker {
diff --git a/src/main/java/com/googlesource/gerrit/plugins/multisite/LibModuleLogFile.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/LibModuleLogFile.java index 106bcde..7c88185 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/multisite/LibModuleLogFile.java +++ b/src/main/java/com/googlesource/gerrit/plugins/multisite/LibModuleLogFile.java
@@ -23,10 +23,11 @@ public abstract class LibModuleLogFile { public LibModuleLogFile(SystemLog systemLog, String logName, Layout layout) { - AsyncAppender asyncAppender = systemLog.createAsyncAppender(logName, layout, true, true); Logger logger = LogManager.getLogger(logName); - logger.removeAppender(logName); - logger.addAppender(asyncAppender); - logger.setAdditivity(false); + if (logger.getAppender(logName) == null) { + AsyncAppender asyncAppender = systemLog.createAsyncAppender(logName, layout, true, true); + logger.addAppender(asyncAppender); + logger.setAdditivity(false); + } } }
diff --git a/src/main/java/com/googlesource/gerrit/plugins/multisite/cache/CacheModule.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/cache/CacheModule.java index 6373f58..5e54951 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/multisite/cache/CacheModule.java +++ b/src/main/java/com/googlesource/gerrit/plugins/multisite/cache/CacheModule.java
@@ -14,18 +14,31 @@ package com.googlesource.gerrit.plugins.multisite.cache; +import com.google.common.annotations.VisibleForTesting; import com.google.gerrit.extensions.events.NewProjectCreatedListener; import com.google.gerrit.extensions.events.ProjectDeletedListener; import com.google.gerrit.extensions.registration.DynamicSet; import com.google.gerrit.lifecycle.LifecycleModule; import com.google.gerrit.server.cache.CacheRemovalListener; +import com.googlesource.gerrit.plugins.multisite.ExecutorProvider; import java.util.concurrent.Executor; public class CacheModule extends LifecycleModule { + private final Class<? extends ExecutorProvider> cacheExecutorProviderClass; + + public CacheModule() { + this(CacheExecutorProvider.class); + } + + @VisibleForTesting + public CacheModule(Class<? extends ExecutorProvider> cacheExecutorProviderClass) { + this.cacheExecutorProviderClass = cacheExecutorProviderClass; + } + @Override protected void configure() { - bind(Executor.class).annotatedWith(CacheExecutor.class).toProvider(CacheExecutorProvider.class); + bind(Executor.class).annotatedWith(CacheExecutor.class).toProvider(cacheExecutorProviderClass); listener().to(CacheExecutorProvider.class); DynamicSet.bind(binder(), CacheRemovalListener.class).to(CacheEvictionHandler.class); DynamicSet.bind(binder(), NewProjectCreatedListener.class).to(ProjectListUpdateHandler.class);
diff --git a/src/main/java/com/googlesource/gerrit/plugins/multisite/event/EventModule.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/event/EventModule.java index 5e70aca..8c67823 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/multisite/event/EventModule.java +++ b/src/main/java/com/googlesource/gerrit/plugins/multisite/event/EventModule.java
@@ -16,9 +16,9 @@ import com.gerritforge.gerrit.eventbroker.publisher.StreamEventPublisherConfig; import com.gerritforge.gerrit.eventbroker.publisher.StreamEventPublisherModule; +import com.google.gerrit.extensions.events.GitBatchRefUpdateListener; 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.multibindings.OptionalBinder; import com.googlesource.gerrit.plugins.multisite.Configuration; @@ -47,7 +47,8 @@ OptionalBinder<ProjectVersionRefUpdate> projectVersionRefUpdateBinder = OptionalBinder.newOptionalBinder(binder(), ProjectVersionRefUpdate.class); if (configuration.getSharedRefDbConfiguration().getSharedRefDb().isEnabled()) { - DynamicSet.bind(binder(), EventListener.class).to(ProjectVersionRefUpdateImpl.class); + DynamicSet.bind(binder(), GitBatchRefUpdateListener.class) + .to(ProjectVersionRefUpdateImpl.class); projectVersionRefUpdateBinder.setBinding().to(ProjectVersionRefUpdateImpl.class); } }
diff --git a/src/main/java/com/googlesource/gerrit/plugins/multisite/index/ChangeCheckerImpl.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/index/ChangeCheckerImpl.java index 6e8fa15..063d7bf 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/multisite/index/ChangeCheckerImpl.java +++ b/src/main/java/com/googlesource/gerrit/plugins/multisite/index/ChangeCheckerImpl.java
@@ -25,6 +25,7 @@ import com.google.inject.assistedinject.Assisted; import com.googlesource.gerrit.plugins.multisite.forwarder.events.ChangeIndexEvent; import java.io.IOException; +import java.sql.Timestamp; import java.util.Objects; import java.util.Optional; import org.eclipse.jgit.errors.MissingObjectException; @@ -170,6 +171,7 @@ } private Optional<Long> computeLastChangeTs() { - return getChangeNotes().map(notes -> notes.getChange().getLastUpdatedOn().getTime() / 1000); + return getChangeNotes() + .map(notes -> Timestamp.from(notes.getChange().getLastUpdatedOn()).getTime() / 1000); } }
diff --git a/src/main/java/com/googlesource/gerrit/plugins/multisite/index/CurrentRequestContext.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/index/CurrentRequestContext.java new file mode 100644 index 0000000..d6598b9 --- /dev/null +++ b/src/main/java/com/googlesource/gerrit/plugins/multisite/index/CurrentRequestContext.java
@@ -0,0 +1,58 @@ +// Copyright (C) 2023 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.googlesource.gerrit.plugins.multisite.index; + +import com.google.common.flogger.FluentLogger; +import com.google.gerrit.server.util.ManualRequestContext; +import com.google.gerrit.server.util.OneOffRequestContext; +import com.google.gerrit.server.util.RequestContext; +import com.google.gerrit.server.util.ThreadLocalRequestContext; +import com.google.inject.Inject; +import com.google.inject.Singleton; +import com.googlesource.gerrit.plugins.multisite.Configuration; +import java.util.function.Consumer; + +@Singleton +public class CurrentRequestContext { + private static final FluentLogger logger = FluentLogger.forEnclosingClass(); + + private ThreadLocalRequestContext threadLocalCtx; + private Configuration cfg; + private OneOffRequestContext oneOffCtx; + + @Inject + public CurrentRequestContext( + ThreadLocalRequestContext threadLocalCtx, Configuration cfg, OneOffRequestContext oneOffCtx) { + this.threadLocalCtx = threadLocalCtx; + this.cfg = cfg; + this.oneOffCtx = oneOffCtx; + } + + public void onlyWithContext(Consumer<RequestContext> body) { + RequestContext ctx = threadLocalCtx.getContext(); + if (ctx == null && !cfg.index().synchronizeForced()) { + logger.atFine().log("No context, skipping event (index.synchronizeForced is false)"); + return; + } + + if (ctx == null) { + try (ManualRequestContext manualCtx = oneOffCtx.open()) { + body.accept(manualCtx); + } + } else { + body.accept(ctx); + } + } +}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/multisite/index/IndexEventHandler.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/index/IndexEventHandler.java index ee16b07..9aee56e 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/multisite/index/IndexEventHandler.java +++ b/src/main/java/com/googlesource/gerrit/plugins/multisite/index/IndexEventHandler.java
@@ -48,6 +48,7 @@ private final ChangeCheckerImpl.Factory changeChecker; private final GroupChecker groupChecker; private final String instanceId; + private final CurrentRequestContext currCtx; @Inject IndexEventHandler( @@ -55,27 +56,32 @@ DynamicSet<IndexEventForwarder> forwarders, ChangeCheckerImpl.Factory changeChecker, GroupChecker groupChecker, - @GerritInstanceId String instanceId) { + @GerritInstanceId String instanceId, + CurrentRequestContext currCtx) { this.forwarders = forwarders; this.executor = executor; this.changeChecker = changeChecker; this.groupChecker = groupChecker; this.instanceId = instanceId; + this.currCtx = currCtx; } @Override public void onAccountIndexed(int id) { - if (!Context.isForwardedEvent()) { - IndexAccountTask task = new IndexAccountTask(new AccountIndexEvent(id, instanceId)); - if (queuedTasks.add(task)) { - executor.execute(task); - } - } + currCtx.onlyWithContext( + (ctx) -> { + if (!Context.isForwardedEvent()) { + IndexAccountTask task = new IndexAccountTask(new AccountIndexEvent(id, instanceId)); + if (queuedTasks.add(task)) { + executor.execute(task); + } + } + }); } @Override public void onChangeIndexed(String projectName, int id) { - executeIndexChangeTask(projectName, id); + currCtx.onlyWithContext((ctx) -> executeIndexChangeTask(projectName, id)); } @Override @@ -178,7 +184,9 @@ @Override public String toString() { - return String.format("Index change %s in target instance", changeIndexEvent.changeId); + return String.format( + "Index change %s for project %s produced by instance %s", + changeIndexEvent.changeId, changeIndexEvent.projectName, changeIndexEvent.instanceId); } } @@ -209,7 +217,9 @@ @Override public String toString() { - return String.format("Index change %s in target instance", changeIndexEvent.changeId); + return String.format( + "Index change %s for project %s produced by instance %s", + changeIndexEvent.changeId, changeIndexEvent.projectName, changeIndexEvent.instanceId); } }
diff --git a/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/ProjectVersionRefUpdateImpl.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/ProjectVersionRefUpdateImpl.java index 4d2515e..150ea52 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/ProjectVersionRefUpdateImpl.java +++ b/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/ProjectVersionRefUpdateImpl.java
@@ -24,11 +24,7 @@ import com.google.common.flogger.FluentLogger; import com.google.gerrit.entities.Project; import com.google.gerrit.entities.Project.NameKey; -import com.google.gerrit.entities.RefNames; -import com.google.gerrit.server.config.GerritInstanceId; -import com.google.gerrit.server.events.Event; -import com.google.gerrit.server.events.EventListener; -import com.google.gerrit.server.events.RefUpdatedEvent; +import com.google.gerrit.extensions.events.GitBatchRefUpdateListener; import com.google.gerrit.server.extensions.events.GitReferenceUpdated; import com.google.gerrit.server.git.GitRepositoryManager; import com.google.inject.Inject; @@ -45,7 +41,8 @@ import org.eclipse.jgit.lib.Repository; @Singleton -public class ProjectVersionRefUpdateImpl implements EventListener, ProjectVersionRefUpdate { +public class ProjectVersionRefUpdateImpl + implements GitBatchRefUpdateListener, ProjectVersionRefUpdate { private static final FluentLogger logger = FluentLogger.forEnclosingClass(); private static final Set<RefUpdate.Result> SUCCESSFUL_RESULTS = ImmutableSet.of(RefUpdate.Result.NEW, RefUpdate.Result.FORCED, RefUpdate.Result.NO_CHANGE); @@ -53,7 +50,6 @@ private final GitRepositoryManager gitRepositoryManager; private final GitReferenceUpdated gitReferenceUpdated; private final ProjectVersionLogger verLogger; - private final String nodeInstanceId; protected final SharedRefDatabaseWrapper sharedRefDb; @@ -62,61 +58,43 @@ GitRepositoryManager gitRepositoryManager, SharedRefDatabaseWrapper sharedRefDb, GitReferenceUpdated gitReferenceUpdated, - ProjectVersionLogger verLogger, - @GerritInstanceId String nodeInstanceId) { + ProjectVersionLogger verLogger) { this.gitRepositoryManager = gitRepositoryManager; this.sharedRefDb = sharedRefDb; this.gitReferenceUpdated = gitReferenceUpdated; this.verLogger = verLogger; - this.nodeInstanceId = nodeInstanceId; } @Override - public void onEvent(Event event) { - logger.atFine().log("Processing event type: %s", event.type); + public void onGitBatchRefUpdate(Event event) { // Producer of the Event use RefUpdatedEvent to trigger the version update - if (nodeInstanceId.equals(event.instanceId) && event instanceof RefUpdatedEvent) { - updateProducerProjectVersionUpdate((RefUpdatedEvent) event); - } + updateProducerProjectVersionUpdate(event); } - private boolean isSpecialRefName(String refName) { - return refName.startsWith(RefNames.REFS_SEQUENCES) - || refName.startsWith(RefNames.REFS_STARRED_CHANGES) - || refName.equals(MULTI_SITE_VERSIONING_REF); - } - - private void updateProducerProjectVersionUpdate(RefUpdatedEvent refUpdatedEvent) { - String refName = refUpdatedEvent.getRefName(); - - if (isSpecialRefName(refName)) { + private void updateProducerProjectVersionUpdate(Event refUpdatedEvent) { + if (refUpdatedEvent.getRefNames().stream() + .allMatch(refName -> refName.equals(MULTI_SITE_VERSIONING_REF))) { logger.atFine().log( "Found a special ref name %s, skipping update for %s", - refName, refUpdatedEvent.getProjectNameKey().get()); + MULTI_SITE_VERSIONING_REF, refUpdatedEvent.getProjectName()); return; } + try { - Project.NameKey projectNameKey = refUpdatedEvent.getProjectNameKey(); + Project.NameKey projectNameKey = Project.nameKey(refUpdatedEvent.getProjectName()); long newVersion = getCurrentGlobalVersionNumber(); - Optional<RefUpdate> newProjectVersionRefUpdate = - updateLocalProjectVersion(projectNameKey, newVersion); + RefUpdate newProjectVersionRefUpdate = updateLocalProjectVersion(projectNameKey, newVersion); - if (newProjectVersionRefUpdate.isPresent()) { - verLogger.log(projectNameKey, newVersion, 0L); + verLogger.log(projectNameKey, newVersion, 0L); - if (updateSharedProjectVersion(projectNameKey, newVersion)) { - gitReferenceUpdated.fire(projectNameKey, newProjectVersionRefUpdate.get(), null); - } - } else { - logger.atWarning().log( - "Ref %s not found on projet %s: skipping project version update", - refUpdatedEvent.getRefName(), projectNameKey); + if (updateSharedProjectVersion(projectNameKey, newVersion)) { + gitReferenceUpdated.fire(projectNameKey, newProjectVersionRefUpdate, null); } } catch (LocalProjectVersionUpdateException | SharedProjectVersionUpdateException e) { logger.atSevere().withCause(e).log( "Issue encountered when updating version for project %s", - refUpdatedEvent.getProjectNameKey()); + refUpdatedEvent.getProjectName()); } } @@ -249,8 +227,7 @@ } @SuppressWarnings("FloggerLogString") - private Optional<RefUpdate> updateLocalProjectVersion( - Project.NameKey projectNameKey, long newVersionNumber) + private RefUpdate updateLocalProjectVersion(Project.NameKey projectNameKey, long newVersionNumber) throws LocalProjectVersionUpdateException { logger.atFine().log( "Updating local version for project %s with version %d", @@ -267,7 +244,7 @@ throw new LocalProjectVersionUpdateException(message); } - return Optional.of(refUpdate); + return refUpdate; } catch (IOException e) { String message = "Cannot create versioning command for " + projectNameKey.get(); logger.atSevere().withCause(e).log(message);
diff --git a/src/main/resources/Documentation/config.md b/src/main/resources/Documentation/config.md index 0cfd6ea..0485d5b 100644 --- a/src/main/resources/Documentation/config.md +++ b/src/main/resources/Documentation/config.md
@@ -13,6 +13,9 @@ ```cache.synchronize``` : Whether to synchronize cache evictions. Set to false when relying on low cache TTLs and therefore cache eviction is not strictly needed. + It requires disabling the background cache evictions notifications in + `gerrit.config` by setting `cache.threads = 0`. + Defaults to true. ```cache.threadPoolSize``` @@ -44,6 +47,11 @@ service such as ElasticSearch. Defaults to true. +```index.synchronizeForced``` +: Whether to synchronize forced index events. E.g. on-line reindex +automatically triggered upon version upgrades. +Defaults to true. + ```index.threadPoolSize``` : Maximum number of threads used to send index events to the target instance. Defaults to 4.
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 0863f55..3a24773 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/multisite/ConfigurationTest.java +++ b/src/test/java/com/googlesource/gerrit/plugins/multisite/ConfigurationTest.java
@@ -21,7 +21,9 @@ import static com.googlesource.gerrit.plugins.multisite.Configuration.Event.EVENT_SECTION; import static com.googlesource.gerrit.plugins.multisite.Configuration.Forwarding.DEFAULT_SYNCHRONIZE; import static com.googlesource.gerrit.plugins.multisite.Configuration.Forwarding.SYNCHRONIZE_KEY; +import static com.googlesource.gerrit.plugins.multisite.Configuration.Index.DEFAULT_SYNCHRONIZE_FORCED; import static com.googlesource.gerrit.plugins.multisite.Configuration.Index.INDEX_SECTION; +import static com.googlesource.gerrit.plugins.multisite.Configuration.Index.SYNCHRONIZE_FORCED_KEY; import static com.googlesource.gerrit.plugins.multisite.Configuration.THREAD_POOL_SIZE_KEY; import com.google.common.collect.ImmutableList; @@ -129,4 +131,13 @@ replicationConfig.setBoolean("gerrit", null, "replicateOnStartup", true); assertThat(new Configuration(globalPluginConfig, replicationConfig).validate()).isNotEmpty(); } + + @Test + public void testGetIndexSynchronizeForced() throws Exception { + assertThat(getConfiguration().index().synchronizeForced()) + .isEqualTo(DEFAULT_SYNCHRONIZE_FORCED); + + globalPluginConfig.setBoolean(INDEX_SECTION, null, SYNCHRONIZE_FORCED_KEY, false); + assertThat(getConfiguration().index().synchronizeForced()).isFalse(); + } }
diff --git a/src/test/java/com/googlesource/gerrit/plugins/multisite/cache/CacheEvictionHandlerTest.java b/src/test/java/com/googlesource/gerrit/plugins/multisite/cache/CacheEvictionHandlerTest.java index ce222d0..ed51e1b 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/multisite/cache/CacheEvictionHandlerTest.java +++ b/src/test/java/com/googlesource/gerrit/plugins/multisite/cache/CacheEvictionHandlerTest.java
@@ -14,7 +14,7 @@ package com.googlesource.gerrit.plugins.multisite.cache; -import static org.mockito.Mockito.verifyZeroInteractions; +import static org.mockito.Mockito.verifyNoInteractions; import com.google.common.cache.RemovalCause; import com.google.common.cache.RemovalNotification; @@ -44,6 +44,6 @@ handler.onRemoval( "test", "accounts", RemovalNotification.create("test", "accounts", RemovalCause.EXPLICIT)); - verifyZeroInteractions(executorMock); + verifyNoInteractions(executorMock); } }
diff --git a/src/test/java/com/googlesource/gerrit/plugins/multisite/cache/ProjectListUpdateHandlerTest.java b/src/test/java/com/googlesource/gerrit/plugins/multisite/cache/ProjectListUpdateHandlerTest.java index 69968b5..3a4242c 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/multisite/cache/ProjectListUpdateHandlerTest.java +++ b/src/test/java/com/googlesource/gerrit/plugins/multisite/cache/ProjectListUpdateHandlerTest.java
@@ -20,7 +20,7 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.verifyZeroInteractions; +import static org.mockito.Mockito.verifyNoInteractions; import static org.mockito.Mockito.when; import com.google.common.util.concurrent.MoreExecutors; @@ -90,7 +90,7 @@ handler.onNewProjectCreated(mock(NewProjectCreatedListener.Event.class)); handler.onProjectDeleted(mock(ProjectDeletedListener.Event.class)); Context.unsetForwardedEvent(); - verifyZeroInteractions(forwarder); + verifyNoInteractions(forwarder); } @Test
diff --git a/src/test/java/com/googlesource/gerrit/plugins/multisite/consumer/IndexEventSubscriberTest.java b/src/test/java/com/googlesource/gerrit/plugins/multisite/consumer/IndexEventSubscriberTest.java index 0e63528..fd4a0aa 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/multisite/consumer/IndexEventSubscriberTest.java +++ b/src/test/java/com/googlesource/gerrit/plugins/multisite/consumer/IndexEventSubscriberTest.java
@@ -128,6 +128,6 @@ Change.id(CHANGE_ID), Account.id(9999), BranchNameKey.create(Project.nameKey(PROJECT_NAME), "refs/heads/master"), - TimeUtil.nowTs()); + TimeUtil.now()); } }
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 6413b18..6a03a6c 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
@@ -16,7 +16,7 @@ import static com.google.common.truth.Truth.assertThat; import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.verifyZeroInteractions; +import static org.mockito.Mockito.verifyNoInteractions; import static org.mockito.Mockito.when; import com.google.common.base.Suppliers; @@ -140,7 +140,7 @@ metrics.updateReplicationStatusMetrics(eventMessage); - verifyZeroInteractions(verLogger); + verifyNoInteractions(verLogger); } @Test @@ -152,7 +152,7 @@ metrics.updateReplicationStatusMetrics(eventMessage); - verifyZeroInteractions(verLogger); + verifyNoInteractions(verLogger); } @Test
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 413b6c1..4ffdbbc 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
@@ -16,7 +16,7 @@ import static com.google.gerrit.testing.GerritJUnit.assertThrows; import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.verifyZeroInteractions; +import static org.mockito.Mockito.verifyNoInteractions; import com.google.gerrit.entities.Account; import com.google.gerrit.server.config.AllUsersName; @@ -73,7 +73,7 @@ verify(indexAccountHandler) .indexAsync(Account.id(event.accountId), ForwardedIndexingHandler.Operation.INDEX); - verifyZeroInteractions(indexChangeHandler, indexGroupHandler, indexProjectHandler); + verifyNoInteractions(indexChangeHandler, indexGroupHandler, indexProjectHandler); } @Test @@ -87,7 +87,7 @@ verify(indexAccountHandler) .indexAsync(Account.id(event.accountId), ForwardedIndexingHandler.Operation.INDEX); - verifyZeroInteractions(indexChangeHandler, indexGroupHandler, indexProjectHandler); + verifyNoInteractions(indexChangeHandler, indexGroupHandler, indexProjectHandler); streamEventRouter.route(new RefReplicationDoneEvent(allUsersName.get(), "refs/any", 1)); @@ -103,7 +103,7 @@ verify(indexGroupHandler) .index(groupId, ForwardedIndexingHandler.Operation.INDEX, Optional.of(event)); - verifyZeroInteractions(indexAccountHandler, indexChangeHandler, indexProjectHandler); + verifyNoInteractions(indexAccountHandler, indexChangeHandler, indexProjectHandler); } @Test @@ -115,7 +115,7 @@ verify(indexProjectHandler) .index(projectName, ForwardedIndexingHandler.Operation.INDEX, Optional.of(event)); - verifyZeroInteractions(indexAccountHandler, indexChangeHandler, indexGroupHandler); + verifyNoInteractions(indexAccountHandler, indexChangeHandler, indexGroupHandler); } @Test @@ -129,7 +129,7 @@ ForwardedIndexingHandler.Operation.INDEX, Optional.of(event)); - verifyZeroInteractions(indexAccountHandler, indexGroupHandler, indexProjectHandler); + verifyNoInteractions(indexAccountHandler, indexGroupHandler, indexProjectHandler); } @Test @@ -143,7 +143,7 @@ ForwardedIndexingHandler.Operation.DELETE, Optional.of(event)); - verifyZeroInteractions(indexAccountHandler, indexGroupHandler, indexProjectHandler); + verifyNoInteractions(indexAccountHandler, indexGroupHandler, indexProjectHandler); } @Test @@ -151,7 +151,7 @@ final IndexEvent newEventType = new IndexEvent("new-type", INSTANCE_ID) {}; assertThrows(UnsupportedOperationException.class, () -> router.route(newEventType)); - verifyZeroInteractions( + verifyNoInteractions( indexAccountHandler, indexChangeHandler, indexGroupHandler, indexProjectHandler); } }
diff --git a/src/test/java/com/googlesource/gerrit/plugins/multisite/event/StreamEventRouterTest.java b/src/test/java/com/googlesource/gerrit/plugins/multisite/event/StreamEventRouterTest.java index 3b4bec5..ac90436 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/multisite/event/StreamEventRouterTest.java +++ b/src/test/java/com/googlesource/gerrit/plugins/multisite/event/StreamEventRouterTest.java
@@ -55,6 +55,6 @@ Change.id(1), Account.id(1), BranchNameKey.create("proj", "refs/heads/master"), - TimeUtil.nowTs()); + TimeUtil.now()); } }
diff --git a/src/test/java/com/googlesource/gerrit/plugins/multisite/forwarder/BrokerForwarderTest.java b/src/test/java/com/googlesource/gerrit/plugins/multisite/forwarder/BrokerForwarderTest.java index 28bf56d..c583455 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/multisite/forwarder/BrokerForwarderTest.java +++ b/src/test/java/com/googlesource/gerrit/plugins/multisite/forwarder/BrokerForwarderTest.java
@@ -16,7 +16,7 @@ import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.verifyZeroInteractions; +import static org.mockito.Mockito.verifyNoInteractions; import com.googlesource.gerrit.plugins.multisite.Configuration; import com.googlesource.gerrit.plugins.multisite.broker.BrokerApiWrapper; @@ -99,21 +99,21 @@ @Test public void shouldSkipEventFromHighAvailabilityPluginThread() { brokerForwarder.send(newForwarderTask(HIGH_AVAILABILITY_PLUGIN), testTopic, testEvent); - verifyZeroInteractions(brokerMock); + verifyNoInteractions(brokerMock); } @Test public void shouldSkipEventFromHighAvailabilityPluginForwardedThread() { brokerForwarder.send(newForwarderTask(HIGH_AVAILABILITY_FORWARDED), testTopic, testEvent); - verifyZeroInteractions(brokerMock); + verifyNoInteractions(brokerMock); } @Test public void shouldSkipEventFromHighAvailabilityPluginBatchForwardedThread() { brokerForwarder.send(newForwarderTask(HIGH_AVAILABILITY_BATCH_FORWARDED), testTopic, testEvent); - verifyZeroInteractions(brokerMock); + verifyNoInteractions(brokerMock); } private ForwarderTask newForwarderTask(String threadName) {
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 75596ed..6ca1a7a 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
@@ -29,10 +29,13 @@ import com.google.gerrit.extensions.registration.RegistrationHandle; import com.google.gerrit.server.cache.CacheRemovalListener; import com.google.gerrit.server.events.EventGson; +import com.google.gerrit.server.git.WorkQueue; import com.google.gerrit.server.project.ProjectCacheImpl; import com.google.gson.Gson; import com.google.inject.AbstractModule; import com.google.inject.Inject; +import com.google.inject.Singleton; +import com.googlesource.gerrit.plugins.multisite.ExecutorProvider; import com.googlesource.gerrit.plugins.multisite.cache.CacheModule; import com.googlesource.gerrit.plugins.multisite.forwarder.events.CacheEvictionEvent; import com.googlesource.gerrit.plugins.multisite.forwarder.router.CacheEvictionEventRouter; @@ -43,7 +46,10 @@ import java.util.Map; import java.util.Set; import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.ScheduledThreadPoolExecutor; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; import org.eclipse.jgit.lib.Config; import org.junit.After; import org.junit.Before; @@ -69,7 +75,7 @@ @Override protected void configure() { install(new ForwarderModule()); - install(new CacheModule()); + install(new CacheModule(TestForwardingExecutorProvider.class)); install(new RouterModule()); install(new IndexModule()); SharedRefDbConfiguration sharedRefDbConfig = @@ -78,11 +84,44 @@ } } + @Singleton + public static class TestForwardingExecutorProvider extends ExecutorProvider { + private final ScheduledThreadPoolExecutor executor; + private final AtomicInteger executionsCounter; + + @Inject + protected TestForwardingExecutorProvider(WorkQueue workQueue) { + super(workQueue, 1, "test"); + executionsCounter = new AtomicInteger(); + executor = + new ScheduledThreadPoolExecutor(1) { + + @Override + public void execute(Runnable command) { + @SuppressWarnings("unused") + int ignored = executionsCounter.incrementAndGet(); + super.execute(command); + } + }; + } + + @Override + public ScheduledExecutorService get() { + return executor; + } + + public int executions() { + return executionsCounter.get(); + } + } + public static class CacheEvictionsTracker<K, V> implements CacheRemovalListener<K, V> { private final Map<String, Set<Object>> trackedEvictions; private final CountDownLatch allExpectedEvictionsArrived; + private final String trackedCacheName; - public CacheEvictionsTracker(int numExpectedEvictions) { + public CacheEvictionsTracker(String cacheName, int numExpectedEvictions) { + this.trackedCacheName = cacheName; allExpectedEvictionsArrived = new CountDownLatch(numExpectedEvictions); trackedEvictions = Maps.newHashMap(); } @@ -99,22 +138,24 @@ @Override public void onRemoval( String pluginName, String cacheName, RemovalNotification<K, V> notification) { - trackedEvictions.compute( - cacheName, - (k, v) -> { - if (v == null) { - return Sets.newHashSet(notification.getKey()); - } - v.add(notification.getKey()); - return v; - }); - allExpectedEvictionsArrived.countDown(); + if (cacheName.equals(trackedCacheName)) { + trackedEvictions.compute( + cacheName, + (k, v) -> { + if (v == null) { + return Sets.newHashSet(notification.getKey()); + } + v.add(notification.getKey()); + return v; + }); + allExpectedEvictionsArrived.countDown(); + } } } @Before public void startTrackingCacheEvictions() { - evictionsCacheTracker = new CacheEvictionsTracker<>(1); + evictionsCacheTracker = new CacheEvictionsTracker<>(ProjectCacheImpl.CACHE_NAME, 1); cacheEvictionRegistrationHandle = cacheRemovalListeners.add("gerrit", evictionsCacheTracker); } @@ -135,6 +176,36 @@ } @Test + @GerritConfig(name = "gerrit.instanceId", value = "testInstanceId") + @GerritConfig(name = "cache.threads", value = "0") + public void shouldNotForwardProjectCacheEvictionsWhenEventIsForwarded() throws Exception { + TestForwardingExecutorProvider cacheForwarder = + plugin.getSysInjector().getInstance(TestForwardingExecutorProvider.class); + Context.setForwardedEvent(true); + projectCache.evict(allProjects); + + evictionsCacheTracker.waitForExpectedEvictions(); + assertThat(evictionsCacheTracker.trackedEvictionsFor(ProjectCacheImpl.CACHE_NAME)) + .contains(allProjects); + + assertThat(cacheForwarder.executions()).isEqualTo(0); + } + + @Test + @GerritConfig(name = "gerrit.instanceId", value = "testInstanceId") + public void shouldForwardProjectCacheEvictions() throws Exception { + TestForwardingExecutorProvider cacheForwarder = + plugin.getSysInjector().getInstance(TestForwardingExecutorProvider.class); + projectCache.evict(allProjects); + + evictionsCacheTracker.waitForExpectedEvictions(); + assertThat(evictionsCacheTracker.trackedEvictionsFor(ProjectCacheImpl.CACHE_NAME)) + .contains(allProjects); + + assertThat(cacheForwarder.executions()).isEqualTo(1); + } + + @Test @GerritConfig(name = "gerrit.instanceId", value = "instance-id") public void shouldEvictProjectCacheWithSlash() throws Exception { ProjectInput in = new ProjectInput();
diff --git a/src/test/java/com/googlesource/gerrit/plugins/multisite/forwarder/ForwardedIndexChangeHandlerTest.java b/src/test/java/com/googlesource/gerrit/plugins/multisite/forwarder/ForwardedIndexChangeHandlerTest.java index b827cdf..2c4b157 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/multisite/forwarder/ForwardedIndexChangeHandlerTest.java +++ b/src/test/java/com/googlesource/gerrit/plugins/multisite/forwarder/ForwardedIndexChangeHandlerTest.java
@@ -87,7 +87,7 @@ public void setUp() throws Exception { when(ctxMock.open()).thenReturn(manualRequestContextMock); id = Change.id(TEST_CHANGE_NUMBER); - change = new Change(null, id, null, null, TimeUtil.nowTs()); + change = new Change(null, id, null, null, TimeUtil.now()); when(changeNotes.getChange()).thenReturn(change); when(changeCheckerFactoryMock.create(any())).thenReturn(changeCheckerAbsentMock); when(configurationMock.index()).thenReturn(index);
diff --git a/src/test/java/com/googlesource/gerrit/plugins/multisite/index/IndexEventHandlerTest.java b/src/test/java/com/googlesource/gerrit/plugins/multisite/index/IndexEventHandlerTest.java index ae5851d..7e27eeb 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/multisite/index/IndexEventHandlerTest.java +++ b/src/test/java/com/googlesource/gerrit/plugins/multisite/index/IndexEventHandlerTest.java
@@ -15,14 +15,26 @@ package com.googlesource.gerrit.plugins.multisite.index; import static com.googlesource.gerrit.plugins.replication.pull.api.PullReplicationEndpoints.APPLY_OBJECT_API_ENDPOINT; -import static org.mockito.Mockito.anyString; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.any; import static org.mockito.Mockito.lenient; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoInteractions; +import static org.mockito.Mockito.when; import com.google.common.util.concurrent.MoreExecutors; import com.google.gerrit.extensions.registration.DynamicSet; +import com.google.gerrit.server.util.OneOffRequestContext; +import com.google.gerrit.server.util.RequestContext; +import com.google.gerrit.server.util.ThreadLocalRequestContext; +import com.googlesource.gerrit.plugins.multisite.Configuration; import com.googlesource.gerrit.plugins.multisite.forwarder.Context; import com.googlesource.gerrit.plugins.multisite.forwarder.IndexEventForwarder; +import com.googlesource.gerrit.plugins.multisite.forwarder.events.ChangeIndexEvent; +import java.io.IOException; +import java.util.Optional; +import java.util.function.Consumer; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -33,21 +45,34 @@ public class IndexEventHandlerTest { private static final String INSTANCE_ID = "instance-id"; + private static final String PROJECT_NAME = "test_project"; + private static final int CHANGE_ID = 1; private IndexEventHandler eventHandler; @Mock private IndexEventForwarder forwarder; @Mock private ChangeCheckerImpl.Factory changeChecker; + @Mock private ChangeChecker changeCheckerMock; + @Mock private RequestContext mockCtx; + + private CurrentRequestContext currCtx = + new CurrentRequestContext(null, null, null) { + @Override + public void onlyWithContext(Consumer<RequestContext> body) { + body.accept(mockCtx); + } + }; @Before - public void setUp() { + public void setUp() throws IOException { eventHandler = new IndexEventHandler( MoreExecutors.directExecutor(), asDynamicSet(forwarder), changeChecker, new TestGroupChecker(true), - INSTANCE_ID); + INSTANCE_ID, + currCtx); } private DynamicSet<IndexEventForwarder> asDynamicSet(IndexEventForwarder forwarder) { @@ -62,17 +87,54 @@ String currentThreadName = Thread.currentThread().getName(); try { Thread.currentThread().setName("pull-replication~" + APPLY_OBJECT_API_ENDPOINT); - int changeId = 1; Context.setForwardedEvent(false); lenient() .when(changeChecker.create(anyString())) .thenThrow( new IllegalStateException("Change indexing event should have not been triggered")); - eventHandler.onChangeIndexed("test_project", changeId); + eventHandler.onChangeIndexed(PROJECT_NAME, CHANGE_ID); verifyNoInteractions(changeChecker); } finally { Thread.currentThread().setName(currentThreadName); } } + + @Test + public void shouldNotForwardIndexChangeWhenContextIsMissingAndForcedIndexingDisabled() + throws Exception { + eventHandler = createIndexEventHandler(changeChecker, false); + eventHandler.onChangeIndexed(PROJECT_NAME, CHANGE_ID); + verifyNoInteractions(changeChecker); + verifyNoInteractions(forwarder); + } + + @Test + public void shouldForwardIndexChangeWhenContextIsMissingAndForcedIndexingEnabled() + throws Exception { + when(changeChecker.create(any())).thenReturn(changeCheckerMock); + when(changeCheckerMock.newIndexEvent(PROJECT_NAME, CHANGE_ID, false)) + .thenReturn(Optional.of(new ChangeIndexEvent(PROJECT_NAME, CHANGE_ID, false, INSTANCE_ID))); + eventHandler = createIndexEventHandler(changeChecker, true); + eventHandler.onChangeIndexed(PROJECT_NAME, CHANGE_ID); + verify(changeCheckerMock).newIndexEvent(PROJECT_NAME, CHANGE_ID, false); + verify(forwarder).index(any(), any()); + } + + private IndexEventHandler createIndexEventHandler( + ChangeCheckerImpl.Factory changeChecker, boolean synchronizeForced) { + ThreadLocalRequestContext threadLocalCtxMock = mock(ThreadLocalRequestContext.class); + OneOffRequestContext oneOffCtxMock = mock(OneOffRequestContext.class); + Configuration cfgMock = mock(Configuration.class); + Configuration.Index cfgIndex = mock(Configuration.Index.class); + when(cfgMock.index()).thenReturn(cfgIndex); + when(cfgIndex.synchronizeForced()).thenReturn(synchronizeForced); + return new IndexEventHandler( + MoreExecutors.directExecutor(), + asDynamicSet(forwarder), + changeChecker, + new TestGroupChecker(true), + INSTANCE_ID, + new CurrentRequestContext(threadLocalCtxMock, cfgMock, oneOffCtxMock)); + } }
diff --git a/src/test/java/com/googlesource/gerrit/plugins/multisite/validation/ProjectVersionRefUpdateTest.java b/src/test/java/com/googlesource/gerrit/plugins/multisite/validation/ProjectVersionRefUpdateTest.java index fbf8ba0..e2e3113 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/multisite/validation/ProjectVersionRefUpdateTest.java +++ b/src/test/java/com/googlesource/gerrit/plugins/multisite/validation/ProjectVersionRefUpdateTest.java
@@ -24,15 +24,13 @@ import static org.mockito.Mockito.atMost; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.verifyZeroInteractions; +import static org.mockito.Mockito.verifyNoInteractions; import static org.mockito.Mockito.when; import com.gerritforge.gerrit.globalrefdb.validation.SharedRefDatabaseWrapper; -import com.google.common.base.Suppliers; import com.google.gerrit.entities.Project; -import com.google.gerrit.entities.RefNames; -import com.google.gerrit.server.data.RefUpdateAttribute; -import com.google.gerrit.server.events.RefUpdatedEvent; +import com.google.gerrit.extensions.common.AccountInfo; +import com.google.gerrit.extensions.events.GitBatchRefUpdateListener; import com.google.gerrit.server.extensions.events.GitReferenceUpdated; import com.google.gerrit.server.project.ProjectConfig; import com.google.gerrit.testing.InMemoryRepositoryManager; @@ -44,6 +42,7 @@ import java.io.UnsupportedEncodingException; import java.nio.charset.StandardCharsets; import java.util.Optional; +import java.util.Set; import org.eclipse.jgit.errors.LargeObjectException; import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository; import org.eclipse.jgit.junit.TestRepository; @@ -51,6 +50,7 @@ import org.eclipse.jgit.lib.ObjectLoader; import org.eclipse.jgit.lib.Ref; import org.eclipse.jgit.revwalk.RevCommit; +import org.eclipse.jgit.transport.ReceiveCommand; import org.junit.Before; import org.junit.Rule; import org.junit.Test; @@ -61,11 +61,14 @@ @RunWith(MockitoJUnitRunner.class) public class ProjectVersionRefUpdateTest implements RefFixture { - private static final String DEFAULT_INSTANCE_ID = "instance-id"; + private static final int A_TEST_ACCOUNT_ID = 1; + private static final GitReferenceUpdated.UpdatedRef A_TEST_UPDATED_REF = + new GitReferenceUpdated.UpdatedRef( + A_TEST_REF_NAME, ObjectId.zeroId(), ObjectId.zeroId(), ReceiveCommand.Type.UPDATE); @Rule public InMemoryTestEnvironment testEnvironment = new InMemoryTestEnvironment(); - @Mock RefUpdatedEvent refUpdatedEvent; + @Mock GitBatchRefUpdateListener.Event refUpdatedEvent; @Mock SharedRefDatabaseWrapper sharedRefDb; @Mock GitReferenceUpdated gitReferenceUpdated; @Mock ProjectVersionLogger verLogger; @@ -78,7 +81,6 @@ @Before public void setUp() throws Exception { - refUpdatedEvent.instanceId = DEFAULT_INSTANCE_ID; InMemoryRepository inMemoryRepo = repoManager.createRepository(A_TEST_PROJECT_NAME_KEY); project = projectConfigFactory.create(A_TEST_PROJECT_NAME_KEY); project.load(inMemoryRepo); @@ -95,12 +97,11 @@ .thenReturn(Optional.of("" + (masterCommit.getCommitTime() - 1))); when(sharedRefDb.compareAndPut(any(Project.NameKey.class), any(String.class), any(), any())) .thenReturn(true); - when(refUpdatedEvent.getProjectNameKey()).thenReturn(A_TEST_PROJECT_NAME_KEY); - when(refUpdatedEvent.getRefName()).thenReturn(A_TEST_REF_NAME); + when(refUpdatedEvent.getProjectName()).thenReturn(A_TEST_PROJECT_NAME); + when(refUpdatedEvent.getRefNames()).thenReturn(Set.of(A_TEST_REF_NAME)); - new ProjectVersionRefUpdateImpl( - repoManager, sharedRefDb, gitReferenceUpdated, verLogger, DEFAULT_INSTANCE_ID) - .onEvent(refUpdatedEvent); + new ProjectVersionRefUpdateImpl(repoManager, sharedRefDb, gitReferenceUpdated, verLogger) + .onGitBatchRefUpdate(refUpdatedEvent); Ref ref = repo.getRepository().findRef(MULTI_SITE_VERSIONING_REF); @@ -117,19 +118,47 @@ } @Test + public void producerShouldUpdateProjectVersionOnceUponMultipleRefUpdatedEvent() { + when(sharedRefDb.compareAndPut(any(Project.NameKey.class), any(String.class), any(), any())) + .thenReturn(true); + when(refUpdatedEvent.getProjectName()).thenReturn(A_TEST_PROJECT_NAME); + when(refUpdatedEvent.getRefNames()) + .thenReturn(Set.of(A_TEST_REF_NAME, A_REF_NAME_OF_A_PATCHSET)); + + new ProjectVersionRefUpdateImpl(repoManager, sharedRefDb, gitReferenceUpdated, verLogger) + .onGitBatchRefUpdate(refUpdatedEvent); + + verify(sharedRefDb, atMost(1)) + .compareAndPut(any(Project.NameKey.class), any(Ref.class), any(ObjectId.class)); + } + + @Test + public void producerShouldUpdateProjectVersionOnceUponMultipleRefWithOneMetaUpdatedEvent() { + when(sharedRefDb.compareAndPut(any(Project.NameKey.class), any(String.class), any(), any())) + .thenReturn(true); + when(refUpdatedEvent.getProjectName()).thenReturn(A_TEST_PROJECT_NAME); + when(refUpdatedEvent.getRefNames()) + .thenReturn(Set.of(A_TEST_REF_NAME, MULTI_SITE_VERSIONING_REF)); + + new ProjectVersionRefUpdateImpl(repoManager, sharedRefDb, gitReferenceUpdated, verLogger) + .onGitBatchRefUpdate(refUpdatedEvent); + + verify(sharedRefDb, atMost(1)) + .compareAndPut(any(Project.NameKey.class), any(Ref.class), any(ObjectId.class)); + } + + @Test public void producerShouldUsePutInsteadOfCompareAndPutWhenExtendedGlobalRefDb() throws IOException { when(sharedRefDb.isSetOperationSupported()).thenReturn(true); - RefUpdatedEvent refUpdatedEvent = new RefUpdatedEvent(); - refUpdatedEvent.instanceId = DEFAULT_INSTANCE_ID; - RefUpdateAttribute refUpdatedAttribute = new RefUpdateAttribute(); - refUpdatedAttribute.project = A_TEST_PROJECT_NAME_KEY.get(); - refUpdatedAttribute.refName = A_TEST_REF_NAME; - refUpdatedEvent.refUpdate = Suppliers.memoize(() -> refUpdatedAttribute); + GitBatchRefUpdateListener.Event refUpdatedEvent = + new GitReferenceUpdated.GitBatchRefUpdateEvent( + A_TEST_PROJECT_NAME_KEY, + Set.of(A_TEST_UPDATED_REF), + new AccountInfo(A_TEST_ACCOUNT_ID)); - new ProjectVersionRefUpdateImpl( - repoManager, sharedRefDb, gitReferenceUpdated, verLogger, DEFAULT_INSTANCE_ID) - .onEvent(refUpdatedEvent); + new ProjectVersionRefUpdateImpl(repoManager, sharedRefDb, gitReferenceUpdated, verLogger) + .onGitBatchRefUpdate(refUpdatedEvent); Ref ref = repo.getRepository().findRef(MULTI_SITE_VERSIONING_REF); @@ -160,13 +189,12 @@ .thenReturn(Optional.of("" + (masterCommit.getCommitTime() - 1))); when(sharedRefDb.compareAndPut(any(Project.NameKey.class), any(String.class), any(), any())) .thenReturn(true); - when(refUpdatedEvent.getProjectNameKey()).thenReturn(A_TEST_PROJECT_NAME_KEY); - when(refUpdatedEvent.getRefName()).thenReturn(A_TEST_REF_NAME); + when(refUpdatedEvent.getProjectName()).thenReturn(A_TEST_PROJECT_NAME); + when(refUpdatedEvent.getRefNames()).thenReturn(Set.of(A_TEST_REF_NAME)); ProjectVersionRefUpdateImpl projectVersion = - new ProjectVersionRefUpdateImpl( - repoManager, sharedRefDb, gitReferenceUpdated, verLogger, DEFAULT_INSTANCE_ID); - projectVersion.onEvent(refUpdatedEvent); + new ProjectVersionRefUpdateImpl(repoManager, sharedRefDb, gitReferenceUpdated, verLogger); + projectVersion.onGitBatchRefUpdate(refUpdatedEvent); Ref ref = repo.getRepository().findRef(MULTI_SITE_VERSIONING_REF); @@ -197,12 +225,11 @@ when(sharedRefDb.compareAndPut(any(Project.NameKey.class), any(String.class), any(), any())) .thenReturn(true); - when(refUpdatedEvent.getProjectNameKey()).thenReturn(A_TEST_PROJECT_NAME_KEY); - when(refUpdatedEvent.getRefName()).thenReturn(A_TEST_REF_NAME); + when(refUpdatedEvent.getProjectName()).thenReturn(A_TEST_PROJECT_NAME); + when(refUpdatedEvent.getRefNames()).thenReturn(Set.of(A_TEST_REF_NAME)); - new ProjectVersionRefUpdateImpl( - repoManager, sharedRefDb, gitReferenceUpdated, verLogger, DEFAULT_INSTANCE_ID) - .onEvent(refUpdatedEvent); + new ProjectVersionRefUpdateImpl(repoManager, sharedRefDb, gitReferenceUpdated, verLogger) + .onGitBatchRefUpdate(refUpdatedEvent); Ref ref = repo.getRepository().findRef(MULTI_SITE_VERSIONING_REF); @@ -218,68 +245,24 @@ verify(verLogger).log(A_TEST_PROJECT_NAME_KEY, storedVersion, 0); } - @Test - public void producerShouldNotUpdateProjectVersionUponSequenceRefUpdatedEvent() throws Exception { - producerShouldNotUpdateProjectVersionUponMagicRefUpdatedEvent(RefNames.REFS_SEQUENCES); - } - - @Test - public void producerShouldNotUpdateProjectVersionUponStarredChangesRefUpdatedEvent() - throws Exception { - producerShouldNotUpdateProjectVersionUponMagicRefUpdatedEvent(RefNames.REFS_STARRED_CHANGES); - } - private long readLongObject(ObjectLoader loader) throws LargeObjectException, UnsupportedEncodingException { String boutString = new String(loader.getBytes(), StandardCharsets.UTF_8.name()); return Long.parseLong(boutString); } - private void producerShouldNotUpdateProjectVersionUponMagicRefUpdatedEvent(String magicRefPrefix) - throws Exception { - String magicRefName = magicRefPrefix + "/foo"; - when(refUpdatedEvent.getProjectNameKey()).thenReturn(A_TEST_PROJECT_NAME_KEY); - when(refUpdatedEvent.getRefName()).thenReturn(magicRefName); - repo.branch(magicRefName).commit().create(); - - new ProjectVersionRefUpdateImpl( - repoManager, sharedRefDb, gitReferenceUpdated, verLogger, DEFAULT_INSTANCE_ID) - .onEvent(refUpdatedEvent); - - Ref ref = repo.getRepository().findRef(MULTI_SITE_VERSIONING_REF); - assertThat(ref).isNull(); - - verifyZeroInteractions(verLogger); - } - - @Test - public void producerShouldNotUpdateProjectVersionUponForwardedRefUpdatedEvent() - throws IOException { - refUpdatedEvent.instanceId = "instance-id-2"; - - new ProjectVersionRefUpdateImpl( - repoManager, sharedRefDb, gitReferenceUpdated, verLogger, DEFAULT_INSTANCE_ID) - .onEvent(refUpdatedEvent); - - Ref ref = repo.getRepository().findRef(MULTI_SITE_VERSIONING_REF); - assertThat(ref).isNull(); - - verifyZeroInteractions(verLogger); - } - @Test public void shouldNotUpdateProjectVersionWhenProjectDoesntExist() throws IOException { - when(refUpdatedEvent.getProjectNameKey()).thenReturn(Project.nameKey("aNonExistentProject")); - when(refUpdatedEvent.getRefName()).thenReturn(A_TEST_REF_NAME); + when(refUpdatedEvent.getProjectName()).thenReturn("aNonExistentProject"); + when(refUpdatedEvent.getRefNames()).thenReturn(Set.of(A_TEST_REF_NAME)); - new ProjectVersionRefUpdateImpl( - repoManager, sharedRefDb, gitReferenceUpdated, verLogger, DEFAULT_INSTANCE_ID) - .onEvent(refUpdatedEvent); + new ProjectVersionRefUpdateImpl(repoManager, sharedRefDb, gitReferenceUpdated, verLogger) + .onGitBatchRefUpdate(refUpdatedEvent); Ref ref = repo.getRepository().findRef(MULTI_SITE_VERSIONING_REF); assertThat(ref).isNull(); - verifyZeroInteractions(verLogger); + verifyNoInteractions(verLogger); } @Test @@ -288,8 +271,7 @@ .thenReturn(Optional.of("123")); Optional<Long> version = - new ProjectVersionRefUpdateImpl( - repoManager, sharedRefDb, gitReferenceUpdated, verLogger, DEFAULT_INSTANCE_ID) + new ProjectVersionRefUpdateImpl(repoManager, sharedRefDb, gitReferenceUpdated, verLogger) .getProjectRemoteVersion(A_TEST_PROJECT_NAME); assertThat(version.isPresent()).isTrue();