Merge branch 'stable-3.11' into stable-3.12 * stable-3.11: Properly cleanup site3 Change-Id: I6e2a52d2038ce67a0d11c82edc9e60ac3aff3379
diff --git a/README.md b/README.md index 1e6882c..2e13f45 100644 --- a/README.md +++ b/README.md
@@ -32,21 +32,38 @@ The multi-site plugin can only be built in tree mode, by cloning Gerrit and the multi-site plugin code, and checking them out on the desired branch. -Example of cloning Gerrit and multi-site for a stable-2.16 build: +To build the multi-site plugin in addition to the gerrit core plugins also the +following plugins need to be present in the plugins directory: + +- pull-replication +- healthcheck +- events-broker +- global-refdb + +Example of cloning Gerrit, the multi-site plugin and the plugins it depends on +for a stable-3.11 build: ``` -git clone -b stable-2.16 https://gerrit.googlesource.com/gerrit -git clone -b stable-2.16 https://gerrit.googlesource.com/plugins/multi-site +git clone --recurse-submodules -b stable-3.11 https://gerrit.googlesource.com/gerrit +git clone -b stable-3.11 https://gerrit.googlesource.com/plugins/multi-site +git clone -b stable-3.11 https://gerrit.googlesource.com/modules/events-broker +git clone -b stable-3.11 https://gerrit.googlesource.com/modules/global-refdb +git clone -b stable-3.11 https://gerrit.googlesource.com/plugins/healthcheck +git clone -b stable-3.11 https://gerrit.googlesource.com/plugins/pull-replication cd gerrit/plugins ln -s ../../multi-site . +ln -s ../../events-broker . +ln -s ../../global-refdb . +ln -s ../../healthcheck . +ln -s ../../pull-replication . ``` Example of building the multi-site plugin: ``` cd gerrit -bazel build plugins/multi-site +bazelisk build plugins/multi-site ``` The multi-site.jar plugin is generated to `bazel-bin/plugins/multi-site/multi-site.jar`. @@ -55,13 +72,13 @@ ``` cd gerrit -bazel test plugins/multi-site:multi_site_tests +bazelisk test plugins/multi-site/... ``` **NOTE**: The multi-site tests include also the use of Docker containers for instantiating and using a Kafka/Zookeeper broker. Make sure you have a Docker daemon running (/var/run/docker.sock accessible) or a DOCKER_HOST pointing to -a Docker server. +a Docker server. In addition docker-compose needs to be installed. ## Pre-requisites
diff --git a/setup_local_env/setup.sh b/setup_local_env/setup.sh index 3e79f14..d7ab200 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.11 +GERRIT_BRANCH=stable-3.12 GERRIT_CI=https://gerrit-ci.gerritforge.com/view/Plugins-$GERRIT_BRANCH/job LAST_BUILD=lastSuccessfulBuild/artifact/bazel-bin/plugins
diff --git a/src/main/java/com/googlesource/gerrit/plugins/multisite/Configuration.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/Configuration.java index 4415aea..1be09aa 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/multisite/Configuration.java +++ b/src/main/java/com/googlesource/gerrit/plugins/multisite/Configuration.java
@@ -58,6 +58,8 @@ private static final String REPLICATION_LAG_REFRESH_INTERVAL = "replicationLagRefreshInterval"; private static final String REPLICATION_LAG_ENABLED = "replicationLagEnabled"; private static final Duration REPLICATION_LAG_REFRESH_INTERVAL_DEFAULT = Duration.ofSeconds(60); + private static final String PUSH_REPLICATION_FILTER_ENABLED = "pushReplicationFilterEnabled"; + private static final String PULL_REPLICATION_FILTER_ENABLED = "pullReplicationFilterEnabled"; private static final String LOCAL_REF_LOCK_TIMEOUT = "localRefLockTimeout"; private static final Duration LOCAL_REF_LOCK_TIMEOUT_DEFAULT = Duration.ofSeconds(30); @@ -81,6 +83,8 @@ private final Config multiSiteConfig; private final Supplier<Duration> replicationLagRefreshInterval; private final Supplier<Boolean> replicationLagEnabled; + private final Supplier<Boolean> pushReplicationFilterEnabled; + private final Supplier<Boolean> pullReplicationFilterEnabled; private final Supplier<Long> localRefLockTimeoutMsec; @Inject @@ -121,6 +125,19 @@ lazyMultiSiteCfg .get() .getBoolean(REF_DATABASE, null, REPLICATION_LAG_ENABLED, true)); + + pushReplicationFilterEnabled = + memoize( + () -> + lazyMultiSiteCfg + .get() + .getBoolean(REF_DATABASE, null, PUSH_REPLICATION_FILTER_ENABLED, true)); + pullReplicationFilterEnabled = + memoize( + () -> + lazyMultiSiteCfg + .get() + .getBoolean(REF_DATABASE, null, PULL_REPLICATION_FILTER_ENABLED, true)); localRefLockTimeoutMsec = memoize( () -> @@ -173,6 +190,14 @@ return replicationLagEnabled.get(); } + public boolean pushReplicationFilterEnabled() { + return pushReplicationFilterEnabled.get(); + } + + public boolean pullRepllicationFilterEnabled() { + return pullReplicationFilterEnabled.get(); + } + public long localRefLockTimeoutMsec() { return localRefLockTimeoutMsec.get(); }
diff --git a/src/main/java/com/googlesource/gerrit/plugins/multisite/PluginModule.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/PluginModule.java index db06c9f..3ced8c6 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/multisite/PluginModule.java +++ b/src/main/java/com/googlesource/gerrit/plugins/multisite/PluginModule.java
@@ -35,14 +35,10 @@ public class PluginModule extends LifecycleModule { private static final FluentLogger log = FluentLogger.forEnclosingClass(); - private static final String[] FILTER_MODULES_CLASS_NAMES = - new String[] { - /* Class names are defined as String for avoiding this class failing to load - * if either replication or pull-replication plugins are missing. - */ - "com.googlesource.gerrit.plugins.multisite.validation.PullReplicationFilterModule", - "com.googlesource.gerrit.plugins.multisite.validation.PushReplicationFilterModule" - }; + public static final String PULL_REPLICATION_FILTER_MODULE = + "com.googlesource.gerrit.plugins.multisite.validation.PullReplicationFilterModule"; + public static final String PUSH_REPLICATION_FILTER_MODULE = + "com.googlesource.gerrit.plugins.multisite.validation.PushReplicationFilterModule"; private final Configuration config; private final WorkQueue workQueue; @@ -89,25 +85,33 @@ private Iterable<AbstractModule> detectFilterModules() { ImmutableList.Builder<AbstractModule> filterModulesBuilder = ImmutableList.builder(); - for (String filterClassName : FILTER_MODULES_CLASS_NAMES) { - try { - @SuppressWarnings("unchecked") - Class<AbstractModule> filterClass = (Class<AbstractModule>) Class.forName(filterClassName); - - AbstractModule filterModule = parentInjector.getInstance(filterClass); - // Check if the filterModule would be valid for creating a child Guice Injector - parentInjector.createChildInjector(filterModule); - - filterModulesBuilder.add(filterModule); - } catch (NoClassDefFoundError | ClassNotFoundException e) { - log.atFine().withCause(e).log( - "Not loading %s because of missing the associated replication plugin", filterClassName); - } catch (Exception e) { - throw new ProvisionException( - "Unable to instantiate replication filter " + filterClassName, e); - } + if (config.pushReplicationFilterEnabled()) { + bindReplicationFilterClass(PUSH_REPLICATION_FILTER_MODULE, filterModulesBuilder); + } + if (config.pullRepllicationFilterEnabled()) { + bindReplicationFilterClass(PULL_REPLICATION_FILTER_MODULE, filterModulesBuilder); } return filterModulesBuilder.build(); } + + private void bindReplicationFilterClass( + String filterClassName, ImmutableList.Builder<AbstractModule> filterModulesBuilder) { + try { + @SuppressWarnings("unchecked") + Class<AbstractModule> filterClass = (Class<AbstractModule>) Class.forName(filterClassName); + + AbstractModule filterModule = parentInjector.getInstance(filterClass); + // Check if the filterModule would be valid for creating a child Guice Injector + parentInjector.createChildInjector(filterModule); + + filterModulesBuilder.add(filterModule); + } catch (NoClassDefFoundError | ClassNotFoundException e) { + log.atWarning().withCause(e).log( + "Not loading %s because of missing the associated replication plugin", filterClassName); + } catch (Exception e) { + throw new ProvisionException( + "Unable to instantiate replication filter " + filterClassName, e); + } + } }
diff --git a/src/main/java/com/googlesource/gerrit/plugins/multisite/index/FetchRefReplicatedEventHandler.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/index/FetchRefReplicatedEventHandler.java index 83076bf..69cdb06 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/multisite/index/FetchRefReplicatedEventHandler.java +++ b/src/main/java/com/googlesource/gerrit/plugins/multisite/index/FetchRefReplicatedEventHandler.java
@@ -18,6 +18,7 @@ import com.google.gerrit.entities.Change; import com.google.gerrit.entities.Project; import com.google.gerrit.entities.RefNames; +import com.google.gerrit.exceptions.StorageException; import com.google.gerrit.server.config.GerritInstanceId; import com.google.gerrit.server.events.Event; import com.google.gerrit.server.events.EventListener; @@ -28,6 +29,7 @@ import com.googlesource.gerrit.plugins.replication.pull.FetchRefReplicatedEvent; import com.googlesource.gerrit.plugins.replication.pull.ReplicationState; import java.io.IOException; +import org.eclipse.jgit.errors.MissingObjectException; import org.eclipse.jgit.lib.RefUpdate; import org.eclipse.jgit.lib.Repository; @@ -63,7 +65,8 @@ fetchRefReplicatedEvent.ref.startsWith(":") ? fetchRefReplicatedEvent.ref.substring(1) : fetchRefReplicatedEvent.ref; - if (!RefNames.isNoteDbMetaRef(refName) + boolean isMetaRef = RefNames.isNoteDbMetaRef(refName); + if (!RefNames.isRefsChanges(refName) || !fetchRefReplicatedEvent .getStatus() .equals(ReplicationState.RefFetchResult.SUCCEEDED.toString())) { @@ -71,6 +74,9 @@ } Project.NameKey projectNameKey = fetchRefReplicatedEvent.getProjectNameKey(); + logger.atFine().log( + "Indexing ref '%s' for project %s", + fetchRefReplicatedEvent.getRefName(), projectNameKey.get()); Change.Id changeId = Change.Id.fromRef(refName); try (Repository repo = gitRepositoryManager.openRepository(projectNameKey)) { @@ -93,12 +99,32 @@ // so that multi-site can use it for computing the virtual change-id and assuring the // delete // of the imported changes as well. - changeIndexer.delete(changeId); + if (isMetaRef) { + catchStorageExceptionForMissingUnknown( + () -> changeIndexer.delete(changeId), + "Skipping deleting from index of " + + projectNameKey + + "~" + + changeId.get() + + " because its patch-sets aren't replicated yet"); + } else { + changeIndexer.delete(changeId); + } } else { logger.atInfo().log( "Indexing change '%s' following update of '%s' on project %s", changeId, refName, projectNameKey.get()); - changeIndexer.index(projectNameKey, changeId); + if (isMetaRef) { + catchStorageExceptionForMissingUnknown( + () -> changeIndexer.index(projectNameKey, changeId), + "Skipping indexing of " + + projectNameKey + + "~" + + changeId.get() + + " because its patch-sets aren't replicated yet"); + } else { + changeIndexer.index(projectNameKey, changeId); + } } } else { logger.atWarning().log( @@ -114,6 +140,23 @@ } } + private void catchStorageExceptionForMissingUnknown(Runnable lambda, String infoMessage) + throws RuntimeException { + try { + lambda.run(); + } catch (StorageException se) { + Throwable cause = se.getCause(); + while (cause != null) { + if (cause instanceof MissingObjectException) { + logger.atInfo().log("%s", infoMessage); + return; + } + cause = cause.getCause(); + } + throw se; + } + } + private boolean isLocalEvent(Event event) { return instanceId.equals(event.instanceId); }
diff --git a/src/main/resources/Documentation/config.md b/src/main/resources/Documentation/config.md index 55e5041..ed72405 100644 --- a/src/main/resources/Documentation/config.md +++ b/src/main/resources/Documentation/config.md
@@ -95,6 +95,16 @@ : Enable the use of a shared ref-database Defaults: true +```ref-database.pushReplicationFilterClassEnabled``` +: Enable the filtering of push replication events checking their + up-to-date status with the global-refdb. + Defaults: true + +```ref-database.pullReplicationFilterClassEnabled``` +: Enable the filtering of pull replication events checking their + up-to-date status with the global-refdb. + Defaults: true + ```ref-database.localRefLockTimeout``` : Timeout waiting for a local ref to become available to accept updates or for starting a replication task.
diff --git a/src/test/java/com/googlesource/gerrit/plugins/multisite/index/FetchRefReplicatedEventHandlerTest.java b/src/test/java/com/googlesource/gerrit/plugins/multisite/index/FetchRefReplicatedEventHandlerTest.java index ed2f958..f7ef474 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/multisite/index/FetchRefReplicatedEventHandlerTest.java +++ b/src/test/java/com/googlesource/gerrit/plugins/multisite/index/FetchRefReplicatedEventHandlerTest.java
@@ -14,9 +14,12 @@ package com.googlesource.gerrit.plugins.multisite.index; +import static com.google.common.truth.Truth.assertThat; +import static com.google.gerrit.testing.GerritJUnit.assertThrows; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.times; @@ -24,12 +27,15 @@ import com.google.gerrit.entities.Change; import com.google.gerrit.entities.Project; +import com.google.gerrit.exceptions.StorageException; import com.google.gerrit.server.git.GitRepositoryManager; import com.google.gerrit.server.index.change.ChangeIndexer; import com.googlesource.gerrit.plugins.replication.pull.Context; import com.googlesource.gerrit.plugins.replication.pull.FetchRefReplicatedEvent; import com.googlesource.gerrit.plugins.replication.pull.ReplicationState; import com.googlesource.gerrit.plugins.replication.pull.ReplicationState.RefFetchResult; +import org.eclipse.jgit.errors.MissingObjectException; +import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.RefUpdate; import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.transport.URIish; @@ -98,6 +104,65 @@ } @Test + public void onEventShouldIgnoreMissingObjectWhenIndexExistingChangeMeta() { + Project.NameKey projectNameKey = Project.nameKey("testProject"); + String ref = "refs/changes/41/41/meta"; + Change.Id changeId = Change.Id.fromRef(ref); + try { + Context.setLocalEvent(true); + StorageException storageExceptionForMissingObject = + new StorageException( + "Test storage exception", + new MissingObjectException(ObjectId.zeroId(), "Missing unknown")); + doThrow(storageExceptionForMissingObject) + .when(changeIndexerMock) + .index(eq(projectNameKey), eq(changeId)); + fetchRefReplicatedEventHandler.onEvent( + newFetchRefReplicatedEvent( + projectNameKey, + ref, + ReplicationState.RefFetchResult.SUCCEEDED, + LOCAL_INSTANCE_ID, + RefUpdate.Result.FAST_FORWARD)); + verify(changeIndexerMock, times(1)).index(eq(projectNameKey), eq(changeId)); + } finally { + Context.unsetLocalEvent(); + } + } + + @Test + public void onEventShouldThrowStorageExceptionWhenIndexPatchSetsWithMissingRefs() { + Project.NameKey projectNameKey = Project.nameKey("testProject"); + String ref = "refs/changes/41/41/1"; + Change.Id changeId = Change.Id.fromRef(ref); + try { + Context.setLocalEvent(true); + StorageException storageExceptionForMissingObject = + new StorageException( + "Test storage exception", + new MissingObjectException(ObjectId.zeroId(), "Missing unknown")); + doThrow(storageExceptionForMissingObject) + .when(changeIndexerMock) + .index(eq(projectNameKey), eq(changeId)); + StorageException indexException = + assertThrows( + StorageException.class, + () -> + fetchRefReplicatedEventHandler.onEvent( + newFetchRefReplicatedEvent( + projectNameKey, + ref, + RefFetchResult.SUCCEEDED, + LOCAL_INSTANCE_ID, + RefUpdate.Result.FAST_FORWARD))); + assertThat(indexException).isEqualTo(storageExceptionForMissingObject); + verify(changeIndexerMock, times(1)).index(eq(projectNameKey), eq(changeId)); + } finally { + Context.unsetLocalEvent(); + } + } + + @Test public void onEventShouldNotIndexIfNotLocalEvent() { Project.NameKey projectNameKey = Project.nameKey("testProject"); String ref = "refs/changes/41/41/meta"; @@ -113,7 +178,7 @@ } @Test - public void onEventShouldIndexOnlyMetaRef() { + public void onEventShouldIndexOnPatchSetsRef() { Project.NameKey projectNameKey = Project.nameKey("testProject"); String ref = "refs/changes/41/41/1"; Change.Id changeId = Change.Id.fromRef(ref); @@ -124,7 +189,7 @@ ReplicationState.RefFetchResult.SUCCEEDED, LOCAL_INSTANCE_ID, RefUpdate.Result.FAST_FORWARD)); - verify(changeIndexerMock, never()).index(eq(projectNameKey), eq(changeId)); + verify(changeIndexerMock).index(eq(projectNameKey), eq(changeId)); } @Test