Merge branch 'stable-3.10' * stable-3.10: DRY out shouldNotBeTrackedAnymoreOnGlobalRefDb Ignore refs/multi-site/version on global-refdb in push replication filter Make the local test environment more realistic Make the local test environment more realistic Add acceptance test for change up-to-date checker Move replication.jar and delete-project.jar to /lib for testing Indexing retries: lower normal condition logs to debug Check for existence of change's target SHA1 for reindexing Do not remove replication.jar from multi-site test sites Update local setup script to stable-3.10 Adapt to replication api changes on stable-3.10 Adapt BrokerApiWrapper to the newest BrokerApi interface Avoid duplicate indexing tasks for the same id Refactor help information with additional detail for setup.sh script When enabled use draft comments for change up-to-date calculation Ignore new project created events which originate from a different node Do not propagate internal indexing operations Update the refs/multi-site/version/value once per batch-refupdate Update refs/multi-site/version for starred changes and sequences Remove dead code when updating project version meta-ref Fix merge Ib6aa8adbd causing 2x updates of refs/multi-site/version/* Fix target branch for the local environment Do not take draft comments into account for change index up-to-date Update Kafka container to bitnami/kafka:3.6.0 Remove unused docker compose Update Zookeeper container to bitnami/zookeeper:3.8.3 Remove double-binding of the ProjectVersionRefUpdateImpl to EventListener Enhance indexing logging Specify cache.threads = 0 in the multi-site config documentation Change-Id: Ic3b8a892b6481f32391b592b292fe38a8baab023
diff --git a/Jenkinsfile b/Jenkinsfile index 6ef43af..be0b4fc 100644 --- a/Jenkinsfile +++ b/Jenkinsfile
@@ -1,4 +1,4 @@ pluginPipeline(formatCheckId: 'gerritforge:multi-site-format-47168e90078b0b3f11401610930e82830e76bff7', buildCheckId: 'gerritforge:multi-site-47168e90078b0b3f11401610930e82830e76bff7', - extraPlugins: [ 'pull-replication' ], + extraPlugins: [ 'pull-replication', 'healthcheck' ], extraModules: [ 'events-broker', 'global-refdb' ])
diff --git a/docker-compose.kafka-broker.yaml b/docker-compose.kafka-broker.yaml deleted file mode 100644 index d3fc713..0000000 --- a/docker-compose.kafka-broker.yaml +++ /dev/null
@@ -1,14 +0,0 @@ -version: '3' -services: - zookeeper: - image: wurstmeister/zookeeper:latest - ports: - - "2181:2181" - kafka: - image: wurstmeister/kafka:2.13-2.6.3 - ports: - - "9092:9092" - environment: - KAFKA_ADVERTISED_HOST_NAME: 127.0.0.1 - KAFKA_CREATE_TOPICS: "gerrit:1:1" - KAFKA_ZOOKEEPER_CONNECT: zookeeper:2181
diff --git a/e2e-tests/docker-compose-kafka.yaml b/e2e-tests/docker-compose-kafka.yaml index addb8d5..b02b58d 100644 --- a/e2e-tests/docker-compose-kafka.yaml +++ b/e2e-tests/docker-compose-kafka.yaml
@@ -1,12 +1,11 @@ version: '3' services: kafka: - image: wurstmeister/kafka:2.13-2.6.3 + image: bitnami/kafka:3.6.0 depends_on: - zookeeper environment: - KAFKA_ADVERTISED_HOST_NAME: kafka - KAFKA_ZOOKEEPER_CONNECT: zookeeper:2181 + - KAFKA_CFG_ZOOKEEPER_CONNECT=zookeeper:2181 gerrit1: depends_on:
diff --git a/e2e-tests/docker-compose.yaml b/e2e-tests/docker-compose.yaml index 3abb757..2ac7b06 100644 --- a/e2e-tests/docker-compose.yaml +++ b/e2e-tests/docker-compose.yaml
@@ -1,7 +1,9 @@ version: '3' services: zookeeper: - image: wurstmeister/zookeeper:latest + image: bitnami/zookeeper:3.8.3 + environment: + ALLOW_ANONYMOUS_LOGIN: "true" gerrit1: image: gerritcodereview/gerrit:${GERRIT_IMAGE}
diff --git a/setup_local_env/configs/replication.config b/setup_local_env/configs/replication.config index 1c2aac4..ff0c248 100644 --- a/setup_local_env/configs/replication.config +++ b/setup_local_env/configs/replication.config
@@ -9,6 +9,7 @@ createMissingRepositories = true replicateProjectDeletions = true replicateHiddenProjects = true + threads = 5 [gerrit] autoReload = true replicateOnStartup = false @@ -17,5 +18,7 @@ maxRetries = 5 useCGitClient = false consumeStreamEvents = false + eventBrokerTopic = gerrit_stream + eventBrokerGroupId = pullreplication_$INSTANCE_ID syncRefs="ALL REFS ASYNC" maxApiPayloadSize=40000
diff --git a/setup_local_env/docker-compose-core.yaml b/setup_local_env/docker-compose-core.yaml index dab168d..f8ef318 100644 --- a/setup_local_env/docker-compose-core.yaml +++ b/setup_local_env/docker-compose-core.yaml
@@ -1,10 +1,12 @@ version: '3' services: zookeeper: - image: wurstmeister/zookeeper:latest + image: bitnami/zookeeper:3.8.3 ports: - "2181:2181" container_name: zk_test_node + environment: + ALLOW_ANONYMOUS_LOGIN: "true" prometheus: container_name: prometheus_test_node image: prom/prometheus:v2.16.0
diff --git a/setup_local_env/docker-compose-kafka.yaml b/setup_local_env/docker-compose-kafka.yaml index 2a5b0fc..70fda58 100644 --- a/setup_local_env/docker-compose-kafka.yaml +++ b/setup_local_env/docker-compose-kafka.yaml
@@ -1,13 +1,13 @@ version: '3' services: kafka: - image: wurstmeister/kafka:2.13-2.6.3 + image: bitnami/kafka:3.6.0 ports: - "9092:9092" container_name: kafka_test_node environment: - KAFKA_ADVERTISED_HOST_NAME: 127.0.0.1 - KAFKA_ZOOKEEPER_CONNECT: zookeeper:2181 + - KAFKA_CFG_ADVERTISED_LISTENERS=PLAINTEXT://localhost:9092 + - KAFKA_CFG_ZOOKEEPER_CONNECT=zookeeper:2181 networks: - setup_local_env_default networks:
diff --git a/setup_local_env/setup.sh b/setup_local_env/setup.sh index 0e84603..2673d3b 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.8 +GERRIT_BRANCH=stable-3.10 GERRIT_CI=https://gerrit-ci.gerritforge.com/view/Plugins-$GERRIT_BRANCH/job LAST_BUILD=lastSuccessfulBuild/artifact/bazel-bin/plugins @@ -234,46 +234,62 @@ { echo >&2 "Cannot download $artifact_name $prefix: Check internet connection. Aborting"; exit 1; } } +function show_help { + echo "Usage: $0 [--option ] " + echo + echo "[--release-war-file] Location to release.war file;" \ + "default: 'bazel-bin/release.war'" + echo "[--multisite-lib-file] Location to lib multi-site.jar file;" \ + "default: 'bazel-bin/plugins/multi-site/multi-site.jar'" + echo "[--eventsbroker-lib-file] Location to lib events-broker.jar file;" \ + "default: 'bazel-bin/plugins/events-broker/events-broker.jar'" + echo "[--globalrefdb-lib-file] Location to lib global-refdb.jar file;" \ + "default: 'bazel-bin/plugins/global-refdb/global-refdb.jar'" + echo + echo "[--new-deployment] Cleans up previous gerrit deployment and re-installs it. default true" + echo "[--get-websession-plugin] Download websession-broker plugin from CI lastSuccessfulBuild; default true" + echo "[--deployment-location] Base location for the test deployment; default /tmp" + echo + echo "[--gerrit-canonical-host] The default host for Gerrit to be accessed through; default localhost" + echo "[--gerrit-canonical-port] The default port for Gerrit to be accessed through; default 8080" + echo + echo "[--gerrit-ssh-advertised-port] Gerrit Instance 1 sshd port; default 29418" + echo + echo "[--gerrit1-httpd-port] Gerrit Instance 1 http port; default 18080" + echo "[--gerrit1-sshd-port] Gerrit Instance 1 sshd port; default 39418" + echo + echo "[--gerrit2-httpd-port] Gerrit Instance 2 http port; default 18081" + echo "[--gerrit2-sshd-port] Gerrit Instance 2 sshd port; default 49418" + echo + echo "[--replication-delay] Replication delay across the two instances in seconds" + echo + echo "[--just-cleanup-env] Cleans up previous deployment; default false" + echo + echo "[--enabled-https] Enabled https; default true" + echo + echo "[--broker-type] events broker type; 'kafka', 'kinesis' or 'gcloud-pubsub'; default 'kafka'" + echo + echo "[--sudo] run docker commands with sudo" + echo + echo "Note: Script should be run from the gerrit root source path to take advantage of local builds"\ + "for release.war, multi-site.jar, events-broker.jar, and global-refdb.jar" + echo + echo "Examples of usage: " + echo "Cleanup last install -> $0 --just-cleanup-env true" + echo "Install and startup multisite based on local artifacts (Gerrit and multi-site)-> $0" +} + +################################################################################ +### Startup while [ $# -ne 0 ] do case "$1" in "--help" ) - echo "Usage: sh $0 [--option $value]" - echo - echo "[--release-war-file] Location to release.war file" - echo "[--multisite-lib-file] Location to lib multi-site.jar file" - echo "[--eventsbroker-lib-file] Location to lib events-broker.jar file" - echo "[--globalrefdb-lib-file] Location to lib global-refdb.jar file" - echo - echo "[--new-deployment] Cleans up previous gerrit deployment and re-installs it. default true" - echo "[--get-websession-plugin] Download websession-broker plugin from CI lastSuccessfulBuild; default true" - echo "[--deployment-location] Base location for the test deployment; default /tmp" - echo - echo "[--gerrit-canonical-host] The default host for Gerrit to be accessed through; default localhost" - echo "[--gerrit-canonical-port] The default port for Gerrit to be accessed throug; default 8080" - echo - echo "[--gerrit-ssh-advertised-port] Gerrit Instance 1 sshd port; default 29418" - echo - echo "[--gerrit1-httpd-port] Gerrit Instance 1 http port; default 18080" - echo "[--gerrit1-sshd-port] Gerrit Instance 1 sshd port; default 39418" - echo - echo "[--gerrit2-httpd-port] Gerrit Instance 2 http port; default 18081" - echo "[--gerrit2-sshd-port] Gerrit Instance 2 sshd port; default 49418" - echo - echo "[--replication-delay] Replication delay across the two instances in seconds" - echo - echo "[--just-cleanup-env] Cleans up previous deployment; default false" - echo - echo "[--enabled-https] Enabled https; default true" - echo - echo "[--broker-type] events broker type; 'kafka', 'kinesis' or 'gcloud-pubsub'. Default 'kafka'" - echo - echo "[--sudo] run docker commands with sudo" - echo + show_help exit 0 ;; "--new-deployment") - NEW_INSTALLATION=$2 + NEW_INSTALLATION=$2 shift shift ;; @@ -562,12 +578,12 @@ echo "Re-deploying configuration files" deploy_config_files $GERRIT_1_HOSTNAME $GERRIT_1_HTTPD_PORT $GERRIT_1_SSHD_PORT $GERRIT_2_HOSTNAME $GERRIT_2_HTTPD_PORT $GERRIT_2_SSHD_PORT -echo "Remove replication plugin from gerrit site 1" -rm $LOCATION_TEST_SITE_1/plugins/replication.jar +echo "Move replication and delete-project plugins to /lib on gerrit site 1" +mv $LOCATION_TEST_SITE_1/plugins/{replication,delete-project}.jar $LOCATION_TEST_SITE_1/lib/. echo "Starting gerrit site 1" $LOCATION_TEST_SITE_1/bin/gerrit.sh restart -echo "Remove replication plugin from gerrit site 2" -rm $LOCATION_TEST_SITE_2/plugins/replication.jar +echo "Move replication plugin to /lib on gerrit site 2" +mv $LOCATION_TEST_SITE_2/plugins/{replication,delete-project}.jar $LOCATION_TEST_SITE_2/lib/. echo "Starting gerrit site 2" $LOCATION_TEST_SITE_2/bin/gerrit.sh restart
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/broker/BrokerApiWrapper.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/broker/BrokerApiWrapper.java index 9cab884..32fb0a5 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/multisite/broker/BrokerApiWrapper.java +++ b/src/main/java/com/googlesource/gerrit/plugins/multisite/broker/BrokerApiWrapper.java
@@ -16,6 +16,7 @@ import com.gerritforge.gerrit.eventbroker.BrokerApi; import com.gerritforge.gerrit.eventbroker.TopicSubscriber; +import com.gerritforge.gerrit.eventbroker.TopicSubscriberWithGroupId; import com.gerritforge.gerrit.eventbroker.log.MessageLogger; import com.google.common.base.Strings; import com.google.common.util.concurrent.FutureCallback; @@ -116,11 +117,21 @@ } @Override + public void receiveAsync(String topic, String groupId, Consumer<Event> consumer) { + apiDelegate.get().receiveAsync(topic, groupId, consumer); + } + + @Override public void disconnect() { apiDelegate.get().disconnect(); } @Override + public void disconnect(String topic, String groupId) { + apiDelegate.get().disconnect(topic, groupId); + } + + @Override public Set<TopicSubscriber> topicSubscribers() { return apiDelegate.get().topicSubscribers(); } @@ -129,4 +140,9 @@ public void replayAllEvents(String topic) { apiDelegate.get().replayAllEvents(topic); } + + @Override + public Set<TopicSubscriberWithGroupId> topicSubscribersWithGroupId() { + return apiDelegate.get().topicSubscribersWithGroupId(); + } }
diff --git a/src/main/java/com/googlesource/gerrit/plugins/multisite/cache/ProjectListUpdateHandler.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/cache/ProjectListUpdateHandler.java index 6a2c7a5..ad42ec6 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/multisite/cache/ProjectListUpdateHandler.java +++ b/src/main/java/com/googlesource/gerrit/plugins/multisite/cache/ProjectListUpdateHandler.java
@@ -25,11 +25,11 @@ import com.googlesource.gerrit.plugins.multisite.forwarder.ForwarderTask; import com.googlesource.gerrit.plugins.multisite.forwarder.ProjectListUpdateForwarder; import com.googlesource.gerrit.plugins.multisite.forwarder.events.ProjectListUpdateEvent; +import java.util.Objects; import java.util.concurrent.Executor; @Singleton public class ProjectListUpdateHandler implements NewProjectCreatedListener, ProjectDeletedListener { - private final DynamicSet<ProjectListUpdateForwarder> forwarders; private final Executor executor; private final String instanceId; @@ -47,7 +47,9 @@ @Override public void onNewProjectCreated( com.google.gerrit.extensions.events.NewProjectCreatedListener.Event event) { - process(event, false); + if (Objects.equals(instanceId, event.getInstanceId())) { + process(event, false); + } } @Override
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 53bcbc0..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,11 +16,10 @@ 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.Scopes; import com.google.inject.multibindings.OptionalBinder; import com.googlesource.gerrit.plugins.multisite.Configuration; import com.googlesource.gerrit.plugins.multisite.forwarder.events.EventTopic; @@ -37,8 +36,6 @@ @Override protected void configure() { - DynamicSet.bind(binder(), EventListener.class).to(ProjectVersionRefUpdateImpl.class); - bind(StreamEventPublisherConfig.class) .toInstance( new StreamEventPublisherConfig( @@ -50,11 +47,9 @@ OptionalBinder<ProjectVersionRefUpdate> projectVersionRefUpdateBinder = OptionalBinder.newOptionalBinder(binder(), ProjectVersionRefUpdate.class); if (configuration.getSharedRefDbConfiguration().getSharedRefDb().isEnabled()) { - DynamicSet.bind(binder(), EventListener.class).to(ProjectVersionRefUpdateImpl.class); - projectVersionRefUpdateBinder - .setBinding() - .to(ProjectVersionRefUpdateImpl.class) - .in(Scopes.SINGLETON); + DynamicSet.bind(binder(), GitBatchRefUpdateListener.class) + .to(ProjectVersionRefUpdateImpl.class); + projectVersionRefUpdateBinder.setBinding().to(ProjectVersionRefUpdateImpl.class); } } }
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 b3fb27c..15f7c13 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
@@ -56,34 +56,49 @@ @Override protected void doIndex(String id, Optional<ChangeIndexEvent> indexEvent) { - attemptToIndex(id, indexEvent, 0); + scheduleIndexing(id, indexEvent, this::indexIfConsistent); + } + + private void indexIfConsistent(String id) { + if (isChangeConsistent(id)) { + reindex(id); + } + } + + private boolean isChangeConsistent(String id) { + ChangeChecker checker = changeCheckerFactory.create(id); + Optional<ChangeNotes> changeNotes = checker.getChangeNotes(); + return changeNotes.isPresent() && checker.isChangeConsistent(); } @Override - protected void attemptToIndex(String id, Optional<ChangeIndexEvent> indexEvent, int retryCount) { + protected void attemptToIndex(String id) { ChangeChecker checker = changeCheckerFactory.create(id); Optional<ChangeNotes> changeNotes = checker.getChangeNotes(); boolean changeIsPresent = changeNotes.isPresent(); boolean changeIsConsistent = checker.isChangeConsistent(); if (changeIsPresent && changeIsConsistent) { - reindexAndCheckIsUpToDate(id, indexEvent, checker, retryCount); + reindexAndCheckIsUpToDate(id, checker); } else { + IndexingRetry retry = indexingRetryTaskMap.get(id); log.warn( "Change {} {} in local Git repository (event={}) after {} attempt(s)", id, !changeIsPresent ? "not present yet" : (changeIsConsistent ? "is" : "is not") + " consistent", - indexEvent, - retryCount); - if (!rescheduleIndex(id, indexEvent, retryCount + 1)) { + retry.getEvent(), + retry.getRetryNumber()); + + retry.incrementRetryNumber(); + if (!rescheduleIndex(id)) { log.error( "Change {} {} in the local Git repository (event={})", id, !changeIsPresent ? "could not be found" : (changeIsConsistent ? "was" : "was not") + " consistent", - indexEvent); + retry.getEvent()); } } }
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 c4906a9..d05405f 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
@@ -52,7 +52,7 @@ @Override protected void doIndex(String uuid, Optional<GroupIndexEvent> event) { - attemptToIndex(uuid, event, 0); + scheduleIndexing(uuid, event, this::reindex); } @Override @@ -66,9 +66,8 @@ } @Override - protected void attemptToIndex( - String uuid, Optional<GroupIndexEvent> groupIndexEvent, int retryCount) { - reindexAndCheckIsUpToDate(uuid, groupIndexEvent, groupChecker, retryCount); + protected void attemptToIndex(String uuid) { + reindexAndCheckIsUpToDate(uuid, groupChecker); } @Override
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 3787a80..2054f10 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
@@ -52,7 +52,7 @@ @Override protected void doIndex(String projectName, Optional<ProjectIndexEvent> event) { - attemptToIndex(projectName, event, 0); + scheduleIndexing(projectName, event, this::reindex); } @Override @@ -66,8 +66,8 @@ } @Override - protected void attemptToIndex(String id, Optional<ProjectIndexEvent> indexEvent, int retryCount) { - reindexAndCheckIsUpToDate(id, indexEvent, projectChecker, retryCount); + protected void attemptToIndex(String id) { + reindexAndCheckIsUpToDate(id, projectChecker); } @Override
diff --git a/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/ForwardedIndexingHandlerWithRetries.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/ForwardedIndexingHandlerWithRetries.java index 5c64431..d135e39 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/ForwardedIndexingHandlerWithRetries.java +++ b/src/main/java/com/googlesource/gerrit/plugins/multisite/forwarder/ForwardedIndexingHandlerWithRetries.java
@@ -19,10 +19,13 @@ import com.googlesource.gerrit.plugins.multisite.Configuration; import com.googlesource.gerrit.plugins.multisite.forwarder.events.IndexEvent; import com.googlesource.gerrit.plugins.multisite.index.UpToDateChecker; +import java.util.Map; import java.util.Optional; +import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.Future; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.TimeUnit; +import java.util.function.Consumer; /** * Base class to handle forwarded indexing. This class is meant to be extended by classes used on @@ -37,6 +40,7 @@ private final int maxTries; private final ScheduledExecutorService indexExecutor; protected final OneOffRequestContext oneOffCtx; + protected final Map<T, IndexingRetry> indexingRetryTaskMap = new ConcurrentHashMap<>(); ForwardedIndexingHandlerWithRetries( ScheduledExecutorService indexExecutor, @@ -55,22 +59,34 @@ protected abstract String indexName(); - protected abstract void attemptToIndex(T id, Optional<E> indexEvent, int retryCount); + protected abstract void attemptToIndex(T id); - protected boolean rescheduleIndex(T id, Optional<E> indexEvent, int retryCount) { - if (retryCount > maxTries) { + protected boolean rescheduleIndex(T id) { + IndexingRetry retry = indexingRetryTaskMap.get(id); + if (retry == null) { + log.debug( + "{} {} successfully indexed by different task, rescheduling isn't needed", + indexName(), + id); + return true; + } + if (retry.getRetryNumber() > maxTries) { log.error( "{} {} could not be indexed after {} retries. {} index could be stale.", indexName(), id, - retryCount, + retry.getRetryNumber(), indexName()); + if (!indexingRetryTaskMap.remove(id, retry)) { + log.debug( + "{} {} not removed from retry map because of racy addition of a new retry indexing retry"); + } return false; } log.warn( "Retrying for the #{} time to index {} {} after {} msecs", - retryCount, + retry.getRetryNumber(), indexName(), id, retryInterval); @@ -80,7 +96,7 @@ () -> { try (ManualRequestContext ctx = oneOffCtx.open()) { Context.setForwardedEvent(true); - attemptToIndex(id, indexEvent, retryCount); + attemptToIndex(id); } catch (Exception e) { log.warn("{} {} could not be indexed", indexName(), id, e); } @@ -90,20 +106,66 @@ return true; } - public final void reindexAndCheckIsUpToDate( - T id, Optional<E> indexEvent, UpToDateChecker<E> upToDateChecker, int retryCount) { - reindex(id); - - if (!upToDateChecker.isUpToDate(indexEvent)) { - log.warn("{} {} is not up-to-date. Rescheduling", indexName(), id); - rescheduleIndex(id, indexEvent, retryCount + 1); + public void scheduleIndexing(T id, Optional<E> event, Consumer<T> indexOnce) { + IndexingRetry retry = new IndexingRetry(event); + if (indexingRetryTaskMap.put(id, retry) != null) { + indexOnce.accept(id); + log.debug( + "Skipping indexing because there is already a running task for the specified id. Index name: {}, task id: {}", + indexName(), + id); return; } - if (retryCount > 0) { + attemptToIndex(id); + } + + public final void reindexAndCheckIsUpToDate(T id, UpToDateChecker<E> upToDateChecker) { + reindex(id); + IndexingRetry retry = indexingRetryTaskMap.get(id); + if (retry == null) { + log.warn("{} {} successfully indexed by different task", indexName(), id); + return; + } + if (!upToDateChecker.isUpToDate(retry.getEvent())) { + log.warn("{} {} is not up-to-date. Rescheduling", indexName(), id); + retry.incrementRetryNumber(); + rescheduleIndex(id); + return; + } + + if (retry.getRetryNumber() > 0) { log.warn( - "{} {} has been eventually indexed after {} attempt(s)", indexName(), id, retryCount); + "{} {} has been eventually indexed after {} attempt(s)", + indexName(), + id, + retry.getRetryNumber()); } else { log.debug("{} {} successfully indexed", indexName(), id); } + if (!indexingRetryTaskMap.remove(id, retry)) { + log.debug( + "{} {} not removed from retry map because of racy addition of a new retry indexing retry"); + } + } + + public class IndexingRetry { + private final Optional<E> event; + private int retryNumber = 0; + + public IndexingRetry(Optional<E> event) { + this.event = event; + } + + public int getRetryNumber() { + return retryNumber; + } + + public Optional<E> getEvent() { + return event; + } + + public void incrementRetryNumber() { + ++retryNumber; + } } }
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 4779258..f3fa2e3 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
@@ -20,18 +20,21 @@ import com.google.gerrit.server.DraftCommentsReader; import com.google.gerrit.server.change.ChangeFinder; import com.google.gerrit.server.config.GerritInstanceId; +import com.google.gerrit.server.config.GerritServerConfig; import com.google.gerrit.server.git.GitRepositoryManager; import com.google.gerrit.server.notedb.ChangeNotes; import com.google.gerrit.server.util.ManualRequestContext; import com.google.gerrit.server.util.OneOffRequestContext; import com.google.inject.Inject; +import com.google.inject.Module; import com.google.inject.assistedinject.Assisted; +import com.google.inject.assistedinject.FactoryModuleBuilder; 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; +import org.eclipse.jgit.lib.Config; import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.Ref; import org.eclipse.jgit.lib.Repository; @@ -47,6 +50,7 @@ private final String changeId; private final ChangeFinder changeFinder; private final String instanceId; + private final boolean enableDraftCommentEvents; private Optional<Long> computedChangeTs = Optional.empty(); private Optional<ChangeNotes> changeNotes = Optional.empty(); @@ -54,6 +58,12 @@ public ChangeChecker create(String changeId); } + public static Module module() { + return new FactoryModuleBuilder() + .implement(ChangeChecker.class, ChangeCheckerImpl.class) + .build(ChangeCheckerImpl.Factory.class); + } + @Inject public ChangeCheckerImpl( GitRepositoryManager gitRepoMgr, @@ -61,6 +71,7 @@ ChangeFinder changeFinder, OneOffRequestContext oneOffReqCtx, @GerritInstanceId String instanceId, + @GerritServerConfig Config config, @Assisted String changeId) { this.changeFinder = changeFinder; this.gitRepoMgr = gitRepoMgr; @@ -68,6 +79,8 @@ this.oneOffReqCtx = oneOffReqCtx; this.changeId = changeId; this.instanceId = instanceId; + this.enableDraftCommentEvents = + config.getBoolean("event", "stream-events", "enableDraftCommentEvents", false); } @Override @@ -108,8 +121,7 @@ .map( e -> (computedChangeTs.get() > e.eventCreatedOn) - || ((computedChangeTs.get() == e.eventCreatedOn) - && (Objects.equals(getBranchTargetSha(), e.targetSha)))) + || ((computedChangeTs.get() == e.eventCreatedOn) && repositoryHas(e.targetSha))) .orElse(true); } @@ -148,6 +160,15 @@ } } + private boolean repositoryHas(String targetSha) { + try (Repository repo = gitRepoMgr.openRepository(changeNotes.get().getProjectName())) { + return repo.parseCommit(ObjectId.fromString(targetSha)) != null; + } catch (IOException e) { + log.warn("Unable to find SHA1 {} for change {}", targetSha, changeId, e); + return false; + } + } + @Override public boolean isChangeConsistent() { Optional<ChangeNotes> notes = getChangeNotes(); @@ -177,20 +198,22 @@ } private Optional<Long> computeLastChangeTs() { - return getChangeNotes().map(notes -> getTsFromChangeAndDraftComments(notes)); + return getChangeNotes().map(this::getTsFromChangeAndDraftComments); } private long getTsFromChangeAndDraftComments(ChangeNotes notes) { Change change = notes.getChange(); Timestamp changeTs = Timestamp.from(change.getLastUpdatedOn()); - try { - for (HumanComment comment : - draftCommentsReader.getDraftsByChangeForAllAuthors(changeNotes.get())) { - Timestamp commentTs = comment.writtenOn; - changeTs = commentTs.after(changeTs) ? commentTs : changeTs; + if (enableDraftCommentEvents) { + try { + for (HumanComment comment : + draftCommentsReader.getDraftsByChangeForAllAuthors(changeNotes.get())) { + Timestamp commentTs = comment.writtenOn; + changeTs = commentTs.after(changeTs) ? commentTs : changeTs; + } + } catch (StorageException e) { + log.warn("Unable to access draft comments for change {}", change, e); } - } catch (StorageException e) { - log.warn("Unable to access draft comments for change {}", change, e); } return changeTs.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/index/IndexModule.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/index/IndexModule.java index 5f47371..0e7a54f 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/multisite/index/IndexModule.java +++ b/src/main/java/com/googlesource/gerrit/plugins/multisite/index/IndexModule.java
@@ -21,7 +21,6 @@ import com.google.gerrit.extensions.registration.DynamicSet; import com.google.gerrit.lifecycle.LifecycleModule; import com.google.gerrit.server.events.EventListener; -import com.google.inject.assistedinject.FactoryModuleBuilder; import java.util.concurrent.Executor; import java.util.concurrent.ScheduledExecutorService; @@ -43,9 +42,6 @@ bind(ProjectChecker.class).to(ProjectCheckerImpl.class); bind(GroupChecker.class).to(GroupCheckerImpl.class); - install( - new FactoryModuleBuilder() - .implement(ChangeChecker.class, ChangeCheckerImpl.class) - .build(ChangeCheckerImpl.Factory.class)); + install(ChangeCheckerImpl.module()); } }
diff --git a/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/AbstractMultisiteReplicationFilter.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/AbstractMultisiteReplicationFilter.java new file mode 100644 index 0000000..bf4a0f9 --- /dev/null +++ b/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/AbstractMultisiteReplicationFilter.java
@@ -0,0 +1,33 @@ +// Copyright (C) 2024 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.validation; + +import static com.googlesource.gerrit.plugins.multisite.validation.ProjectVersionRefUpdate.MULTI_SITE_VERSIONING_REF; + +abstract class AbstractMultisiteReplicationFilter { + + /* + * Since ac43a5f94c773c9db7a73d44035961d69d13fa53 the 'refs/multi-site/version' is + * not updated anymore on the global-refdb; however, the values stored already + * on the global-refdb could get in the way and prevent replication from happening + * as expected. + * + * Exclude the 'refs/multi-site/version' from local vs. global refdb checking + * pretending that the global-refdb for that ref did not exist. + */ + protected boolean shouldNotBeTrackedAnymoreOnGlobalRefDb(String ref) { + return MULTI_SITE_VERSIONING_REF.equals(ref); + } +}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/MultisiteReplicationFetchFilter.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/MultisiteReplicationFetchFilter.java index 3fc57ae..c5571c0 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/MultisiteReplicationFetchFilter.java +++ b/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/MultisiteReplicationFetchFilter.java
@@ -14,7 +14,6 @@ package com.googlesource.gerrit.plugins.multisite.validation; -import static com.googlesource.gerrit.plugins.multisite.validation.ProjectVersionRefUpdate.MULTI_SITE_VERSIONING_REF; import static com.googlesource.gerrit.plugins.replication.pull.PullReplicationLogger.repLog; import com.gerritforge.gerrit.globalrefdb.GlobalRefDbLockException; @@ -38,7 +37,8 @@ import org.eclipse.jgit.lib.Repository; @Singleton -public class MultisiteReplicationFetchFilter implements ReplicationFetchFilter { +public class MultisiteReplicationFetchFilter extends AbstractMultisiteReplicationFilter + implements ReplicationFetchFilter { private static final String ZERO_ID_NAME = ObjectId.zeroId().name(); private static final FluentLogger logger = FluentLogger.forEnclosingClass(); @@ -91,19 +91,6 @@ } } - /* - * Since ac43a5f94c773c9db7a73d44035961d69d13fa53 the 'refs/multi-site/version' is - * not updated anymore on the global-refdb; however, the values stored already - * on the global-refdb could get in the way and prevent replication from happening - * as expected. - * - * Exclude the 'refs/multi-site/version' from local vs. global refdb checking - * pretending that the global-refdb for that ref did not exist. - */ - private boolean shouldNotBeTrackedAnymoreOnGlobalRefDb(String ref) { - return MULTI_SITE_VERSIONING_REF.equals(ref); - } - /* If the ref to fetch has been set to all zeros on the global-refdb, it means * that whatever is the situation locally, we do not need to fetch it: * - If the remote still has it, fetching it will be useless because the global
diff --git a/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/MultisiteReplicationPushFilter.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/MultisiteReplicationPushFilter.java index 0aba056..5955e17 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/MultisiteReplicationPushFilter.java +++ b/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/MultisiteReplicationPushFilter.java
@@ -23,7 +23,7 @@ import com.google.inject.Inject; import com.google.inject.Singleton; import com.googlesource.gerrit.plugins.multisite.Configuration; -import com.googlesource.gerrit.plugins.replication.ReplicationPushFilter; +import com.googlesource.gerrit.plugins.replication.api.ReplicationPushFilter; import java.io.IOException; import java.util.Collections; import java.util.HashSet; @@ -40,7 +40,8 @@ import org.slf4j.LoggerFactory; @Singleton -public class MultisiteReplicationPushFilter implements ReplicationPushFilter { +public class MultisiteReplicationPushFilter extends AbstractMultisiteReplicationFilter + implements ReplicationPushFilter { private static final FluentLogger logger = FluentLogger.forEnclosingClass(); private static final String REF_META_SUFFIX = "/meta"; @@ -111,6 +112,10 @@ private Optional<RemoteRefUpdate> isUpToDateWithRetry( String projectName, Repository repository, RemoteRefUpdate refUpdate) { + if (shouldNotBeTrackedAnymoreOnGlobalRefDb(refUpdate.getSrcRef())) { + return Optional.of(refUpdate); + } + String ref = refUpdate.getSrcRef(); try { if (sharedRefDb.isUpToDate(
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 aa589b6..adbef1b 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
@@ -25,15 +25,12 @@ 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.gerrit.server.update.context.RefUpdateContext; import com.google.inject.Inject; +import com.google.inject.Singleton; import com.googlesource.gerrit.plugins.multisite.ProjectVersionLogger; import java.io.IOException; import java.nio.charset.StandardCharsets; @@ -45,7 +42,9 @@ import org.eclipse.jgit.lib.RefUpdate; import org.eclipse.jgit.lib.Repository; -public class ProjectVersionRefUpdateImpl implements EventListener, ProjectVersionRefUpdate { +@Singleton +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 +52,6 @@ private final GitRepositoryManager gitRepositoryManager; private final GitReferenceUpdated gitReferenceUpdated; private final ProjectVersionLogger verLogger; - private final String nodeInstanceId; protected final SharedRefDatabaseWrapper sharedRefDb; @@ -62,61 +60,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 +229,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", @@ -268,7 +247,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/java/com/googlesource/gerrit/plugins/multisite/validation/PushReplicationFilterModule.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/PushReplicationFilterModule.java index 8a8e826..77ac478 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/PushReplicationFilterModule.java +++ b/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/PushReplicationFilterModule.java
@@ -16,7 +16,7 @@ import com.google.gerrit.extensions.registration.DynamicItem; import com.google.inject.AbstractModule; -import com.googlesource.gerrit.plugins.replication.ReplicationPushFilter; +import com.googlesource.gerrit.plugins.replication.api.ReplicationPushFilter; public class PushReplicationFilterModule extends AbstractModule {
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/ProjectListUpdateHandlerTest.java b/src/test/java/com/googlesource/gerrit/plugins/multisite/cache/ProjectListUpdateHandlerTest.java index 3a4242c..ed1160d 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
@@ -61,10 +61,11 @@ } @Test - public void shouldForwardAddedProject() throws Exception { + public void shouldForwardAddedProjectForLocalEvent() { String projectName = "projectToAdd"; NewProjectCreatedListener.Event event = mock(NewProjectCreatedListener.Event.class); when(event.getProjectName()).thenReturn(projectName); + when(event.getInstanceId()).thenReturn(INSTANCE_ID); handler.onNewProjectCreated(event); verify(forwarder) .updateProjectList( @@ -73,6 +74,14 @@ } @Test + public void shouldNotForwardAddedProjectForRemoteEvent() { + NewProjectCreatedListener.Event event = mock(NewProjectCreatedListener.Event.class); + when(event.getInstanceId()).thenReturn("aRandomInstanceId"); + handler.onNewProjectCreated(event); + verifyNoInteractions(forwarder); + } + + @Test public void shouldForwardDeletedProject() throws Exception { String projectName = "projectToDelete"; ProjectDeletedListener.Event event = mock(ProjectDeletedListener.Event.class); @@ -98,6 +107,7 @@ String projectName = "projectToAdd"; NewProjectCreatedListener.Event event = mock(NewProjectCreatedListener.Event.class); when(event.getProjectName()).thenReturn(projectName); + when(event.getInstanceId()).thenReturn(INSTANCE_ID); handler.onNewProjectCreated(event); verify(forwarder, never()) .updateProjectList(
diff --git a/src/test/java/com/googlesource/gerrit/plugins/multisite/index/ChangeCheckerIT.java b/src/test/java/com/googlesource/gerrit/plugins/multisite/index/ChangeCheckerIT.java new file mode 100644 index 0000000..d3c5e8d --- /dev/null +++ b/src/test/java/com/googlesource/gerrit/plugins/multisite/index/ChangeCheckerIT.java
@@ -0,0 +1,114 @@ +// Copyright (C) 2024 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 static com.google.common.truth.Truth.assertThat; + +import com.google.gerrit.acceptance.LightweightPluginDaemonTest; +import com.google.gerrit.acceptance.PushOneCommit; +import com.google.gerrit.acceptance.TestPlugin; +import com.google.gerrit.acceptance.config.GerritConfig; +import com.google.gerrit.entities.RefNames; +import com.google.gerrit.extensions.restapi.RestApiException; +import com.google.inject.AbstractModule; +import com.googlesource.gerrit.plugins.multisite.forwarder.events.ChangeIndexEvent; +import java.util.Optional; +import org.eclipse.jgit.revwalk.RevCommit; +import org.junit.Test; + +@TestPlugin( + name = "multi-site", + sysModule = "com.googlesource.gerrit.plugins.multisite.index.ChangeCheckerIT$TestModule") +public class ChangeCheckerIT extends LightweightPluginDaemonTest { + private static final String TEST_BRANCH_REFS_HEADS = RefNames.fullName("master"); + private static final String NONEXISTENTSHA1 = "d3d192b8f704aec0c764cca427fdaca88db71513"; + private ChangeCheckerImpl.Factory changeCheckerFactory; + + public static class TestModule extends AbstractModule { + @Override + protected void configure() { + install(ChangeCheckerImpl.module()); + } + } + + @Override + public void setUpTestPlugin() throws Exception { + super.setUpTestPlugin(); + + changeCheckerFactory = plugin.getSysInjector().getInstance(ChangeCheckerImpl.Factory.class); + } + + @Test + @GerritConfig(name = "gerrit.instanceId", value = "test-instance") + public void shouldBeUpToDateIfTargetBranchSHA1HasAdvanced() throws Exception { + RevCommit headCommit = createCommit(); + int changeNum = newChangeNum(); + String changeId = changeProjectAndNum(changeNum); + long changeCommitTs = changeCommitTs(changeNum); + + ChangeIndexEvent indexChangeEvent = newIndexChangeEvent(changeNum); + indexChangeEvent.eventCreatedOn = changeCommitTs / 1000L; + indexChangeEvent.targetSha = headCommit.getId().getName(); + + RevCommit secondCommit = createCommit(); + assertThat(secondCommit.getId().getName()).isNotEqualTo(indexChangeEvent.targetSha); + assertThat(changeCheckerFactory.create(changeId).isUpToDate(Optional.of(indexChangeEvent))) + .isTrue(); + } + + @Test + @GerritConfig(name = "gerrit.instanceId", value = "test-instance") + public void shouldNotBeUpToDateIfTargetSHA1Absent() throws Exception { + int changeNum = newChangeNum(); + String changeId = changeProjectAndNum(changeNum); + long changeCommitTs = changeCommitTs(changeNum); + + ChangeIndexEvent indexChangeEvent = newIndexChangeEvent(changeNum); + indexChangeEvent.eventCreatedOn = changeCommitTs / 1000L; + indexChangeEvent.targetSha = NONEXISTENTSHA1; + + assertThat(changeCheckerFactory.create(changeId).isUpToDate(Optional.of(indexChangeEvent))) + .isFalse(); + } + + private ChangeIndexEvent newIndexChangeEvent(int changeNum) { + ChangeIndexEvent indexChangeEvent = + new ChangeIndexEvent(project.get(), changeNum, false, instanceId); + return indexChangeEvent; + } + + private int newChangeNum() throws Exception { + PushOneCommit.Result changeRes = createChange(); + changeRes.assertOkStatus(); + int changeNum = changeRes.getChange().getId().get(); + return changeNum; + } + + private String changeProjectAndNum(int changeNum) { + String changeId = String.format("%s~%d", project.get(), changeNum); + return changeId; + } + + private long changeCommitTs(int changeNum) throws RestApiException { + long changeCommitTs = gApi.changes().id(project.get(), changeNum).get().updated.getTime(); + return changeCommitTs; + } + + private RevCommit createCommit() throws Exception { + PushOneCommit.Result result = pushTo(TEST_BRANCH_REFS_HEADS); + result.assertOkStatus(); + return result.getCommit(); + } +}
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 1cfb2cd..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
@@ -28,11 +28,9 @@ 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,63 +245,19 @@ 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(); - - verifyNoInteractions(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(); - - verifyNoInteractions(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(); @@ -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();