Use rescheduleDelay instead of replicationDelay when reschedulingv2.14.2

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

5 files changed, 94 insertions, 2 deletions

diff --git a/BUILD b/BUILD
index eb92f6d..1cad80c 100644
--- a/BUILD
+++ b/BUILD
@@ -8,6 +8,7 @@ gerrit_plugin(
         "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 @@ public class Destination {
         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.lib.Config;
 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 @@ class DestinationConfiguration {
     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 @@ class DestinationConfiguration {
     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 @@ remote.NAME.replicationDelay
 
 	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.