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
