Merge branch 'stable-3.8' into stable-3.9
* stable-3.8:
DRY out shouldNotBeTrackedAnymoreOnGlobalRefDb
Ignore refs/multi-site/version on global-refdb in push replication filter
Change-Id: I783c2204945743d5c95b0acd713bf232231014a5
diff --git a/.gitignore b/.gitignore
index 73f4353..8bccbdb 100644
--- a/.gitignore
+++ b/.gitignore
@@ -13,3 +13,4 @@
/bazel-testlogs
/eclipse-out/
+setup_local_env/**/*.env
diff --git a/DESIGN.md b/DESIGN.md
index b114eb3..870cc03 100644
--- a/DESIGN.md
+++ b/DESIGN.md
@@ -200,7 +200,7 @@
This plugin expands upon the excellent work on the high-availability plugin,
introduced by Ericsson for implementing mutli-master at Stage #4. The git log history
-of this projects still shows the 'branching point' where it started.
+of this project still shows the 'branching point' where it started.
The v2.16.x (with NoteDb) of the multi-site plugin was at Stage #7.
The current version of the multi-site plugin is at Stage #9, it is now possible for
@@ -218,7 +218,7 @@
In theory, one could keep a single code-base to manage both approaches, however the
result would be very complicated and difficult to configure and install.
-Having two more focussed plugins, one for high availability and another for
+Having two more focused plugins, one for high availability and another for
multi-site, allows us to have a simpler, more usable experience, both for developers
of the plugin and for the Gerrit administrators using it.
@@ -259,7 +259,7 @@
Having to deal with a very high number of site requires the implementation of a quorum on
all the nodes available for replication.
-- **Requires Gerrit v3.0 or later**: Data conisistency requires a server completely
+- **Requires Gerrit v3.0 or later**: Data consistency requires a server completely
based on NoteDb.
If you are not familiar with NoteDb, please read the relevant
[section in the Gerrit documentation](https://gerrit-documentation.storage.googleapis.com/Documentation/3.0.12/note-db.html).
@@ -278,7 +278,7 @@
both from the Gerrit UI and on the Git repository served locally.
In contrast, a developer located in San Francisco will always see on his repository
the "latest and greatest" of everything.
-Things are exactly in the other way around for a repository that is mainly
+Things are exactly the other way around for a repository that is mainly
receiving pushes from developers in Bangalore.
Should the central site in San Francisco become unavailable for a
@@ -344,11 +344,9 @@
When no specific implementation is provided, then the [Global Ref-DB Noop implementation](#global-ref-db-noop-implementation)
then libModule interfaces are mapped to internal no-ops implementations.
-- **replication plugin**: enables asynchronous push replication of the _Git repositories_
- across sites.
-
-- **pull replication plugin**: enables the synchronous replication of the _Git repositories_
- across sites.
+- **replication plugin**:This can be either the *replication plugin* which enables asynchronous push replication of the _Git repositories_,
+ or the *pull replication plugin* which enables asynchronous pull replication of the _Git repositories_.
+ As far as we're aware, most installations use *pull replication*.
- **web-session broker plugin**: supports the storage of _active sessions_
to a message broker topic, which is then broadcasted across sites.
@@ -482,12 +480,12 @@
The concept of the instance-id is very useful. Since other plugins could benefit
from it, it will be the first candidate to move into the Gerrit core,
generated and maintained with the rest of the configuration. Then it can be
-included in **all** stream events, at which time the multi-site plugin's
+included in **all** stream events, at which time the multi-site plugin's
"enveloping of events" will become redundant.
### Manage failures
-The broker based solutions improve the resilience and scalability of the system.
+The broker based solution improves the resilience and scalability of the system.
But there is still a point of failure: the availability of the broker itself. However,
using the broker does allow having a high-level of redundancy and a multi-master
/ multi-site configuration at the transport and storage level.
@@ -659,4 +657,4 @@
the message broker
- Implement a "fast replication path" for NoteDb-only changes, instead of relying on the
- Git protocol
\ No newline at end of file
+ Git protocol
diff --git a/README.md b/README.md
index f683f4e..1e6882c 100644
--- a/README.md
+++ b/README.md
@@ -96,9 +96,8 @@
For more details on the configuration settings, please refer to the
[multi-site configuration documentation](src/main/resources/Documentation/config.md).
-You also need to setup the Git-level replication between nodes, for more details
-please refer to the
-[replication plugin documentation](https://gerrit.googlesource.com/plugins/replication/+/refs/heads/master/src/main/resources/Documentation/config.md).
+You also need to setup the Git-level replication between nodes.
+This can be done with either [pull](https://gerrit.googlesource.com/plugins/pull-replication/+/refs/heads/master/src/main/resources/Documentation/config.md) or [push](https://gerrit.googlesource.com/plugins/replication/+/refs/heads/master/src/main/resources/Documentation/config.md) replication plugin.
# HTTP endpoints
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/setup.sh b/setup_local_env/setup.sh
index 0e84603..96a4204 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.9
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/ChangeCheckerImpl.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/index/ChangeCheckerImpl.java
index 063d7bf..7b90468 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
@@ -14,9 +14,13 @@
package com.googlesource.gerrit.plugins.multisite.index;
+import com.google.gerrit.entities.Change;
+import com.google.gerrit.entities.HumanComment;
import com.google.gerrit.exceptions.StorageException;
+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;
@@ -29,6 +33,7 @@
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;
@@ -39,10 +44,12 @@
public class ChangeCheckerImpl implements ChangeChecker {
private static final Logger log = LoggerFactory.getLogger(ChangeCheckerImpl.class);
private final GitRepositoryManager gitRepoMgr;
+ private final DraftCommentsReader draftCommentsReader;
private final OneOffRequestContext oneOffReqCtx;
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();
@@ -53,15 +60,20 @@
@Inject
public ChangeCheckerImpl(
GitRepositoryManager gitRepoMgr,
+ DraftCommentsReader draftCommentsReader,
ChangeFinder changeFinder,
OneOffRequestContext oneOffReqCtx,
@GerritInstanceId String instanceId,
+ @GerritServerConfig Config config,
@Assisted String changeId) {
this.changeFinder = changeFinder;
this.gitRepoMgr = gitRepoMgr;
+ this.draftCommentsReader = draftCommentsReader;
this.oneOffReqCtx = oneOffReqCtx;
this.changeId = changeId;
this.instanceId = instanceId;
+ this.enableDraftCommentEvents =
+ config.getBoolean("event", "stream-events", "enableDraftCommentEvents", false);
}
@Override
@@ -171,7 +183,23 @@
}
private Optional<Long> computeLastChangeTs() {
- return getChangeNotes()
- .map(notes -> Timestamp.from(notes.getChange().getLastUpdatedOn()).getTime() / 1000);
+ return getChangeNotes().map(this::getTsFromChangeAndDraftComments);
+ }
+
+ private long getTsFromChangeAndDraftComments(ChangeNotes notes) {
+ Change change = notes.getChange();
+ Timestamp changeTs = Timestamp.from(change.getLastUpdatedOn());
+ 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);
+ }
+ }
+ return changeTs.getTime() / 1000;
}
}
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
new file mode 100644
index 0000000..d45f526
--- /dev/null
+++ b/src/main/java/com/googlesource/gerrit/plugins/multisite/index/FetchRefReplicatedEventHandler.java
@@ -0,0 +1,77 @@
+// 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.entities.Change;
+import com.google.gerrit.entities.Project;
+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.index.change.ChangeIndexer;
+import com.google.inject.Inject;
+import com.googlesource.gerrit.plugins.multisite.forwarder.Context;
+import com.googlesource.gerrit.plugins.replication.pull.FetchRefReplicatedEvent;
+import com.googlesource.gerrit.plugins.replication.pull.ReplicationState;
+
+public class FetchRefReplicatedEventHandler implements EventListener {
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+ private ChangeIndexer changeIndexer;
+ private final String instanceId;
+
+ @Inject
+ FetchRefReplicatedEventHandler(ChangeIndexer changeIndexer, @GerritInstanceId String instanceId) {
+ this.changeIndexer = changeIndexer;
+ this.instanceId = instanceId;
+ }
+
+ @Override
+ public void onEvent(Event event) {
+ boolean isForwarded = Context.isForwardedEvent();
+ try {
+ Context.setForwardedEvent(true);
+
+ if (event instanceof FetchRefReplicatedEvent && isLocalEvent(event)) {
+ FetchRefReplicatedEvent fetchRefReplicatedEvent = (FetchRefReplicatedEvent) event;
+ if (!RefNames.isNoteDbMetaRef(fetchRefReplicatedEvent.getRefName())
+ || !fetchRefReplicatedEvent
+ .getStatus()
+ .equals(ReplicationState.RefFetchResult.SUCCEEDED.toString())) {
+ return;
+ }
+
+ Project.NameKey projectNameKey = fetchRefReplicatedEvent.getProjectNameKey();
+ logger.atFine().log(
+ "Indexing ref '%s' for project %s",
+ fetchRefReplicatedEvent.getRefName(), projectNameKey.get());
+ Change.Id changeId = Change.Id.fromRef(fetchRefReplicatedEvent.getRefName());
+ if (changeId != null) {
+ changeIndexer.index(projectNameKey, changeId);
+ } else {
+ logger.atWarning().log(
+ "Couldn't get changeId from refName. Skipping indexing of change %s for project %s",
+ fetchRefReplicatedEvent.getRefName(), projectNameKey.get());
+ }
+ }
+ } finally {
+ Context.setForwardedEvent(isForwarded);
+ }
+ }
+
+ private boolean isLocalEvent(Event event) {
+ return instanceId.equals(event.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 075388f..5f47371 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
@@ -20,6 +20,7 @@
import com.google.gerrit.extensions.events.ProjectIndexedListener;
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;
@@ -37,6 +38,7 @@
DynamicSet.bind(binder(), AccountIndexedListener.class).to(IndexEventHandler.class);
DynamicSet.bind(binder(), GroupIndexedListener.class).to(IndexEventHandler.class);
DynamicSet.bind(binder(), ProjectIndexedListener.class).to(IndexEventHandler.class);
+ DynamicSet.bind(binder(), EventListener.class).to(FetchRefReplicatedEventHandler.class);
bind(ProjectChecker.class).to(ProjectCheckerImpl.class);
bind(GroupChecker.class).to(GroupCheckerImpl.class);
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
new file mode 100644
index 0000000..47b6072
--- /dev/null
+++ b/src/test/java/com/googlesource/gerrit/plugins/multisite/index/FetchRefReplicatedEventHandlerTest.java
@@ -0,0 +1,128 @@
+// 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 static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+
+import com.google.gerrit.entities.Change;
+import com.google.gerrit.entities.Project;
+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.lib.RefUpdate;
+import org.eclipse.jgit.transport.URIish;
+import org.junit.Before;
+import org.junit.Test;
+
+public class FetchRefReplicatedEventHandlerTest {
+ private static final String LOCAL_INSTANCE_ID = "local-instance-id";
+ private static final String REMOTE_INSTANCE_ID = "remote-instance-id";
+ private ChangeIndexer changeIndexerMock;
+ private FetchRefReplicatedEventHandler fetchRefReplicatedEventHandler;
+ private static URIish sourceUri;
+
+ @Before
+ public void setUp() throws Exception {
+ changeIndexerMock = mock(ChangeIndexer.class);
+ fetchRefReplicatedEventHandler =
+ new FetchRefReplicatedEventHandler(changeIndexerMock, LOCAL_INSTANCE_ID);
+ sourceUri = new URIish("git://aSourceNode/testProject.git");
+ }
+
+ @Test
+ public void onEventShouldIndexExistingChange() {
+ Project.NameKey projectNameKey = Project.nameKey("testProject");
+ String ref = "refs/changes/41/41/meta";
+ Change.Id changeId = Change.Id.fromRef(ref);
+ try {
+ Context.setLocalEvent(true);
+ 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 onEventShouldNotIndexIfNotLocalEvent() {
+ Project.NameKey projectNameKey = Project.nameKey("testProject");
+ String ref = "refs/changes/41/41/meta";
+ Change.Id changeId = Change.Id.fromRef(ref);
+ fetchRefReplicatedEventHandler.onEvent(
+ newFetchRefReplicatedEvent(
+ projectNameKey, ref, ReplicationState.RefFetchResult.SUCCEEDED, REMOTE_INSTANCE_ID));
+ verify(changeIndexerMock, never()).index(eq(projectNameKey), eq(changeId));
+ }
+
+ @Test
+ public void onEventShouldIndexOnlyMetaRef() {
+ 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));
+ }
+
+ @Test
+ public void onEventShouldNotIndexMissingChange() {
+ fetchRefReplicatedEventHandler.onEvent(
+ newFetchRefReplicatedEvent(
+ Project.nameKey("testProject"),
+ "invalidRef",
+ ReplicationState.RefFetchResult.SUCCEEDED,
+ LOCAL_INSTANCE_ID));
+ verify(changeIndexerMock, never()).index(any(), any());
+ }
+
+ @Test
+ public void onEventShouldNotIndexFailingChange() {
+ Project.NameKey projectNameKey = Project.nameKey("testProject");
+ String ref = "refs/changes/41/41/meta";
+ fetchRefReplicatedEventHandler.onEvent(
+ newFetchRefReplicatedEvent(
+ projectNameKey, ref, ReplicationState.RefFetchResult.FAILED, LOCAL_INSTANCE_ID));
+ verify(changeIndexerMock, never()).index(any(), any());
+ }
+
+ @Test
+ public void onEventShouldNotIndexNotAttemptedChange() {
+ Project.NameKey projectNameKey = Project.nameKey("testProject");
+ String ref = "refs/changes/41/41/meta";
+ fetchRefReplicatedEventHandler.onEvent(
+ newFetchRefReplicatedEvent(
+ projectNameKey, ref, ReplicationState.RefFetchResult.NOT_ATTEMPTED, LOCAL_INSTANCE_ID));
+ verify(changeIndexerMock, never()).index(any(), any());
+ }
+
+ private FetchRefReplicatedEvent newFetchRefReplicatedEvent(
+ Project.NameKey projectNameKey, String ref, RefFetchResult fetchResult, String instanceId) {
+ FetchRefReplicatedEvent event =
+ new FetchRefReplicatedEvent(
+ projectNameKey.get(), ref, sourceUri, fetchResult, RefUpdate.Result.FAST_FORWARD);
+ event.instanceId = instanceId;
+ return event;
+ }
+}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/multisite/index/GroupCheckerImplTest.java b/src/test/java/com/googlesource/gerrit/plugins/multisite/index/GroupCheckerImplTest.java
index 09f8f59..c612119 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/multisite/index/GroupCheckerImplTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/multisite/index/GroupCheckerImplTest.java
@@ -30,7 +30,6 @@
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.ObjectIdRef;
import org.eclipse.jgit.lib.Ref;
-import org.eclipse.jgit.lib.RefDatabase;
import org.eclipse.jgit.lib.Repository;
import org.junit.Before;
import org.junit.Test;
@@ -45,13 +44,11 @@
GroupCheckerImpl objectUnderTest;
@Mock private GitRepositoryManager repoManagerMock;
- @Mock private RefDatabase refDatabaseMock;
@Mock private Repository repoMock;
@Before
public void setUp() throws Exception {
doReturn(repoMock).when(repoManagerMock).openRepository(allUsers);
- doReturn(refDatabaseMock).when(repoMock).getRefDatabase();
objectUnderTest = new GroupCheckerImpl(repoManagerMock, allUsers);
}
@@ -118,6 +115,6 @@
throws IOException {
String groupRefName = RefNames.refsGroups(AccountGroup.uuid(groupUUID.toString()));
ObjectIdRef.Unpeeled aRef = new ObjectIdRef.Unpeeled(Ref.Storage.LOOSE, groupRefName, objectId);
- doReturn(objectId == null ? null : aRef).when(refDatabaseMock).exactRef(groupRefName);
+ doReturn(objectId == null ? null : aRef).when(repoMock).exactRef(groupRefName);
}
}
