This is an automated email from the ASF dual-hosted git repository.
cstamas pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/maven-resolver.git
The following commit(s) were added to refs/heads/master by this push:
new 3c073bcc [MRESOLVER-396] Back off on too many requests (#326)
3c073bcc is described below
commit 3c073bccc73ebe4265631eb8e2ca9616c89afbd0
Author: Tamas Cservenak <[email protected]>
AuthorDate: Mon Sep 4 15:17:33 2023 +0200
[MRESOLVER-396] Back off on too many requests (#326)
Provide a way to "back off", and lower the request pace if remote claims
"too many requests". Also, if server sends Retry-After header, obey that.
---
https://issues.apache.org/jira/projects/MRESOLVER/issues/MRESOLVER-396
---
.../eclipse/aether/ConfigurationProperties.java | 54 +++++++++
.../aether/transport/http/HttpTransporter.java | 128 +++++++++++++++++++++
src/site/markdown/configuration.md | 3 +
3 files changed, 185 insertions(+)
diff --git
a/maven-resolver-api/src/main/java/org/eclipse/aether/ConfigurationProperties.java
b/maven-resolver-api/src/main/java/org/eclipse/aether/ConfigurationProperties.java
index fc435d87..a5519514 100644
---
a/maven-resolver-api/src/main/java/org/eclipse/aether/ConfigurationProperties.java
+++
b/maven-resolver-api/src/main/java/org/eclipse/aether/ConfigurationProperties.java
@@ -144,6 +144,60 @@ public final class ConfigurationProperties {
*/
public static final int DEFAULT_HTTP_RETRY_HANDLER_COUNT = 3;
+ /**
+ * The initial retry interval of request to a remote server should be
waited in case of "too many requests"
+ * (HTTP codes 429 and 503). Accepts long as milliseconds. This value is
used if remote server does not use
+ * {@code Retry-After} header, in which case Server value is obeyed.
+ *
+ * @see #DEFAULT_HTTP_RETRY_HANDLER_INTERVAL
+ * @since 1.9.16
+ */
+ public static final String HTTP_RETRY_HANDLER_INTERVAL = PREFIX_CONNECTOR
+ "http.retryHandler.interval";
+
+ /**
+ * The default initial retry interval to use if {@link
#HTTP_RETRY_HANDLER_INTERVAL} isn't set.
+ * Default value 5000ms.
+ *
+ * @since 1.9.16
+ */
+ public static final long DEFAULT_HTTP_RETRY_HANDLER_INTERVAL = 5000L;
+
+ /**
+ * The maximum retry interval of request to a remote server above which
the request should be aborted instead.
+ * In theory, a malicious server could tell Maven "come back after 100
years" that would stall the build for
+ * some. Using this parameter Maven will fail the request instead, if
interval is above this value.
+ *
+ * @see #DEFAULT_HTTP_RETRY_HANDLER_INTERVAL_MAX
+ * @since 1.9.16
+ */
+ public static final String HTTP_RETRY_HANDLER_INTERVAL_MAX =
PREFIX_CONNECTOR + "http.retryHandler.intervalMax";
+
+ /**
+ * The default retry interval maximum to use if {@link
#HTTP_RETRY_HANDLER_INTERVAL_MAX} isn't set.
+ * Default value 5 minutes.
+ *
+ * @since 1.9.16
+ */
+ public static final long DEFAULT_HTTP_RETRY_HANDLER_INTERVAL_MAX =
300_000L;
+
+ /**
+ * The HTTP codes of remote server responses that should be handled as
"too many requests"
+ * (examples: HTTP codes 429 and 503). Accepts comma separated list of
HTTP response codes.
+ *
+ * @see #DEFAULT_HTTP_RETRY_HANDLER_SERVICE_UNAVAILABLE
+ * @since 1.9.16
+ */
+ public static final String HTTP_RETRY_HANDLER_SERVICE_UNAVAILABLE =
+ PREFIX_CONNECTOR + "http.retryHandler.serviceUnavailable";
+
+ /**
+ * The default HTTP codes of remote server responses that should be
handled as "too many requests".
+ * Default value: "429,503".
+ *
+ * @since 1.9.16
+ */
+ public static final String DEFAULT_HTTP_RETRY_HANDLER_SERVICE_UNAVAILABLE
= "429,503";
+
/**
* Should HTTP client use preemptive auth (w/ BASIC) or not?
*
diff --git
a/maven-resolver-transport-http/src/main/java/org/eclipse/aether/transport/http/HttpTransporter.java
b/maven-resolver-transport-http/src/main/java/org/eclipse/aether/transport/http/HttpTransporter.java
index d1848cc9..87020474 100644
---
a/maven-resolver-transport-http/src/main/java/org/eclipse/aether/transport/http/HttpTransporter.java
+++
b/maven-resolver-transport-http/src/main/java/org/eclipse/aether/transport/http/HttpTransporter.java
@@ -34,8 +34,10 @@ import java.nio.file.StandardCopyOption;
import java.nio.file.attribute.FileTime;
import java.util.Collections;
import java.util.Date;
+import java.util.HashSet;
import java.util.List;
import java.util.Map;
+import java.util.Set;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
@@ -44,12 +46,14 @@ import org.apache.http.HttpEntity;
import org.apache.http.HttpEntityEnclosingRequest;
import org.apache.http.HttpHeaders;
import org.apache.http.HttpHost;
+import org.apache.http.HttpResponse;
import org.apache.http.HttpStatus;
import org.apache.http.auth.AuthSchemeProvider;
import org.apache.http.auth.AuthScope;
import org.apache.http.client.CredentialsProvider;
import org.apache.http.client.HttpRequestRetryHandler;
import org.apache.http.client.HttpResponseException;
+import org.apache.http.client.ServiceUnavailableRetryStrategy;
import org.apache.http.client.config.AuthSchemes;
import org.apache.http.client.config.RequestConfig;
import org.apache.http.client.methods.CloseableHttpResponse;
@@ -76,6 +80,7 @@ import org.apache.http.impl.client.CloseableHttpClient;
import org.apache.http.impl.client.DefaultHttpRequestRetryHandler;
import org.apache.http.impl.client.HttpClientBuilder;
import org.apache.http.impl.client.StandardHttpRequestRetryHandler;
+import org.apache.http.protocol.HttpContext;
import org.apache.http.util.EntityUtils;
import org.eclipse.aether.ConfigurationProperties;
import org.eclipse.aether.RepositorySystemSession;
@@ -147,6 +152,7 @@ final class HttpTransporter extends AbstractTransporter {
private final boolean supportWebDav;
+ @SuppressWarnings("checkstyle:methodlength")
HttpTransporter(
Map<String, ChecksumExtractor> checksumExtractors,
RemoteRepository repository,
@@ -230,6 +236,21 @@ final class HttpTransporter extends AbstractTransporter {
ConfigurationProperties.DEFAULT_HTTP_RETRY_HANDLER_COUNT,
ConfigurationProperties.HTTP_RETRY_HANDLER_COUNT + "." +
repository.getId(),
ConfigurationProperties.HTTP_RETRY_HANDLER_COUNT);
+ long retryInterval = ConfigUtils.getLong(
+ session,
+ ConfigurationProperties.DEFAULT_HTTP_RETRY_HANDLER_INTERVAL,
+ ConfigurationProperties.HTTP_RETRY_HANDLER_INTERVAL + "." +
repository.getId(),
+ ConfigurationProperties.HTTP_RETRY_HANDLER_INTERVAL);
+ long retryIntervalMax = ConfigUtils.getLong(
+ session,
+
ConfigurationProperties.DEFAULT_HTTP_RETRY_HANDLER_INTERVAL_MAX,
+ ConfigurationProperties.HTTP_RETRY_HANDLER_INTERVAL_MAX + "."
+ repository.getId(),
+ ConfigurationProperties.HTTP_RETRY_HANDLER_INTERVAL_MAX);
+ String serviceUnavailableCodesString = ConfigUtils.getString(
+ session,
+
ConfigurationProperties.DEFAULT_HTTP_RETRY_HANDLER_SERVICE_UNAVAILABLE,
+ ConfigurationProperties.HTTP_RETRY_HANDLER_SERVICE_UNAVAILABLE
+ "." + repository.getId(),
+
ConfigurationProperties.HTTP_RETRY_HANDLER_SERVICE_UNAVAILABLE);
String retryHandlerName = ConfigUtils.getString(
session,
HTTP_RETRY_HANDLER_NAME_STANDARD,
@@ -269,11 +290,24 @@ final class HttpTransporter extends AbstractTransporter {
throw new IllegalArgumentException(
"Unsupported parameter " + HTTP_RETRY_HANDLER_NAME + "
value: " + retryHandlerName);
}
+ Set<Integer> serviceUnavailableCodes = new HashSet<>();
+ try {
+ for (String code :
ConfigUtils.parseCommaSeparatedUniqueNames(serviceUnavailableCodesString)) {
+ serviceUnavailableCodes.add(Integer.parseInt(code));
+ }
+ } catch (NumberFormatException e) {
+ throw new IllegalArgumentException(
+ "Illegal HTTP codes for " +
ConfigurationProperties.HTTP_RETRY_HANDLER_SERVICE_UNAVAILABLE
+ + " (list of integers): " +
serviceUnavailableCodesString);
+ }
+ ServiceUnavailableRetryStrategy serviceUnavailableRetryStrategy = new
ResolverServiceUnavailableRetryStrategy(
+ retryCount, retryInterval, retryIntervalMax,
serviceUnavailableCodes);
HttpClientBuilder builder = HttpClientBuilder.create()
.setUserAgent(userAgent)
.setDefaultSocketConfig(socketConfig)
.setDefaultRequestConfig(requestConfig)
+
.setServiceUnavailableRetryStrategy(serviceUnavailableRetryStrategy)
.setRetryHandler(retryHandler)
.setDefaultAuthSchemeRegistry(authSchemeRegistry)
.setConnectionManager(state.getConnectionManager())
@@ -704,4 +738,98 @@ final class HttpTransporter extends AbstractTransporter {
}
}
}
+
+ private static class ResolverServiceUnavailableRetryStrategy implements
ServiceUnavailableRetryStrategy {
+ private final int retryCount;
+
+ private final long retryInterval;
+
+ private final long retryIntervalMax;
+
+ private final Set<Integer> serviceUnavailableHttpCodes;
+
+ /**
+ * Ugly, but forced by HttpClient API {@link
ServiceUnavailableRetryStrategy}: the calls for
+ * {@link #retryRequest(HttpResponse, int, HttpContext)} and {@link
#getRetryInterval()} are done by same
+ * thread and are actually done from spot that are very close to each
other (almost subsequent calls).
+ */
+ private static final ThreadLocal<Long> RETRY_INTERVAL_HOLDER = new
ThreadLocal<>();
+
+ private ResolverServiceUnavailableRetryStrategy(
+ int retryCount, long retryInterval, long retryIntervalMax,
Set<Integer> serviceUnavailableHttpCodes) {
+ if (retryCount < 0) {
+ throw new IllegalArgumentException("retryCount must be >= 0");
+ }
+ if (retryInterval < 0L) {
+ throw new IllegalArgumentException("retryInterval must be >=
0");
+ }
+ if (retryIntervalMax < 0L) {
+ throw new IllegalArgumentException("retryIntervalMax must be
>= 0");
+ }
+ this.retryCount = retryCount;
+ this.retryInterval = retryInterval;
+ this.retryIntervalMax = retryIntervalMax;
+ this.serviceUnavailableHttpCodes =
requireNonNull(serviceUnavailableHttpCodes);
+ }
+
+ @Override
+ public boolean retryRequest(HttpResponse response, int executionCount,
HttpContext context) {
+ final boolean retry = executionCount <= retryCount
+ && (serviceUnavailableHttpCodes.contains(
+ response.getStatusLine().getStatusCode()));
+ if (retry) {
+ Long retryInterval = retryInterval(response, executionCount,
context);
+ if (retryInterval != null) {
+ RETRY_INTERVAL_HOLDER.set(retryInterval);
+ return true;
+ }
+ }
+ RETRY_INTERVAL_HOLDER.remove();
+ return false;
+ }
+
+ /**
+ * Calculates retry interval in milliseconds. If {@link
HttpHeaders#RETRY_AFTER} header present, it obeys it.
+ * Otherwise, it returns {@link this#retryInterval} long value
multiplied with {@code executionCount} (starts
+ * from 1 and goes 2, 3,...).
+ *
+ * @return Long representing the retry interval as millis, or {@code
null} if the request should be failed.
+ */
+ private Long retryInterval(HttpResponse httpResponse, int
executionCount, HttpContext httpContext) {
+ Long result = null;
+ Header header =
httpResponse.getFirstHeader(HttpHeaders.RETRY_AFTER);
+ if (header != null && header.getValue() != null) {
+ String headerValue = header.getValue();
+ if (headerValue.contains(":")) { // is date when to retry
+ Date when = DateUtils.parseDate(headerValue); //
presumably future
+ if (when != null) {
+ result = Math.max(when.getTime() -
System.currentTimeMillis(), 0L);
+ }
+ } else {
+ try {
+ result = Long.parseLong(headerValue) * 1000L; // is in
seconds
+ } catch (NumberFormatException e) {
+ // fall through
+ }
+ }
+ }
+ if (result == null) {
+ result = executionCount * this.retryInterval;
+ }
+ if (result > retryIntervalMax) {
+ return null;
+ }
+ return result;
+ }
+
+ @Override
+ public long getRetryInterval() {
+ Long ri = RETRY_INTERVAL_HOLDER.get();
+ if (ri == null) {
+ return 0L;
+ }
+ RETRY_INTERVAL_HOLDER.remove();
+ return ri;
+ }
+ }
}
diff --git a/src/site/markdown/configuration.md
b/src/site/markdown/configuration.md
index 4f66f3f4..fbee0a98 100644
--- a/src/site/markdown/configuration.md
+++ b/src/site/markdown/configuration.md
@@ -43,8 +43,11 @@ Option | Type | Description | Default Value | Supports Repo
ID Suffix
`aether.connector.http.preemptiveAuth` | boolean | Should HTTP client use
preemptive-authentication for all HTTP verbs (works only w/ BASIC). By default
is disabled, as it is considered less secure. | `false` | yes
`aether.connector.http.preemptivePutAuth` | boolean | Should HTTP client use
preemptive-authentication for HTTP PUTs only (works only w/ BASIC). By default
is enabled (same as Wagon). | `true` | yes
`aether.connector.http.retryHandler.count` | int | The maximum number of times
a request to a remote HTTP server should be retried in case of an error. | `3`
| yes
+`aether.connector.http.retryHandler.interval` | long | The initial retry
interval in milliseconds, if server responds with "too many requests". Is
multiplied by 1, 2,.. on each try. If server emits `Retry-After` header, it is
obeyed instead of this value. | `5000` | yes
+`aether.connector.http.retryHandler.intervalMax` | long | The retry interval
maximum in milliseconds, after request should be given up (5 minutes). |
`300000` | yes
`aether.connector.http.retryHandler.name` | String | The name of retryHandler,
supported values are "standard", that obeys RFC-2616, regarding idempotent
methods, and "default" that considers requests w/o payload as idempotent. |
`standard` | yes
`aether.connector.http.retryHandler.requestSentEnabled` | boolean | Set to
`true` if it is acceptable to retry non-idempotent requests, that have been
sent. | `false` | yes
+`aether.connector.http.retryHandler.serviceUnavailable` | String | Comma
separated list of HTTP codes that should be handled as "too many requests". |
`"429,503"` | yes
`aether.connector.http.reuseConnections` | boolean | Should HTTP client reuse
connections (in other words, pool connections) or not? | `true` | yes
`aether.connector.http.supportWebDav` | boolean | If enabled, transport makes
best effort to deploy to WebDAV server. This mode is not recommended, better
use real Maven Repository Manager instead. | `false` | yes
`aether.connector.http.useSystemProperties` | boolean | If enabled, underlying
Apache HttpClient will use system properties as well to configure itself
(typically used to set up HTTP Proxy via Java system properties). See <a
href="https://hc.apache.org/httpcomponents-client-4.5.x/current/httpclient/apidocs/org/apache/http/impl/client/HttpClientBuilder.html";>HttpClientBuilder</a>
for used properties. This mode is **not recommended**, better use documented
ways of configuration instead. | [...]
