Allow GitHub HTTP connection timeout settings
GitHub API response time can be very unstable and we don't want pending
calls to cause starvation of Gerrit threads, harming the stability
of the system. HTTP connection and read timeout can be set using the
"human readable form" values for the httpConnectionTimeout and
httpReadTimeout settings under [github] stanza.
Example:
[github]
httpConnectionTimeout = 10 seconds
httpReadTimeout = 1 minute
Change-Id: Ic19f7bf6c8b4685c363a8204c32d827ffda52ec0
diff --git a/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/GitHubHttpConnector.java b/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/GitHubHttpConnector.java
new file mode 100644
index 0000000..9881404
--- /dev/null
+++ b/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/GitHubHttpConnector.java
@@ -0,0 +1,43 @@
+// Copyright (C) 2015 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.googlesource.gerrit.plugins.github.oauth;
+
+import java.io.IOException;
+import java.net.HttpURLConnection;
+import java.net.URL;
+
+import org.kohsuke.github.HttpConnector;
+
+import com.google.inject.Inject;
+
+public class GitHubHttpConnector implements HttpConnector {
+
+ private final GitHubOAuthConfig config;
+
+ @Inject
+ public GitHubHttpConnector(final GitHubOAuthConfig config) {
+ this.config = config;
+ }
+
+ @Override
+ public HttpURLConnection connect(URL url) throws IOException {
+ HttpURLConnection huc = (HttpURLConnection) url.openConnection();
+ HttpURLConnection.setFollowRedirects(true);
+ huc.setConnectTimeout((int) config.httpConnectionTimeout);
+ huc.setReadTimeout((int) config.httpReadTimeout);
+ return huc;
+ }
+
+}
diff --git a/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/GitHubLogin.java b/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/GitHubLogin.java
index 6d04697..63ddb5d 100644
--- a/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/GitHubLogin.java
+++ b/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/GitHubLogin.java
@@ -34,6 +34,8 @@
import org.kohsuke.github.GHMyself;
import org.kohsuke.github.GitHub;
+import org.kohsuke.github.GitHubBuilder;
+import org.kohsuke.github.HttpConnector;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -67,6 +69,7 @@
private SortedSet<Scope> loginScopes;
private final GitHubOAuthConfig config;
+ private final HttpConnector httpConnector;
public GHMyself getMyself() throws IOException {
if (isLoggedIn()) {
@@ -83,8 +86,10 @@
}
@Inject
- public GitHubLogin(final GitHubOAuthConfig config) {
+ public GitHubLogin(GitHubOAuthConfig config,
+ GitHubHttpConnector httpConnector) {
this.config = config;
+ this.httpConnector = httpConnector;
}
public boolean isLoggedIn() {
@@ -140,7 +145,11 @@
if (token == null) {
return null;
}
- return GitHub.connectUsingOAuth(config.gitHubApiUrl, token.accessToken);
+ return new GitHubBuilder()
+ .withEndpoint(config.gitHubApiUrl)
+ .withOAuthToken(token.accessToken)
+ .withConnector(httpConnector)
+ .build();
}
private String getScopesKey(HttpServletRequest request,
diff --git a/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/GitHubOAuthConfig.java b/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/GitHubOAuthConfig.java
index d44480a..07b3543 100644
--- a/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/GitHubOAuthConfig.java
+++ b/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/GitHubOAuthConfig.java
@@ -17,6 +17,7 @@
import java.util.List;
import java.util.Map;
import java.util.Set;
+import java.util.concurrent.TimeUnit;
import lombok.Getter;
@@ -30,6 +31,7 @@
import com.google.gerrit.reviewdb.client.AuthType;
import com.google.gerrit.server.config.AuthConfig;
import com.google.gerrit.server.config.CanonicalWebUrl;
+import com.google.gerrit.server.config.ConfigUtil;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.inject.Inject;
import com.google.inject.Singleton;
@@ -70,6 +72,8 @@
@Getter
public final String scopeSelectionUrl;
+ public final long httpConnectionTimeout;
+ public final long httpReadTimeout;
@Inject
protected
@@ -118,6 +122,18 @@
config.getInt(CONF_SECTION, "fileUpdateMaxRetryCount", 3);
fileUpdateMaxRetryIntervalMsec =
config.getInt(CONF_SECTION, "fileUpdateMaxRetryIntervalMsec", 3000);
+
+ httpConnectionTimeout =
+ TimeUnit.MILLISECONDS.convert(
+ ConfigUtil.getTimeUnit(config,
+ CONF_SECTION, null, "httpConnectionTimeout",
+ 30, TimeUnit.SECONDS), TimeUnit.SECONDS);
+
+ httpReadTimeout =
+ TimeUnit.MILLISECONDS.convert(
+ ConfigUtil.getTimeUnit(config,
+ CONF_SECTION, null, "httpReadTimeout",
+ 30, TimeUnit.SECONDS), TimeUnit.SECONDS);
}
private Map<String, List<Scope>> getScopes(Config config) {
diff --git a/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/IdentifiedUserGitHubLoginProvider.java b/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/IdentifiedUserGitHubLoginProvider.java
index b03a61d..ebaae7b 100644
--- a/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/IdentifiedUserGitHubLoginProvider.java
+++ b/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/IdentifiedUserGitHubLoginProvider.java
@@ -39,16 +39,20 @@
+ OAuthWebFilter.GITHUB_EXT_ID;
private final Provider<IdentifiedUser> userProvider;
- private GitHubOAuthConfig config;
- private AccountCache accountCache;
+ private final GitHubOAuthConfig config;
+ private final AccountCache accountCache;
+ private final GitHubHttpConnector httpConnector;
@Inject
public IdentifiedUserGitHubLoginProvider(
- final Provider<IdentifiedUser> identifiedUserProvider,
- final GitHubOAuthConfig config, final AccountCache accountCache) {
+ Provider<IdentifiedUser> identifiedUserProvider,
+ GitHubOAuthConfig config,
+ GitHubHttpConnector httpConnector,
+ AccountCache accountCache) {
this.userProvider = identifiedUserProvider;
this.config = config;
this.accountCache = accountCache;
+ this.httpConnector = httpConnector;
}
@Override
@@ -63,7 +67,7 @@
try {
AccessToken accessToken = newAccessTokenFromUser(username);
if (accessToken != null) {
- GitHubLogin login = new GitHubLogin(config);
+ GitHubLogin login = new GitHubLogin(config, httpConnector);
login.login(accessToken);
return login;
}
diff --git a/github-plugin/src/main/resources/Documentation/config.md b/github-plugin/src/main/resources/Documentation/config.md
index 9d7c87b..e8fc5fa 100644
--- a/github-plugin/src/main/resources/Documentation/config.md
+++ b/github-plugin/src/main/resources/Documentation/config.md
@@ -66,3 +66,21 @@
for a detailed description of available scopes and their associated permissions.
Default is empty read-only access to public
information (includes public user profile info, public repository info, and gists).
+
+github.httpConnectionTimeout
+: Maximum time to wait for GitHub API to answer to a new HTTP connection attempt.
+ Values should use common common unit unit suffixes to express their setting:
+ * ms, milliseconds
+ * s, sec, second, seconds
+ * m, min, minute, minutes
+ * h, hr, hour, hours
+ Default value: 30 seconds
+
+github.httpReadTimeout
+: Maximum time to wait for GitHub API to respond or send data over an existing HTTP connection.
+ Values should use common common unit unit suffixes to express their setting:
+ * ms, milliseconds
+ * s, sec, second, seconds
+ * m, min, minute, minutes
+ * h, hr, hour, hours
+ Default value: 30 seconds
\ No newline at end of file
