Merge branch 'stable-3.3' into stable-3.4 * stable-3.3: Revert "Set forward context for events having a different instanceId" Use Gerrit v3.2.14 on Ubuntu as CentOS is discontinued Serve reindexing simulating a GET request for /meta ref caching Auto-reload the indexTs file for auto-reindexing Download plugins from archive-ci.gerritforge.com Pin haproxy to 1.8.30-buster and fix associated issues Pin haproxy to 1.8.30-buster and fix associated issues Fix issue with change indexing during the NoteDb online migration Remove references to ReviewDb in README.md Change-Id: I0c89172943e017111dffe726358b82584d794d5e
diff --git a/README.md b/README.md index 8e7ef27..c8ac4b4 100644 --- a/README.md +++ b/README.md
@@ -1,13 +1,12 @@ # Gerrit high-availability plugin This plugin allows deploying a cluster of multiple Gerrit masters -on the same data-center sharing the same ReviewDb and Git repositories. +on the same data-center sharing the same Git repositories. Requirements for the Gerrit masters are: - Gerrit v2.14.20 or later - Externally mounted filesystem shared among the cluster -- ReviewDb on an external DataBase Server - Load-balancer (HAProxy or similar) ## License @@ -25,12 +24,12 @@ `gerrit-02.mycompany.com`, listening on the HTTP port 8080, with a shared volume mounted under `/shared`, see below the minimal configuration steps. -1. Install one Gerrit master on the first node (e.g. `gerrit-01.mycompany.com`) using an external - ReviewDb on a DB server and the repositories location under the shared volume (e.g. `/shared/git`). - Init the site in order to create the DB Schema and the initial repositories. +1. Install one Gerrit master on the first node (e.g. `gerrit-01.mycompany.com`) + with the repositories location under the shared volume (e.g. `/shared/git`). + Init the site in order to create the initial repositories. 2. Copy all the files of the first Gerrit master onto the second node (e.g. `gerrit-02.mycompany.com`) - so that it points to the same ReviewDb and the same repositories location. + so that it points to the same repositories location. 3. Install the high-availability plugin into the `$GERRIT_SITE/plugins` directory of both the Gerrit servers.
diff --git a/src/main/java/com/ericsson/gerrit/plugins/highavailability/autoreindex/IndexTs.java b/src/main/java/com/ericsson/gerrit/plugins/highavailability/autoreindex/IndexTs.java index 62d9c10..a6539e0 100644 --- a/src/main/java/com/ericsson/gerrit/plugins/highavailability/autoreindex/IndexTs.java +++ b/src/main/java/com/ericsson/gerrit/plugins/highavailability/autoreindex/IndexTs.java
@@ -36,6 +36,7 @@ import java.time.format.DateTimeFormatter; import java.util.Optional; import java.util.concurrent.ScheduledExecutorService; +import org.eclipse.jgit.internal.storage.file.FileSnapshot; @Singleton public class IndexTs @@ -62,15 +63,17 @@ class FlusherRunner implements Runnable { private final AbstractIndexRestApiServlet.IndexName index; + private final Path tsPath; + private FileSnapshot tsPathSnapshot; @Override public void run() { + reloadIndexTimeStampIfChanged(); LocalDateTime latestTs = getIndexTimeStamp(); Optional<LocalDateTime> currTs = getUpdateTs(index); if (!currTs.isPresent() || latestTs.isAfter(currTs.get())) { - Path indexTsFile = dataDir.resolve(index.name().toLowerCase()); try { - Files.write(indexTsFile, latestTs.format(formatter).getBytes(StandardCharsets.UTF_8)); + Files.write(tsPath, latestTs.format(formatter).getBytes(StandardCharsets.UTF_8)); } catch (IOException e) { log.atSevere().withCause(e).log("Unable to update last timestamp for index %s", index); } @@ -79,6 +82,8 @@ FlusherRunner(AbstractIndexRestApiServlet.IndexName index) { this.index = index; + this.tsPath = getIndexTsPath(index); + this.tsPathSnapshot = FileSnapshot.save(tsPath.toFile()); } private LocalDateTime getIndexTimeStamp() { @@ -95,6 +100,31 @@ throw new IllegalArgumentException("Unsupported index " + index); } } + + private void reloadIndexTimeStampIfChanged() { + if (tsPathSnapshot.isModified(tsPath.toFile())) { + getUpdateTs(index).ifPresent(this::setIndexTimeStamp); + } + } + + private void setIndexTimeStamp(LocalDateTime newTs) { + switch (index) { + case CHANGE: + changeTs = newTs; + break; + case GROUP: + groupTs = newTs; + break; + case ACCOUNT: + accountTs = newTs; + break; + case PROJECT: + projectTs = newTs; + break; + } + + tsPathSnapshot = FileSnapshot.save(tsPath.toFile()); + } } @Inject @@ -152,7 +182,7 @@ public Optional<LocalDateTime> getUpdateTs(AbstractIndexRestApiServlet.IndexName index) { try { - Path indexTsFile = dataDir.resolve(index.name().toLowerCase()); + Path indexTsFile = getIndexTsPath(index); if (indexTsFile.toFile().exists()) { String tsString = Files.readAllLines(indexTsFile).get(0); return Optional.of(LocalDateTime.parse(tsString, formatter)); @@ -185,4 +215,8 @@ throw new IllegalArgumentException("Unsupported index " + index); } } + + private Path getIndexTsPath(AbstractIndexRestApiServlet.IndexName index) { + return dataDir.resolve(index.name().toLowerCase()); + } }
diff --git a/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/ForwardedAwareEventBroker.java b/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/ForwardedAwareEventBroker.java index ce2caaf..29d4e56 100644 --- a/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/ForwardedAwareEventBroker.java +++ b/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/ForwardedAwareEventBroker.java
@@ -25,7 +25,6 @@ import com.google.gerrit.server.plugincontext.PluginSetContext; import com.google.gerrit.server.project.ProjectCache; import com.google.inject.Inject; -import java.util.Objects; class ForwardedAwareEventBroker extends EventBroker { @@ -51,17 +50,8 @@ this.allowedListeners = allowedListeners; } - private boolean isProducedByLocalInstance(Event event) { - return Objects.equals(event.instanceId, gerritInstanceId); - } - @Override protected void fireEventForUnrestrictedListeners(Event event) { - // An event should not be dispatched when it is "forwarded". - // Meaning, it was either produced somewhere else - if (!isProducedByLocalInstance(event)) { - Context.setForwardedEvent(true); - } // or it was consumed by the high-availability rest endpoint and // thus the context of its consumption has already been set to "forwarded". unrestrictedListeners.runEach(l -> fireEventForListener(l, event));
diff --git a/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/ForwardedIndexChangeHandler.java b/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/ForwardedIndexChangeHandler.java index 91e5a02..770259d 100644 --- a/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/ForwardedIndexChangeHandler.java +++ b/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/ForwardedIndexChangeHandler.java
@@ -21,6 +21,7 @@ import com.ericsson.gerrit.plugins.highavailability.index.ForwardedIndexExecutor; import com.google.common.base.Splitter; 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.google.gerrit.server.notedb.ChangeNotes; @@ -30,6 +31,7 @@ import com.google.inject.Inject; import com.google.inject.Singleton; import java.io.IOException; +import java.util.List; import java.util.Optional; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.TimeUnit; @@ -157,7 +159,15 @@ } private static Change.Id parseChangeId(String id) { - return Change.id(Integer.parseInt(Splitter.on("~").splitToList(id).get(1))); + return Change.id(Integer.parseInt(getChangeIdParts(id).get(1))); + } + + private static Project.NameKey parseProject(String id) { + return Project.nameKey(getChangeIdParts(id).get(0)); + } + + private static List<String> getChangeIdParts(String id) { + return Splitter.on("~").splitToList(id); } private static boolean isCausedByNoSuchChangeException(Throwable throwable) {
diff --git a/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/AbstractIndexRestApiServlet.java b/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/AbstractIndexRestApiServlet.java index f9ec516..a567f61 100644 --- a/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/AbstractIndexRestApiServlet.java +++ b/src/main/java/com/ericsson/gerrit/plugins/highavailability/forwarder/rest/AbstractIndexRestApiServlet.java
@@ -88,6 +88,7 @@ setHeaders(rsp); String path = req.getRequestURI(); T id = parse(path.substring(path.lastIndexOf('/') + 1)); + try { forwardedIndexingHandler.index(id, operation, parseBody(req)); rsp.setStatus(SC_NO_CONTENT);
diff --git a/src/main/resources/Documentation/config.md b/src/main/resources/Documentation/config.md index e7ce6b2..d94a91e 100644 --- a/src/main/resources/Documentation/config.md +++ b/src/main/resources/Documentation/config.md
@@ -270,23 +270,3 @@ Ignore the alignment with the global ref-db for AProject on refs/heads/feature. Defaults to no rule. All projects are REQUIRED to be consistent on all refs. - -File 'gerrit.config' --------------------- - -```gerrit.instanceId``` -: Optional identifier for this Gerrit instance. -[[docs](https://gerrit-documentation.storage.googleapis.com/Documentation/3.2.0/config-gerrit.html#gerrit.instanceId)]. - -Whilst this is not, specifically, a high-availability plugin configuration, it plays -an important role on which events are forwarded to peers. - -If `instanceId` is set, events produced by that Gerrit instance will contain its -origin in the `instanceId` field, allowing to track where they are coming from. - -The high-availability plugin will check the `instanceId` value to decide whether -an event should be forwarded: events that originated from different Gerrit instances -will not be forwarded. - -When neither `gerrit.instanceId` nor `event.instanceId` are set, it is not possible -to identify the origin of the event and thus the event is always forwarded.
diff --git a/src/test/docker/gerrit/Dockerfile b/src/test/docker/gerrit/Dockerfile index 602c04d..b4f9c6e 100644 --- a/src/test/docker/gerrit/Dockerfile +++ b/src/test/docker/gerrit/Dockerfile
@@ -1,16 +1,16 @@ -FROM gerritcodereview/gerrit:3.4.5 +FROM gerritcodereview/gerrit:3.4.5-ubuntu20 ENV GERRIT_BRANCH=stable-3.4 -ENV GERRIT_CI_URL=https://gerrit-ci.gerritforge.com/job +ENV GERRIT_CI_URL=https://archive-ci.gerritforge.com/job USER root -RUN yum install -y iputils nmap curl lsof gettext net-tools sudo +RUN apt update && apt install -y iputils-ping nmap curl lsof gettext net-tools sudo USER gerrit -ADD --chown=gerrit:gerrit $GERRIT_CI_URL/plugin-javamelody-bazel-master-$GERRIT_BRANCH/lastSuccessfulBuild/artifact/bazel-bin/plugins/javamelody/javamelody.jar /var/gerrit/plugins/javamelody.jar +ADD --chown=gerrit:gerrit $GERRIT_CI_URL/plugin-javamelody-bazel-$GERRIT_BRANCH/lastSuccessfulBuild/artifact/bazel-bin/plugins/javamelody/javamelody.jar /var/gerrit/plugins/javamelody.jar ADD --chown=gerrit:gerrit $GERRIT_CI_URL/plugin-high-availability-bazel-$GERRIT_BRANCH/lastSuccessfulBuild/artifact/bazel-bin/plugins/high-availability/high-availability.jar /var/gerrit/plugins/high-availability.jar USER root
diff --git a/src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/ForwardedAwareEventBrokerTest.java b/src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/ForwardedAwareEventBrokerTest.java index 7142841..4c639ba 100644 --- a/src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/ForwardedAwareEventBrokerTest.java +++ b/src/test/java/com/ericsson/gerrit/plugins/highavailability/forwarder/ForwardedAwareEventBrokerTest.java
@@ -14,11 +14,9 @@ package com.ericsson.gerrit.plugins.highavailability.forwarder; -import static org.mockito.Mockito.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyZeroInteractions; -import static org.mockito.Mockito.when; import com.google.gerrit.extensions.registration.DynamicSet; import com.google.gerrit.server.events.Event; @@ -33,9 +31,7 @@ private EventListener listenerMock; private AllowedForwardedEventListener allowListenerMock; private ForwardedAwareEventBroker broker; - private ForwardedAwareEventBroker brokerWithGerritInstanceId; - private Event event; - private String gerritInstanceId = "gerrit-instance-id"; + private Event event = new TestEvent(); @Before public void setUp() { @@ -44,13 +40,9 @@ allowListenerMock = mock(AllowedForwardedEventListener.class); DynamicSet<EventListener> set = DynamicSet.emptySet(); set.add("high-availability", listenerMock); - event = new TestEvent(); PluginSetContext<EventListener> listeners = new PluginSetContext<>(set, mockMetrics); broker = new ForwardedAwareEventBroker(null, listeners, null, null, null, null, allowListenerMock); - brokerWithGerritInstanceId = - new ForwardedAwareEventBroker( - null, listeners, null, null, null, gerritInstanceId, allowListenerMock); } @Test @@ -69,55 +61,4 @@ } verifyZeroInteractions(listenerMock); } - - @Test - public void shouldNotDispatchEventWhenEventInstanceIdIsDefinedButGerritInstanceIdIsNot() { - event.instanceId = gerritInstanceId; - try { - broker.fireEventForUnrestrictedListeners(event); - } finally { - Context.unsetForwardedEvent(); - } - verifyZeroInteractions(listenerMock); - } - - @Test - public void shouldNotDispatchEventWhenGerritInstanceIdIsDefinedButEventInstanceIdIsNot() { - try { - brokerWithGerritInstanceId.fireEventForUnrestrictedListeners(event); - } finally { - Context.unsetForwardedEvent(); - } - verifyZeroInteractions(listenerMock); - } - - @Test - public void shouldNotDispatchEventWhenInstanceIdsAreDifferent() { - event.instanceId = "some-other-gerrit-instance-id"; - try { - brokerWithGerritInstanceId.fireEventForUnrestrictedListeners(event); - } finally { - Context.unsetForwardedEvent(); - } - verifyZeroInteractions(listenerMock); - } - - @Test - public void shouldDispatchEventWhenInstanceIdsAreDifferentToAllowedListener() { - event.instanceId = "some-other-gerrit-instance-id"; - when(allowListenerMock.isAllowed(any())).thenReturn(true); - try { - brokerWithGerritInstanceId.fireEventForUnrestrictedListeners(event); - } finally { - Context.unsetForwardedEvent(); - } - verify(listenerMock).onEvent(event); - } - - @Test - public void shouldDispatchEventWhenInstanceIdsAreEqual() { - event.instanceId = gerritInstanceId; - brokerWithGerritInstanceId.fireEventForUnrestrictedListeners(event); - verify(listenerMock).onEvent(event); - } }