Allow to configure timeout for SSH connection and commands

The timeout for SSH command execution is hard-coded to 0 which means
no timeout, and the timeout for SSH connections is hard-coded to 2
minutes.

Introduce new configuration options to allow configuring each timeout
individually:

  gerrit.sshCommandTimeout
  gerrit.sshConnectionTimeout

When not set, both default to their previously hard-coded values,
i.e. no limit and 2 minutes respectively.

Change-Id: Ibd377d45543ef4631082e8d521aeeb99209003f7
Signed-off-by: Dariusz Luksza <dluksza@collab.net>
Signed-off-by: David Pursehouse <dpursehouse@collab.net>

5 files changed, 52 insertions, 5 deletions

diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/AutoReloadConfigDecorator.java b/src/main/java/com/googlesource/gerrit/plugins/replication/AutoReloadConfigDecorator.java
index 8b6b8fc..5d6e409 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/AutoReloadConfigDecorator.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/AutoReloadConfigDecorator.java
@@ -110,4 +110,14 @@ public class AutoReloadConfigDecorator implements ReplicationConfig {
   public synchronized void startup(WorkQueue workQueue) {
     currentConfig.startup(workQueue);
   }
+
+  @Override
+  public synchronized int getSshConnectionTimeout() {
+    return currentConfig.getSshConnectionTimeout();
+  }
+
+  @Override
+  public synchronized int getSshCommandTimeout() {
+    return currentConfig.getSshCommandTimeout();
+  }
 }
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationConfig.java b/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationConfig.java
index e94abbd..869a49b 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationConfig.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationConfig.java
@@ -35,4 +35,8 @@ public interface ReplicationConfig {
   int shutdown();
 
   void startup(WorkQueue workQueue);
+
+  int getSshConnectionTimeout();
+
+  int getSshCommandTimeout();
 }
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationFileBasedConfig.java b/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationFileBasedConfig.java
index ee1f16d..e641be2 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationFileBasedConfig.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationFileBasedConfig.java
@@ -13,10 +13,13 @@
 // limitations under the License.
 package com.googlesource.gerrit.plugins.replication;
 
+import static java.util.concurrent.TimeUnit.MINUTES;
+import static java.util.concurrent.TimeUnit.SECONDS;
 import static java.util.stream.Collectors.toList;
 
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Lists;
+import com.google.gerrit.server.config.ConfigUtil;
 import com.google.gerrit.server.config.SitePaths;
 import com.google.gerrit.server.git.WorkQueue;
 import com.google.inject.Inject;
@@ -45,6 +48,8 @@ public class ReplicationFileBasedConfig implements ReplicationConfig {
   private Path cfgPath;
   private boolean replicateAllOnPluginStart;
   private boolean defaultForceUpdate;
+  private int sshCommandTimeout;
+  private int sshConnectionTimeout;
   private final FileBasedConfig config;
 
   @Inject
@@ -104,6 +109,13 @@ public class ReplicationFileBasedConfig implements ReplicationConfig {
 
     defaultForceUpdate = config.getBoolean("gerrit", "defaultForceUpdate", false);
 
+    sshCommandTimeout =
+        (int) ConfigUtil.getTimeUnit(config, "gerrit", null, "sshCommandTimeout", 0, SECONDS);
+    sshConnectionTimeout =
+        (int)
+            SECONDS.toMillis(
+                ConfigUtil.getTimeUnit(config, "gerrit", null, "sshConnectionTimeout", 2, MINUTES));
+
     ImmutableList.Builder<Destination> dest = ImmutableList.builder();
     for (RemoteConfig c : allRemotes(config)) {
       if (c.getURIs().isEmpty()) {
@@ -203,4 +215,14 @@ public class ReplicationFileBasedConfig implements ReplicationConfig {
       cfg.start(workQueue);
     }
   }
+
+  @Override
+  public int getSshConnectionTimeout() {
+    return sshConnectionTimeout;
+  }
+
+  @Override
+  public int getSshCommandTimeout() {
+    return sshCommandTimeout;
+  }
 }
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/SshHelper.java b/src/main/java/com/googlesource/gerrit/plugins/replication/SshHelper.java
index 68e9652..46f6ac0 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/SshHelper.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/SshHelper.java
@@ -26,18 +26,21 @@ import org.eclipse.jgit.util.FS;
 import org.eclipse.jgit.util.io.StreamCopyThread;
 
 class SshHelper {
-  private static final int SSH_REMOTE_TIMEOUT = 120 * 1000; // 2 minutes = 120 * 1000ms
-
   private final Provider<SshSessionFactory> sshSessionFactoryProvider;
+  private final int commandTimeout;
+  private final int connectionTimeout;
 
   @Inject
-  SshHelper(Provider<SshSessionFactory> sshSessionFactoryProvider) {
+  SshHelper(
+      ReplicationConfig replicationConfig, Provider<SshSessionFactory> sshSessionFactoryProvider) {
     this.sshSessionFactoryProvider = sshSessionFactoryProvider;
+    this.commandTimeout = replicationConfig.getSshCommandTimeout();
+    this.connectionTimeout = replicationConfig.getSshConnectionTimeout();
   }
 
   int executeRemoteSsh(URIish uri, String cmd, OutputStream errStream) throws IOException {
     RemoteSession ssh = connect(uri);
-    Process proc = ssh.exec(cmd, 0);
+    Process proc = ssh.exec(cmd, commandTimeout);
     proc.getOutputStream().close();
     StreamCopyThread out = new StreamCopyThread(proc.getInputStream(), errStream);
     StreamCopyThread err = new StreamCopyThread(proc.getErrorStream(), errStream);
@@ -84,6 +87,6 @@ class SshHelper {
   }
 
   RemoteSession connect(URIish uri) throws TransportException {
-    return sshSessionFactoryProvider.get().getSession(uri, null, FS.DETECTED, SSH_REMOTE_TIMEOUT);
+    return sshSessionFactoryProvider.get().getSession(uri, null, FS.DETECTED, connectionTimeout);
   }
 }
diff --git a/src/main/resources/Documentation/config.md b/src/main/resources/Documentation/config.md
index c066513..e70094c 100644
--- a/src/main/resources/Documentation/config.md
+++ b/src/main/resources/Documentation/config.md
@@ -77,6 +77,14 @@ gerrit.defaultForceUpdate
 :	If true, the default push refspec will be set to use forced
 	update to the remote when no refspec is given.  By default, false.
 
+gerrit.sshCommandTimeout
+:	Timeout for SSH command execution. If 0, there is no timeout and
+	the client waits indefinitely. By default, 0.
+
+gerrit.sshConnectionTimeout
+:	Timeout for SSH connections. If 0, there is no timeout and
+        the client waits indefinitely. By default, 2 minutes.
+
 replication.lockErrorMaxRetries
 :	Number of times to retry a replication operation if a lock
 	error is detected.