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>
This commit is contained in:
parent
7929d874a0
commit
d6296556c6
|
@ -1695,6 +1695,14 @@ pool by a simple FIFO scheduling system.
|
|||
+
|
||||
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
|
||||
|
|
|
@ -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.Environment;
|
|||
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 @@ class CommandFactoryProvider implements Provider<CommandFactory> {
|
|||
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 @@ class CommandFactoryProvider implements Provider<CommandFactory> {
|
|||
}
|
||||
|
||||
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 {
|
||||
|
|
|
@ -71,6 +71,10 @@ class SshScope {
|
|||
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 @@ class SshScope {
|
|||
static class SshSessionProvider implements Provider<SshSession> {
|
||||
@Override
|
||||
public SshSession get() {
|
||||
return getContext().session;
|
||||
return getContext().getSession();
|
||||
}
|
||||
}
|
||||
|
||||
|
|
Loading…
Reference in New Issue