transfer.timeout: Support configurable timeouts for dead clients
Broken (or simply evil) clients can open a command and hang forever, without exchanging packets with us. These connections consume a work thread from the thread pool, and tie up server memory that could be used to assist another client. By configuring transfer.timeout in gerrit.config a site administrator can now limit how long their server will wait for a single IO operation to complete before disconnecting a client with a timeout error. The timeout is currently disabled by default because it requires spinning up (and tearing down) a thread for each command executed. The thread exists only to count-down the timeout period and interrupt the blocked IO operation on the real work thread. Hopefully we can modify JGit to either support non-blocking, asynchronous IO here, or to permit using timer thread pools, to reduce the thread overhead involved with setting up a timeout for a network socket. Change-Id: Ic1608d4905082bb0639c2a0b35fd3bd9a6fccd43 Signed-off-by: Shawn O. Pearce <sop@google.com>
This commit is contained in:
@@ -1524,6 +1524,23 @@ code, or standard color name.
|
||||
+
|
||||
By default a shade of yellow, `FFFFCC`.
|
||||
|
||||
[[transfer]] Section transfer
|
||||
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
|
||||
|
||||
[[transfer.timeout]]transfer.timeout::
|
||||
+
|
||||
Number of seconds to wait for a single network read or write
|
||||
to complete before giving up and declaring the remote side is
|
||||
not responding. If 0, there is no timeout, and this server will
|
||||
wait indefinitely for a transfer to finish.
|
||||
+
|
||||
A timeout should be large enough to mostly transfer the objects to
|
||||
the other side. 1 second may be too small for larger projects,
|
||||
especially over a WAN link, while 10-30 seconds is a much more
|
||||
reasonable timeout value.
|
||||
+
|
||||
Defaults to 0 seconds, wait indefinitely.
|
||||
|
||||
[[user]] Section user
|
||||
~~~~~~~~~~~~~~~~~~~~~
|
||||
|
||||
|
||||
@@ -81,6 +81,7 @@ public class SshModule extends FactoryModule {
|
||||
bind(PublickeyAuthenticator.class).to(DatabasePubKeyAuth.class);
|
||||
bind(PasswordAuthenticator.class).to(DatabasePasswordAuth.class);
|
||||
bind(KeyPairProvider.class).toProvider(HostKeyProvider.class).in(SINGLETON);
|
||||
bind(TransferConfig.class);
|
||||
|
||||
install(new DefaultCommandModule());
|
||||
|
||||
|
||||
@@ -0,0 +1,40 @@
|
||||
// Copyright (C) 2010 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.google.gerrit.sshd;
|
||||
|
||||
import com.google.gerrit.server.config.ConfigUtil;
|
||||
import com.google.gerrit.server.config.GerritServerConfig;
|
||||
import com.google.inject.Inject;
|
||||
import com.google.inject.Singleton;
|
||||
|
||||
import org.eclipse.jgit.lib.Config;
|
||||
|
||||
import java.util.concurrent.TimeUnit;
|
||||
|
||||
@Singleton
|
||||
public class TransferConfig {
|
||||
private final int timeout;
|
||||
|
||||
@Inject
|
||||
TransferConfig(@GerritServerConfig final Config cfg) {
|
||||
timeout = (int) ConfigUtil.getTimeUnit(cfg, "transfer", null, "timeout", //
|
||||
0, TimeUnit.SECONDS);
|
||||
}
|
||||
|
||||
/** @return configured timeout, in seconds. 0 if the timeout is infinite. */
|
||||
public int getTimeout() {
|
||||
return timeout;
|
||||
}
|
||||
}
|
||||
@@ -18,12 +18,14 @@ import com.google.gerrit.reviewdb.Account;
|
||||
import com.google.gerrit.server.IdentifiedUser;
|
||||
import com.google.gerrit.server.git.ReceiveCommits;
|
||||
import com.google.gerrit.sshd.AbstractGitCommand;
|
||||
import com.google.gerrit.sshd.TransferConfig;
|
||||
import com.google.inject.Inject;
|
||||
|
||||
import org.eclipse.jgit.transport.ReceivePack;
|
||||
import org.kohsuke.args4j.Option;
|
||||
|
||||
import java.io.IOException;
|
||||
import java.io.InterruptedIOException;
|
||||
import java.util.HashSet;
|
||||
import java.util.Set;
|
||||
|
||||
@@ -38,6 +40,9 @@ final class Receive extends AbstractGitCommand {
|
||||
@Inject
|
||||
private IdentifiedUser.GenericFactory identifiedUserFactory;
|
||||
|
||||
@Inject
|
||||
private TransferConfig config;
|
||||
|
||||
private final Set<Account.Id> reviewerId = new HashSet<Account.Id>();
|
||||
private final Set<Account.Id> ccId = new HashSet<Account.Id>();
|
||||
|
||||
@@ -68,7 +73,12 @@ final class Receive extends AbstractGitCommand {
|
||||
|
||||
final ReceivePack rp = receive.getReceivePack();
|
||||
rp.setRefLogIdent(currentUser.newRefLogIdent());
|
||||
rp.receive(in, out, err);
|
||||
rp.setTimeout(config.getTimeout());
|
||||
try {
|
||||
rp.receive(in, out, err);
|
||||
} catch (InterruptedIOException err) {
|
||||
throw new Failure(128, "fatal: client IO read/write timeout", err);
|
||||
}
|
||||
}
|
||||
|
||||
private void verifyProjectVisible(final String type, final Set<Account.Id> who)
|
||||
|
||||
@@ -17,24 +17,34 @@ package com.google.gerrit.sshd.commands;
|
||||
import com.google.gerrit.reviewdb.ReviewDb;
|
||||
import com.google.gerrit.server.git.VisibleRefFilter;
|
||||
import com.google.gerrit.sshd.AbstractGitCommand;
|
||||
import com.google.gerrit.sshd.TransferConfig;
|
||||
import com.google.inject.Inject;
|
||||
import com.google.inject.Provider;
|
||||
|
||||
import org.eclipse.jgit.transport.UploadPack;
|
||||
|
||||
import java.io.IOException;
|
||||
import java.io.InterruptedIOException;
|
||||
|
||||
/** Publishes Git repositories over SSH using the Git upload-pack protocol. */
|
||||
final class Upload extends AbstractGitCommand {
|
||||
@Inject
|
||||
private Provider<ReviewDb> db;
|
||||
|
||||
@Inject
|
||||
private TransferConfig config;
|
||||
|
||||
@Override
|
||||
protected void runImpl() throws IOException {
|
||||
protected void runImpl() throws IOException, Failure {
|
||||
final UploadPack up = new UploadPack(repo);
|
||||
if (!projectControl.allRefsAreVisible()) {
|
||||
up.setRefFilter(new VisibleRefFilter(repo, projectControl, db.get()));
|
||||
}
|
||||
up.upload(in, out, err);
|
||||
up.setTimeout(config.getTimeout());
|
||||
try {
|
||||
up.upload(in, out, err);
|
||||
} catch (InterruptedIOException err) {
|
||||
throw new Failure(128, "fatal: client IO read/write timeout", err);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user