Move SSH command creation off NioProcessors
Creating an SSH command for an incoming user request may require
looking up group information in LDAP if the user's groups are not hot
in the cache. This can take some time and may temporarily block an
NioProcessor thread preventing network IO from occurring for other
active user sessions.
Shift command creation onto a background work queue that only does
command construction for incoming requests. This way active commands
are not blocked by LDAP group lookups.
Two threads are used to try and avoid a single LDAP lookup from
blocking all new command creation on the server.
Change-Id: I1b49a836ba3443a9a85c29b7e3156558ca34ac47
Signed-off-by: Shawn O. Pearce <sop@google.com>
diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt
index 35fc3da..59175b6 100644
--- a/Documentation/config-gerrit.txt
+++ b/Documentation/config-gerrit.txt
@@ -1695,6 +1695,14 @@
+
By default, 1 plus the number of CPUs available to the JVM.
+[sshd.commandStartThreads]]sshd.commandStartThreads::
++
+Number of threads used to parse a command line submitted by a client
+over SSH for execution, create the internal data structures used by
+that command, and schedule it for execution on another thread.
++
+By default, 2.
+
[[sshd.maxAuthTries]]sshd.maxAuthTries::
+
Maximum number of authentication attempts before the server
diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/CommandFactoryProvider.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/CommandFactoryProvider.java
index b6bb360..89338c4 100644
--- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/CommandFactoryProvider.java
+++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/CommandFactoryProvider.java
@@ -14,6 +14,8 @@
package com.google.gerrit.sshd;
+import com.google.gerrit.server.config.GerritServerConfig;
+import com.google.gerrit.server.git.WorkQueue;
import com.google.gerrit.sshd.SshScope.Context;
import com.google.inject.Inject;
import com.google.inject.Provider;
@@ -24,26 +26,38 @@
import org.apache.sshd.server.ExitCallback;
import org.apache.sshd.server.SessionAware;
import org.apache.sshd.server.session.ServerSession;
+import org.eclipse.jgit.lib.Config;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
import java.util.ArrayList;
import java.util.List;
+import java.util.concurrent.Executor;
/**
* Creates a CommandFactory using commands registered by {@link CommandModule}.
*/
class CommandFactoryProvider implements Provider<CommandFactory> {
+ private static final Logger logger = LoggerFactory
+ .getLogger(CommandFactoryProvider.class);
+
private final DispatchCommandProvider dispatcher;
private final SshLog log;
+ private final Executor startExecutor;
@Inject
CommandFactoryProvider(
@CommandName(Commands.ROOT) final DispatchCommandProvider d,
+ @GerritServerConfig final Config cfg, final WorkQueue workQueue,
final SshLog l) {
dispatcher = d;
log = l;
+
+ int threads = cfg.getInt("sshd","commandStartThreads", 2);
+ startExecutor = workQueue.createQueue(threads, "SshCommandStart");
}
@Override
@@ -62,6 +76,7 @@
private OutputStream out;
private OutputStream err;
private ExitCallback exit;
+ private Environment env;
private Context ctx;
private DispatchCommand cmd;
private boolean logged;
@@ -93,6 +108,25 @@
}
public void start(final Environment env) throws IOException {
+ this.env = env;
+ startExecutor.execute(new Runnable() {
+ public void run() {
+ try {
+ onStart();
+ } catch (Exception e) {
+ logger.warn("Cannot start command \"" + ctx.getCommandLine()
+ + "\" for user " + ctx.getSession().getUsername(), e);
+ }
+ }
+
+ @Override
+ public String toString() {
+ return "start (user " + ctx.getSession().getUsername() + ")";
+ }
+ });
+ }
+
+ private void onStart() throws IOException {
synchronized (this) {
final Context old = SshScope.set(ctx);
try {
diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/SshScope.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/SshScope.java
index 2b89d07..d32d429 100644
--- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/SshScope.java
+++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/SshScope.java
@@ -71,6 +71,10 @@
return commandLine;
}
+ SshSession getSession() {
+ return session;
+ }
+
synchronized <T> T get(Key<T> key, Provider<T> creator) {
@SuppressWarnings("unchecked")
T t = (T) map.get(key);
@@ -96,7 +100,7 @@
static class SshSessionProvider implements Provider<SshSession> {
@Override
public SshSession get() {
- return getContext().session;
+ return getContext().getSession();
}
}
