From 624ec820682e0d7b163e1de262134b7b94087441 Mon Sep 17 00:00:00 2001 From: Dariusz Luksza Date: Thu, 3 Sep 2015 17:27:50 +0900 Subject: 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 Signed-off-by: David Pursehouse --- .../replication/AutoReloadConfigDecorator.java | 10 ++++++++++ .../plugins/replication/ReplicationConfig.java | 4 ++++ .../replication/ReplicationFileBasedConfig.java | 22 ++++++++++++++++++++++ .../gerrit/plugins/replication/SshHelper.java | 13 ++++++++----- src/main/resources/Documentation/config.md | 8 ++++++++ 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 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 sshSessionFactoryProvider; + private final int commandTimeout; + private final int connectionTimeout; @Inject - SshHelper(Provider sshSessionFactoryProvider) { + SshHelper( + ReplicationConfig replicationConfig, Provider 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. -- cgit v1.2.3