Use rescheduleDelay instead of replicationDelay when rescheduling
The replicationDelay was used for two purposes:
a) initial delay to schedule a replication
b) delay when rescheduling the replication due to an in-flight push
The replicationDelay could be set to zero, which make sense for the case a) but
doesn't make sense for the case b). Actually, when replicationDelay is set to
zero, the case b) will cause an excessive number of retries (over 1K/second) and
will also fire a replication failed event for each retry. The latter is yet
another issue and will be addressed in another change.
Introduce a new config parameter rescheduleDelay and use it for the
case b). Make sure it cannot bet set to a value lower than 3 seconds to
avoid the same issue.
In order to keep backwards compabitility with using the replicationDelay
as rescheduleDelay, a plugin init step is added. For each remote section
which doesn't already define rescheduleDelay, it will set rescheduleDelay
to the current value of the replicationDelay, unless replicationDelay is
set to zero. In the latter case it will assume the default value for
the rescheduleDelay.
Change-Id: Ia78f46460b531b04ee36ec2d5ab4228dba5c0c50
diff --git a/BUILD b/BUILD
index eb92f6d..1cad80c 100644
--- a/BUILD
+++ b/BUILD
@@ -8,6 +8,7 @@
"Implementation-Title: Replication plugin",
"Implementation-URL: https://gerrit-review.googlesource.com/#/admin/projects/plugins/replication",
"Gerrit-PluginName: replication",
+ "Gerrit-InitStep: com.googlesource.gerrit.plugins.replication.Init",
"Gerrit-Module: com.googlesource.gerrit.plugins.replication.ReplicationModule",
"Gerrit-SshModule: com.googlesource.gerrit.plugins.replication.SshModule",
],
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/Destination.java b/src/main/java/com/googlesource/gerrit/plugins/replication/Destination.java
index dd401af..f8e2d7b 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/Destination.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/Destination.java
@@ -389,7 +389,7 @@
pending.put(uri, pushOp);
switch (reason) {
case COLLISION:
- pool.schedule(pushOp, config.getDelay(), TimeUnit.SECONDS);
+ pool.schedule(pushOp, config.getRescheduleDelay(), TimeUnit.SECONDS);
break;
case TRANSPORT_ERROR:
case REPOSITORY_MISSING:
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/DestinationConfiguration.java b/src/main/java/com/googlesource/gerrit/plugins/replication/DestinationConfiguration.java
index fc109bf..856ffb1 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/DestinationConfiguration.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/DestinationConfiguration.java
@@ -20,7 +20,11 @@
import org.eclipse.jgit.transport.RemoteConfig;
class DestinationConfiguration {
+ static final int DEFAULT_REPLICATION_DELAY = 15;
+ static final int DEFAULT_RESCHEDULE_DELAY = 3;
+
private final int delay;
+ private final int rescheduleDelay;
private final int retryDelay;
private final int lockErrorMaxRetries;
private final ImmutableList<String> adminUrls;
@@ -40,7 +44,9 @@
this.remoteConfig = remoteConfig;
String name = remoteConfig.getName();
urls = ImmutableList.copyOf(cfg.getStringList("remote", name, "url"));
- delay = Math.max(0, getInt(remoteConfig, cfg, "replicationdelay", 15));
+ delay = Math.max(0, getInt(remoteConfig, cfg, "replicationdelay", DEFAULT_REPLICATION_DELAY));
+ rescheduleDelay =
+ Math.max(3, getInt(remoteConfig, cfg, "rescheduledelay", DEFAULT_RESCHEDULE_DELAY));
projects = ImmutableList.copyOf(cfg.getStringList("remote", name, "projects"));
adminUrls = ImmutableList.copyOf(cfg.getStringList("remote", name, "adminUrl"));
retryDelay = Math.max(0, getInt(remoteConfig, cfg, "replicationretry", 1));
@@ -63,6 +69,10 @@
return delay;
}
+ public int getRescheduleDelay() {
+ return rescheduleDelay;
+ }
+
public int getRetryDelay() {
return retryDelay;
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/Init.java b/src/main/java/com/googlesource/gerrit/plugins/replication/Init.java
new file mode 100644
index 0000000..a9fdb4f
--- /dev/null
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/Init.java
@@ -0,0 +1,70 @@
+// Copyright (C) 2017 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;
+
+import static com.googlesource.gerrit.plugins.replication.DestinationConfiguration.DEFAULT_REPLICATION_DELAY;
+import static com.googlesource.gerrit.plugins.replication.DestinationConfiguration.DEFAULT_RESCHEDULE_DELAY;
+
+import com.google.common.base.Strings;
+import com.google.gerrit.extensions.annotations.PluginName;
+import com.google.gerrit.pgm.init.api.ConsoleUI;
+import com.google.gerrit.pgm.init.api.InitStep;
+import com.google.gerrit.server.config.SitePaths;
+import com.google.inject.Inject;
+import java.io.File;
+import org.eclipse.jgit.storage.file.FileBasedConfig;
+import org.eclipse.jgit.util.FS;
+
+public class Init implements InitStep {
+ private final String pluginName;
+ private final SitePaths site;
+ private final ConsoleUI ui;
+
+ @Inject
+ Init(@PluginName String pluginName, SitePaths site, ConsoleUI ui) {
+ this.pluginName = pluginName;
+ this.site = site;
+ this.ui = ui;
+ }
+
+ @Override
+ public void run() throws Exception {
+ File configFile = site.etc_dir.resolve(pluginName + ".config").toFile();
+ if (!configFile.exists()) {
+ return;
+ }
+
+ FileBasedConfig config = new FileBasedConfig(configFile, FS.DETECTED);
+ config.load();
+ for (String name : config.getSubsections("remote")) {
+ if (!Strings.isNullOrEmpty(config.getString("remote", name, "rescheduleDelay"))) {
+ continue;
+ }
+
+ int replicationDelay =
+ config.getInt("remote", name, "replicationDelay", DEFAULT_REPLICATION_DELAY);
+ if (replicationDelay > 0) {
+ int delay = Math.max(replicationDelay, DEFAULT_RESCHEDULE_DELAY);
+ ui.message("Setting remote.%s.rescheduleDelay = %d\n", name, delay);
+ config.setInt("remote", name, "rescheduleDelay", delay);
+ } else {
+ ui.message(
+ "INFO: Assuming default (%d s) for remote.%s.rescheduleDelay\n",
+ DEFAULT_RESCHEDULE_DELAY, name);
+ }
+ }
+ config.save();
+ }
+}
diff --git a/src/main/resources/Documentation/config.md b/src/main/resources/Documentation/config.md
index 50664dd..d8fe9e5 100644
--- a/src/main/resources/Documentation/config.md
+++ b/src/main/resources/Documentation/config.md
@@ -216,6 +216,17 @@
By default, 15 seconds.
+remote.NAME.rescheduleDelay
+: Delay when rescheduling a push operation due to an in-flight push
+ running for the same project.
+
+ Cannot be set to a value lower than 3 seconds to avoid a tight loop
+ of schedule/run which could cause 1K+ retries per second.
+
+ A configured value lower than 3 seconds will be rounded to 3 seconds.
+
+ By default, 3 seconds.
+
remote.NAME.replicationRetry
: Time to wait before scheduling a remote push operation previously
failed due to an offline remote server.
