Introduce the `remote.NAME.fetchEvery` configuration param
Currently, there are three methods for obtaining repository updates in
pull-replication:
* Apply objects - This transfers updated refs and objects via the REST
API; optimisation on the top of Git fetch for small payloads and for
subset of refs
* Trigger git fetch (as configured preference) or fallback in all cases
when apply object is not suitable or fails
* Event tiggered through the event-broker.
The parameter (if configured and when implemented) enables the fourth,
time-based (every `n` seconds) method, that is independent of the
REST API. It will periodically invoke git fetch to detect and retrieve
new data as it becomes available. Note that it is meant for the remote
that doesn't offer any events or webhooks to provide information about
new data.
The default parameter value is `0s` which means that periodical fetch
is disabled. Note that larger time units (`m`, `h`, etc...) can be
used to configure it conveniently.
Note that enbaling periodic fetch and REST API (IOW having both `apiUrl`
and `fetchEvery` configured) may lead to racy writes to the repository
and as such is considered an invalid configuration. Such configuration
results in reported as error and prevents the plugin from starting.
Bug: Issue 322146240
Change-Id: I23fc5e23aabff2dd0f053de235245614cf4706c9
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/SourceConfigParser.java b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/SourceConfigParser.java
index 26a754a..2ff9189 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/SourceConfigParser.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/SourceConfigParser.java
@@ -80,6 +80,22 @@
}
}
}
+
+ if (sourceConfig.fetchEvery() > SourceConfiguration.DEFAULT_PERIODIC_FETCH_DISABLED
+ && !sourceConfig.getApis().isEmpty()) {
+ logger.atSevere().log(
+ "Receiving updates through periodic fetch (every %ds) and from Gerrit API(s) (%s) as a result of "
+ + "received events (in %s node) may result in racy writes to the repo (in extreme cases to its "
+ + "corruption). Periodic fetch is meant ONLY for remote that that doesn't offer events or "
+ + "webhooks that could be used otherwise for new data detection.",
+ sourceConfig.fetchEvery(), sourceConfig.getApis(), c.getName());
+ throw new ConfigInvalidException(
+ String.format(
+ "The [%s] remote has both 'fetchEvery' (every %ds) and `apiUrl` (%s) set which is "
+ + "considered an invalid configuration.",
+ c.getName(), sourceConfig.fetchEvery(), sourceConfig.getApis()));
+ }
+
sourceConfigs.add(sourceConfig);
}
return sourceConfigs.build();
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/SourceConfiguration.java b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/SourceConfiguration.java
index d481dbb..faba805 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/SourceConfiguration.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/SourceConfiguration.java
@@ -32,6 +32,7 @@
static final int DEFAULT_CONNECTION_TIMEOUT_MS = 5000;
static final int DEFAULT_CONNECTIONS_PER_ROUTE = 100;
static final int DEFAULT_DRAIN_SHUTDOWN_TIMEOUT_SECS = 300;
+ static final long DEFAULT_PERIODIC_FETCH_DISABLED = 0L;
private final int delay;
private final int rescheduleDelay;
@@ -59,6 +60,7 @@
private boolean useCGitClient;
private int refsBatchSize;
private boolean enableBatchedRefs;
+ private final long fetchEvery;
public SourceConfiguration(RemoteConfig remoteConfig, Config cfg) {
this.remoteConfig = remoteConfig;
@@ -130,6 +132,10 @@
+ "details on the `enableBatchedRefs` configuration.",
name);
}
+
+ fetchEvery =
+ cfg.getTimeUnit(
+ "remote", name, "fetchEvery", DEFAULT_PERIODIC_FETCH_DISABLED, TimeUnit.SECONDS);
}
@Override
@@ -261,4 +267,8 @@
public boolean enableBatchedRefs() {
return enableBatchedRefs;
}
+
+ public long fetchEvery() {
+ return fetchEvery;
+ }
}
diff --git a/src/main/resources/Documentation/config.md b/src/main/resources/Documentation/config.md
index 3b76be9..5e791d4 100644
--- a/src/main/resources/Documentation/config.md
+++ b/src/main/resources/Documentation/config.md
@@ -616,6 +616,21 @@
>
> By default, true.
+remote.NAME.fetchEvery
+: Fetch the ref-spec `remote.NAME.fetch` from the remote repository defined
+ at `remote.NAME.url` for remote changes every `n` seconds.
+ By default set to `0s` which means that polling is disabled. Note that
+ larger time units (`m`, `h`, etc...) can be used to specify it conveniently.
+
+ Setting `fetchEvery` to value greater than `0` means that It will,
+ periodically, invoke git fetch to detect and retrieve new data from the
+ remote.
+
+> *NOTE:* it is meant for the remote that doesn't offer events or webhooks
+> that could be used otherwise for new data detection and cannot be configured
+> together with `remote.NAME.apiUrl`; such configuration is considered
+> invalid and prevents the plugin from starting
+
Directory `replication`
--------------------
The optional directory `$site_path/etc/replication` contains Git-style
diff --git a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/SourceConfigParserTest.java b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/SourceConfigParserTest.java
new file mode 100644
index 0000000..306153a
--- /dev/null
+++ b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/SourceConfigParserTest.java
@@ -0,0 +1,57 @@
+// Copyright (C) 2024 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.replication.pull;
+
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.testing.GerritJUnit.assertThrows;
+
+import com.google.inject.util.Providers;
+import com.googlesource.gerrit.plugins.replication.ReplicationConfig;
+import org.eclipse.jgit.errors.ConfigInvalidException;
+import org.eclipse.jgit.lib.Config;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.junit.MockitoJUnitRunner;
+
+@RunWith(MockitoJUnitRunner.class)
+public class SourceConfigParserTest {
+ @Mock ReplicationConfig replicationConfigMock;
+
+ private SourceConfigParser objectUnderTest;
+
+ @Before
+ public void setup() {
+ objectUnderTest = new SourceConfigParser(false, Providers.of(replicationConfigMock));
+ }
+
+ @Test
+ public void shouldThrowWhenBothFetchEveryAndApiUrlsConfigured() {
+ // given
+ Config config = new Config();
+ config.setString("remote", "test_remote", "fetch", "+refs/*:refs/*");
+ config.setString("remote", "test_remote", "projects", "to_be_replicated");
+
+ config.setString("remote", "test_remote", "apiUrl", "http://foo.bar/api");
+ config.setString("remote", "test_remote", "fetchEvery", "1s");
+
+ // when/then
+ String errorMessage =
+ assertThrows(ConfigInvalidException.class, () -> objectUnderTest.parseRemotes(config))
+ .getMessage();
+ assertThat(errorMessage).contains("invalid configuration");
+ }
+}
