Merge branch 'stable-3.12' * stable-3.12: Do not fail reindexing changes when the patch-sets aren't replicated yet Set GERRIT_BRANCH to stable-3.12 for local E2E test environment Set GERRIT_BRANCH to stable-3.11 for local E2E test environment Change-Id: I33231eceb492cd18b12b6b782b879673d77648d7
diff --git a/setup_local_env/setup.sh b/setup_local_env/setup.sh index 2673d3b..b6cdab4 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.10 +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/index/FetchRefReplicatedEventHandler.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/index/FetchRefReplicatedEventHandler.java index d45f526..f47cea5 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; @@ -26,6 +27,7 @@ import com.googlesource.gerrit.plugins.multisite.forwarder.Context; import com.googlesource.gerrit.plugins.replication.pull.FetchRefReplicatedEvent; import com.googlesource.gerrit.plugins.replication.pull.ReplicationState; +import org.eclipse.jgit.errors.MissingObjectException; public class FetchRefReplicatedEventHandler implements EventListener { private static final FluentLogger logger = FluentLogger.forEnclosingClass(); @@ -46,7 +48,8 @@ if (event instanceof FetchRefReplicatedEvent && isLocalEvent(event)) { FetchRefReplicatedEvent fetchRefReplicatedEvent = (FetchRefReplicatedEvent) event; - if (!RefNames.isNoteDbMetaRef(fetchRefReplicatedEvent.getRefName()) + boolean isMetaRef = RefNames.isNoteDbMetaRef(fetchRefReplicatedEvent.getRefName()); + if (!RefNames.isRefsChanges(fetchRefReplicatedEvent.getRefName()) || !fetchRefReplicatedEvent .getStatus() .equals(ReplicationState.RefFetchResult.SUCCEEDED.toString())) { @@ -58,7 +61,15 @@ "Indexing ref '%s' for project %s", fetchRefReplicatedEvent.getRefName(), projectNameKey.get()); Change.Id changeId = Change.Id.fromRef(fetchRefReplicatedEvent.getRefName()); - if (changeId != null) { + if (changeId != null && isMetaRef) { + catchStorageExceptionForMissingUnknown( + () -> changeIndexer.index(projectNameKey, changeId), + "Skipping indexing of " + + projectNameKey + + "~" + + changeId.get() + + " because its patch-sets aren't replicated yet"); + } else if (changeId != null) { changeIndexer.index(projectNameKey, changeId); } else { logger.atWarning().log( @@ -71,6 +82,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/test/java/com/googlesource/gerrit/plugins/multisite/index/FetchRefReplicatedEventHandlerTest.java b/src/test/java/com/googlesource/gerrit/plugins/multisite/index/FetchRefReplicatedEventHandlerTest.java index 47b6072..e7e0d8a 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,8 +14,11 @@ 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.doThrow; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.times; @@ -23,11 +26,14 @@ import com.google.gerrit.entities.Change; import com.google.gerrit.entities.Project; +import com.google.gerrit.exceptions.StorageException; 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.transport.URIish; import org.junit.Before; @@ -65,6 +71,57 @@ } @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)); + 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))); + 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"; @@ -76,14 +133,14 @@ } @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); fetchRefReplicatedEventHandler.onEvent( newFetchRefReplicatedEvent( projectNameKey, ref, ReplicationState.RefFetchResult.SUCCEEDED, LOCAL_INSTANCE_ID)); - verify(changeIndexerMock, never()).index(eq(projectNameKey), eq(changeId)); + verify(changeIndexerMock).index(eq(projectNameKey), eq(changeId)); } @Test