Introduce readTimeout and connectTimeout to Jira
Define explicit timeouts for connection and read from Jira
backend. Allows to prevent incoming git-receive-pack threads
to become blocked because of a slowdown of connectivity
or execution times on the Jira side.
Bug: Issue 15702
Change-Id: Id94892a50363d5be3fee4007c1357cf64927ce41
diff --git a/src/main/java/com/googlesource/gerrit/plugins/its/jira/InitJira.java b/src/main/java/com/googlesource/gerrit/plugins/its/jira/InitJira.java
index a5c16ec..ccab69c 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/its/jira/InitJira.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/its/jira/InitJira.java
@@ -21,6 +21,7 @@
import com.google.gerrit.pgm.init.api.ConsoleUI;
import com.google.gerrit.pgm.init.api.InitFlags;
import com.google.gerrit.pgm.init.api.Section;
+import com.google.gerrit.server.config.ConfigUtil;
import com.google.inject.Inject;
import com.google.inject.Singleton;
import com.googlesource.gerrit.plugins.its.base.its.InitIts;
@@ -30,7 +31,9 @@
import com.googlesource.gerrit.plugins.its.jira.restapi.JiraURL;
import java.io.IOException;
import java.net.MalformedURLException;
+import java.time.Duration;
import java.util.Arrays;
+import java.util.concurrent.TimeUnit;
import org.eclipse.jgit.errors.ConfigInvalidException;
/** Initialize the GitRepositoryManager configuration section. */
@@ -43,6 +46,8 @@
private JiraURL jiraUrl;
private String jiraUsername;
private String jiraPassword;
+ private String jiraConnectionTimeout;
+ private String jiraReadTimeout;
@Inject
InitJira(
@@ -123,9 +128,24 @@
public void enterJiraConnectivity() throws MalformedURLException {
String jiraUrlString = jira.string("Jira URL (empty to skip)", "url", null);
if (jiraUrlString != null) {
- jiraUrl = new JiraURL(jiraUrlString);
jiraUsername = jira.string("Jira username", "username", "");
jiraPassword = jira.password("username", "password");
+ jiraConnectionTimeout =
+ jira.string(
+ "Connection timeout",
+ JiraConfig.GERRIT_CONFIG_CONNECT_TIMEOUT,
+ JiraConfig.GERRIT_CONFIG_CONNECT_TIMEOUT_DEFAULT.toMillis() + " ms");
+ jiraReadTimeout =
+ jira.string(
+ "Read timeout",
+ JiraConfig.GERRIT_CONFIG_READ_TIMEOUT,
+ JiraConfig.GERRIT_CONFIG_READ_TIMEOUT_DEFAULT.toMillis() + " ms");
+ jiraUrl =
+ new JiraURL(
+ jiraUrlString,
+ Duration.ofMillis(
+ ConfigUtil.getTimeUnit(jiraConnectionTimeout, 0, TimeUnit.MILLISECONDS)),
+ Duration.ofMillis(ConfigUtil.getTimeUnit(jiraReadTimeout, 0, TimeUnit.MILLISECONDS)));
}
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/its/jira/JiraConfig.java b/src/main/java/com/googlesource/gerrit/plugins/its/jira/JiraConfig.java
index 339d16c..b0e8bcb 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/its/jira/JiraConfig.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/its/jira/JiraConfig.java
@@ -21,6 +21,7 @@
import com.google.gerrit.entities.StoredCommentLinkInfo;
import com.google.gerrit.extensions.annotations.PluginName;
import com.google.gerrit.server.GerritPersonIdent;
+import com.google.gerrit.server.config.ConfigUtil;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.config.PluginConfig;
import com.google.gerrit.server.config.PluginConfigFactory;
@@ -35,6 +36,8 @@
import com.googlesource.gerrit.plugins.its.jira.restapi.JiraURL;
import com.googlesource.gerrit.plugins.its.jira.restapi.JiraVisibilityType;
import java.io.IOException;
+import java.time.Duration;
+import java.util.concurrent.TimeUnit;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.PersonIdent;
@@ -58,6 +61,10 @@
private static final String GERRIT_CONFIG_PASSWORD = "password";
private static final String GERRIT_CONFIG_COMMENT_VISIBILITY_TYPE = "visibilityType";
private static final String GERRIT_CONFIG_COMMENT_VISIBILITY_VALUE = "visibilityValue";
+ public static final String GERRIT_CONFIG_CONNECT_TIMEOUT = "connectTimeout";
+ public static final Duration GERRIT_CONFIG_CONNECT_TIMEOUT_DEFAULT = Duration.ofMinutes(2);
+ public static final String GERRIT_CONFIG_READ_TIMEOUT = "readTimeout";
+ public static final Duration GERRIT_CONFIG_READ_TIMEOUT_DEFAULT = Duration.ofSeconds(30);
private final String pluginName;
private final PluginConfigFactory cfgFactory;
@@ -96,9 +103,33 @@
gerritConfig.getEnum(
pluginName, null, GERRIT_CONFIG_COMMENT_VISIBILITY_TYPE, JiraVisibilityType.NOTSET),
gerritConfig.getString(pluginName, null, GERRIT_CONFIG_COMMENT_VISIBILITY_VALUE))
+ .connectTimeout(
+ getDurationFromConfig(
+ gerritConfig,
+ pluginName,
+ GERRIT_CONFIG_CONNECT_TIMEOUT,
+ GERRIT_CONFIG_CONNECT_TIMEOUT_DEFAULT))
+ .readTimeout(
+ getDurationFromConfig(
+ gerritConfig,
+ pluginName,
+ GERRIT_CONFIG_READ_TIMEOUT,
+ GERRIT_CONFIG_READ_TIMEOUT_DEFAULT))
.build();
}
+ private static Duration getDurationFromConfig(
+ Config gerritConfig, String pluginName, String setting, Duration defaultDuration) {
+ return Duration.ofMillis(
+ ConfigUtil.getTimeUnit(
+ gerritConfig,
+ pluginName,
+ null,
+ setting,
+ defaultDuration.toMillis(),
+ TimeUnit.MILLISECONDS));
+ }
+
JiraItsServerInfo getDefaultServerInfo() {
return defaultJiraServerInfo;
}
@@ -119,9 +150,24 @@
PROJECT_CONFIG_COMMENT_VISIBILITY_TYPE,
JiraVisibilityType.NOTSET),
pluginConfig.getString(PROJECT_CONFIG_COMMENT_VISIBILITY_VALUE, null))
+ .connectTimeout(
+ getDurationFromConfig(
+ pluginConfig, GERRIT_CONFIG_CONNECT_TIMEOUT, GERRIT_CONFIG_CONNECT_TIMEOUT_DEFAULT))
+ .readTimeout(
+ getDurationFromConfig(
+ pluginConfig, GERRIT_CONFIG_READ_TIMEOUT, GERRIT_CONFIG_READ_TIMEOUT_DEFAULT))
.build();
}
+ private static Duration getDurationFromConfig(
+ PluginConfig pluginConfig, String setting, Duration defaultDuration) {
+ return Duration.ofMillis(
+ ConfigUtil.getTimeUnit(
+ pluginConfig.getString(setting, ""),
+ defaultDuration.toMillis(),
+ TimeUnit.MILLISECONDS));
+ }
+
void addCommentLinksSection(Project.NameKey projectName, JiraItsServerInfo jiraItsServerInfo) {
try (Repository git = repoManager.openRepository(projectName);
MetaDataUpdate md = new MetaDataUpdate(GitReferenceUpdated.DISABLED, projectName, git)) {
diff --git a/src/main/java/com/googlesource/gerrit/plugins/its/jira/JiraItsServerInfo.java b/src/main/java/com/googlesource/gerrit/plugins/its/jira/JiraItsServerInfo.java
index 4ffb72d..5a92d84 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/its/jira/JiraItsServerInfo.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/its/jira/JiraItsServerInfo.java
@@ -15,25 +15,27 @@
package com.googlesource.gerrit.plugins.its.jira;
+import static com.google.common.base.Preconditions.checkNotNull;
+
import com.googlesource.gerrit.plugins.its.jira.restapi.JiraURL;
import com.googlesource.gerrit.plugins.its.jira.restapi.JiraVisibility;
import com.googlesource.gerrit.plugins.its.jira.restapi.JiraVisibilityType;
import java.net.MalformedURLException;
+import java.time.Duration;
public class JiraItsServerInfo {
public static class Builder {
private JiraItsServerInfo instance = new JiraItsServerInfo();
+ private String projectUrl;
+ private Duration connectTimeout;
+ private Duration readTimeout;
private Builder() {}
public Builder url(String projectUrl) {
- try {
- instance.url = projectUrl != null ? new JiraURL(projectUrl) : null;
- return this;
- } catch (MalformedURLException e) {
- throw new IllegalArgumentException("Unable to resolve URL", e);
- }
+ this.projectUrl = projectUrl;
+ return this;
}
public Builder username(String username) {
@@ -55,8 +57,26 @@
return this;
}
+ public Builder connectTimeout(Duration timeout) {
+ this.connectTimeout = timeout;
+ return this;
+ }
+
+ public Builder readTimeout(Duration timeout) {
+ this.readTimeout = timeout;
+ return this;
+ }
+
public JiraItsServerInfo build() {
- return instance;
+ try {
+ checkNotNull(connectTimeout, "Missing connection timeout");
+ checkNotNull(readTimeout, "Missing read timeout");
+ instance.url =
+ projectUrl != null ? new JiraURL(projectUrl, connectTimeout, readTimeout) : null;
+ return instance;
+ } catch (MalformedURLException e) {
+ throw new IllegalArgumentException("Unable to resolve URL", e);
+ }
}
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/its/jira/JiraUrlProjectConfigEntry.java b/src/main/java/com/googlesource/gerrit/plugins/its/jira/JiraUrlProjectConfigEntry.java
index 40c248c..247720e 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/its/jira/JiraUrlProjectConfigEntry.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/its/jira/JiraUrlProjectConfigEntry.java
@@ -37,7 +37,7 @@
public ConfigValue preUpdate(ConfigValue configValue) {
if (configValue.value != null && !configValue.value.isEmpty()) {
try {
- new JiraURL(configValue.value);
+ JiraURL.validateUrl(configValue.value);
} catch (MalformedURLException e) {
configValue.value = INVALID_URL_MSG;
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/its/jira/restapi/JiraRestApi.java b/src/main/java/com/googlesource/gerrit/plugins/its/jira/restapi/JiraRestApi.java
index ceec13d..ef4c43f 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/its/jira/restapi/JiraRestApi.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/its/jira/restapi/JiraRestApi.java
@@ -70,7 +70,7 @@
* @throws IOException generated if unexpected failCode is returned
*/
public T doGet(String spec, int passCode, int[] failCodes) throws IOException {
- HttpURLConnection conn = prepHttpConnection(spec, "GET", false);
+ HttpURLConnection conn = openConnection(spec, "GET", false);
try {
if (validateResponse(conn, passCode, failCodes)) {
readIncomingData(conn);
@@ -99,18 +99,8 @@
return sendPayload("PUT", spec, jsonInput, passCode);
}
- private boolean sendPayload(String method, String spec, String jsonInput, int passCode)
- throws IOException {
- HttpURLConnection conn = prepHttpConnection(spec, method, true);
- try {
- writeBodyData(jsonInput, conn);
- return validateResponse(conn, passCode, null);
- } finally {
- conn.disconnect();
- }
- }
-
- private HttpURLConnection prepHttpConnection(String spec, String method, boolean withPayload)
+ /** Open an HTTP connection with the Jira's endpoint URL. */
+ public HttpURLConnection openConnection(String spec, String method, boolean withPayload)
throws IOException {
JiraURL url = baseUrl.withSpec(spec);
ProxySelector proxySelector = ProxySelector.getDefault();
@@ -125,6 +115,17 @@
return conn;
}
+ private boolean sendPayload(String method, String spec, String jsonInput, int passCode)
+ throws IOException {
+ HttpURLConnection conn = openConnection(spec, method, true);
+ try {
+ writeBodyData(jsonInput, conn);
+ return validateResponse(conn, passCode, null);
+ } finally {
+ conn.disconnect();
+ }
+ }
+
/** Write the data to the HTTP connection. */
private void writeBodyData(String data, HttpURLConnection conn) throws IOException {
if (data != null) {
diff --git a/src/main/java/com/googlesource/gerrit/plugins/its/jira/restapi/JiraURL.java b/src/main/java/com/googlesource/gerrit/plugins/its/jira/restapi/JiraURL.java
index 0de96d8..6f31f39 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/its/jira/restapi/JiraURL.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/its/jira/restapi/JiraURL.java
@@ -23,6 +23,7 @@
import java.net.Proxy;
import java.net.ProxySelector;
import java.net.URL;
+import java.time.Duration;
import java.util.Objects;
import org.eclipse.jgit.util.HttpSupport;
import org.slf4j.Logger;
@@ -36,18 +37,31 @@
private final URL url;
- public JiraURL(String spec) throws MalformedURLException {
- this.url = new URL(CharMatcher.is('/').trimFrom(spec) + "/");
+ private final int connectionTimeoutMs;
+
+ private final int readTimeoutMs;
+
+ public static URL validateUrl(String spec) throws MalformedURLException {
+ return new URL(CharMatcher.is('/').trimFrom(spec) + "/");
}
- private JiraURL(URL url) {
+ public JiraURL(String spec, Duration connectionTimeout, Duration readTimeout)
+ throws MalformedURLException {
+ this.url = validateUrl(spec);
+ this.connectionTimeoutMs = (int) connectionTimeout.toMillis();
+ this.readTimeoutMs = (int) readTimeout.toMillis();
+ }
+
+ private JiraURL(URL url, int connectionTimeoutMs, int readTimeoutMs) {
this.url = requireNonNull(url);
+ this.connectionTimeoutMs = connectionTimeoutMs;
+ this.readTimeoutMs = readTimeoutMs;
}
public JiraURL resolveUrl(String... paths) {
String relativePath = String.join("", paths);
try {
- return new JiraURL(new URL(url, relativePath));
+ return new JiraURL(new URL(url, relativePath), connectionTimeoutMs, readTimeoutMs);
} catch (MalformedURLException e) {
log.error("Unexpected exception while composing URL {} with path {}", url, relativePath, e);
throw new IllegalArgumentException(e);
@@ -63,12 +77,15 @@
}
public JiraURL withSpec(String spec) throws MalformedURLException {
- return new JiraURL(new URL(url, spec));
+ return new JiraURL(new URL(url, spec), connectionTimeoutMs, readTimeoutMs);
}
public HttpURLConnection openConnection(ProxySelector proxySelector) throws IOException {
Proxy proxy = HttpSupport.proxyFor(proxySelector, url);
- return (HttpURLConnection) url.openConnection(proxy);
+ HttpURLConnection conn = (HttpURLConnection) url.openConnection(proxy);
+ conn.setConnectTimeout(connectionTimeoutMs);
+ conn.setReadTimeout(readTimeoutMs);
+ return conn;
}
public String getPath() {
diff --git a/src/main/resources/Documentation/config.md b/src/main/resources/Documentation/config.md
index a86b80c..4827dfd 100644
--- a/src/main/resources/Documentation/config.md
+++ b/src/main/resources/Documentation/config.md
@@ -101,6 +101,24 @@
Jira credentials and connectivity details are asked and verified during the Gerrit init.
+Timeouts
+--------
+
+The HTTP connection and read timeouts are configurable in every place where the Jira URL is specified,
+either in the `gerrit.config` or `project.config`, whichever has the precedence.
+
+`connectTimeout`
+: Timeout period for making a new HTTP connection to the Jira URL.
+ The value is in the usual time-unit format like "1 s", "100 ms", etc.
+ By default `2 minutes`.
+
+`readTimeout`
+: The read timeout for the HTTP operation to complete.
+ The value is in the usual time-unit format like "1 s", "100 ms", etc.
+ A timeout can be used to avoid blocking all of the incoming Git receive-packs threads in case
+ Jira server becomes slow.
+ By default `30 seconds`.
+
Gerrit init integration
-----------------------
diff --git a/src/test/java/com/googlesource/gerrit/plugins/its/jira/restapi/JiraRestApiTest.java b/src/test/java/com/googlesource/gerrit/plugins/its/jira/restapi/JiraRestApiTest.java
index d3b4812..27c5fc7 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/its/jira/restapi/JiraRestApiTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/its/jira/restapi/JiraRestApiTest.java
@@ -22,6 +22,7 @@
import java.io.ByteArrayOutputStream;
import java.net.HttpURLConnection;
import java.net.MalformedURLException;
+import java.time.Duration;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.junit.MockitoJUnitRunner;
@@ -33,17 +34,20 @@
private static final String JSON_PAYLOAD = "{}";
private static final String USERNAME = "user";
private static final String PASSWORD = "pass";
+ private static final Duration CONNECTION_TIMEOUT = Duration.ofSeconds(30);
+ private static final Duration READ_TIMEOUT = Duration.ofSeconds(30);
private JiraURL url;
private JiraRestApi restApi;
- private void setURL(String jiraUrl) throws MalformedURLException {
- url = new JiraURL(jiraUrl);
+ private void setURL(String jiraUrl, Duration connectionTimeout, Duration readTimeout)
+ throws MalformedURLException {
+ url = new JiraURL(jiraUrl, connectionTimeout, readTimeout);
}
@Test
public void testJiraServerInfoForNonRootJiraUrl() throws Exception {
- setURL("http://jira.mycompany.com/myroot/");
+ setURL("http://jira.mycompany.com/myroot/", CONNECTION_TIMEOUT, READ_TIMEOUT);
restApi = new JiraRestApi(url, USERNAME, PASSWORD, JiraIssue.class, ISSUE_CLASS_PREFIX);
String jiraApiUrl = restApi.getBaseUrl().toString();
assertThat(jiraApiUrl).startsWith(url.toString());
@@ -51,7 +55,7 @@
@Test
public void testJiraServerInfoForNonRootJiraUrlNotEndingWithSlash() throws Exception {
- setURL("http://jira.mycompany.com/myroot");
+ setURL("http://jira.mycompany.com/myroot", CONNECTION_TIMEOUT, READ_TIMEOUT);
restApi = new JiraRestApi(url, USERNAME, PASSWORD, JiraIssue.class, ISSUE_CLASS_PREFIX);
String jiraApiUrl = restApi.getBaseUrl().toString();
assertThat(jiraApiUrl).startsWith(url.toString());
@@ -59,13 +63,24 @@
@Test
public void testJiraServerInfoForRootJiraUrl() throws Exception {
- setURL("http://jira.mycompany.com/myroot");
+ setURL("http://jira.mycompany.com/myroot", CONNECTION_TIMEOUT, READ_TIMEOUT);
restApi = new JiraRestApi(url, USERNAME, PASSWORD, JiraIssue.class, ISSUE_CLASS_PREFIX);
String jiraApiUrl = restApi.getBaseUrl().toString();
assertThat(jiraApiUrl).startsWith(url.toString());
}
@Test
+ public void testJiraServerInfoWithTimeouts() throws Exception {
+ Duration connectionTimeout = Duration.ofSeconds(500L);
+ Duration readTimeout = Duration.ofMillis(10L);
+ setURL("http://jira.mycompany.com/myroot", connectionTimeout, readTimeout);
+ restApi = new JiraRestApi(url, USERNAME, PASSWORD, JiraIssue.class, ISSUE_CLASS_PREFIX);
+ HttpURLConnection httpConnection = restApi.openConnection("/", "GET", false);
+ assertThat(httpConnection.getConnectTimeout()).isEqualTo(connectionTimeout.toMillis());
+ assertThat(httpConnection.getReadTimeout()).isEqualTo(readTimeout.toMillis());
+ }
+
+ @Test
public void testDoPut() throws Exception {
JiraURL url = mock(JiraURL.class);
when(url.resolveUrl(any())).thenReturn(url);
