From 4508199732b4dc0c8280f94a5f8c6ac51997557f Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Wed, 17 Jan 2024 17:55:27 +0100 Subject: [PATCH 1/3] [MRESOLVER-464] Workaround for JDK-822647 bug Artificially limit the max concurrent request count doable with JDK HttpClient, as otherwise you may end up with IOEx. The value is exposed as config param, so users can tweak it. Value 100 is verified to work with Maven Central. JDK Bug: https://bugs.openjdk.org/browse/JDK-8225647 --- https://issues.apache.org/jira/browse/MRESOLVER-464 --- .../aether/transport/jdk/JdkTransporter.java | 25 +++++++++++++++---- .../jdk/JdkTransporterConfigurationKeys.java | 15 +++++++++++ 2 files changed, 35 insertions(+), 5 deletions(-) diff --git a/maven-resolver-transport-jdk-parent/maven-resolver-transport-jdk-11/src/main/java/org/eclipse/aether/transport/jdk/JdkTransporter.java b/maven-resolver-transport-jdk-parent/maven-resolver-transport-jdk-11/src/main/java/org/eclipse/aether/transport/jdk/JdkTransporter.java index a113162e4..e87e6fbde 100644 --- a/maven-resolver-transport-jdk-parent/maven-resolver-transport-jdk-11/src/main/java/org/eclipse/aether/transport/jdk/JdkTransporter.java +++ b/maven-resolver-transport-jdk-parent/maven-resolver-transport-jdk-11/src/main/java/org/eclipse/aether/transport/jdk/JdkTransporter.java @@ -43,6 +43,7 @@ import java.util.HashMap; import java.util.Locale; import java.util.Map; +import java.util.concurrent.Semaphore; import java.util.function.Supplier; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -64,8 +65,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import static org.eclipse.aether.transport.jdk.JdkTransporterConfigurationKeys.CONFIG_PROP_HTTP_VERSION; -import static org.eclipse.aether.transport.jdk.JdkTransporterConfigurationKeys.DEFAULT_HTTP_VERSION; +import static org.eclipse.aether.transport.jdk.JdkTransporterConfigurationKeys.*; /** * JDK Transport using {@link HttpClient}. @@ -117,6 +117,8 @@ final class JdkTransporter extends AbstractTransporter implements HttpTransporte private final Boolean expectContinue; + private final Semaphore maxConcurrentRequests; + JdkTransporter(RepositorySystemSession session, RemoteRepository repository, int javaVersion) throws NoTransporterException { try { @@ -180,6 +182,9 @@ final class JdkTransporter extends AbstractTransporter implements HttpTransporte } } + this.maxConcurrentRequests = new Semaphore( + ConfigUtils.getInteger(session, DEFAULT_MAX_CONCURRENT_REQUESTS, CONFIG_PROP_MAX_CONCURRENT_REQUESTS)); + this.headers = headers; this.client = getOrCreateClient(session, repository, javaVersion); } @@ -211,7 +216,7 @@ protected void implPeek(PeekTask task) throws Exception { .method("HEAD", HttpRequest.BodyPublishers.noBody()); headers.forEach(request::setHeader); try { - HttpResponse response = client.send(request.build(), HttpResponse.BodyHandlers.discarding()); + HttpResponse response = send(request.build(), HttpResponse.BodyHandlers.discarding()); if (response.statusCode() >= MULTIPLE_CHOICES) { throw new HttpTransporterException(response.statusCode()); } @@ -244,7 +249,7 @@ protected void implGet(GetTask task) throws Exception { } try { - response = client.send(request.build(), HttpResponse.BodyHandlers.ofInputStream()); + response = send(request.build(), HttpResponse.BodyHandlers.ofInputStream()); if (response.statusCode() >= MULTIPLE_CHOICES) { closeBody(response); if (resume && response.statusCode() == PRECONDITION_FAILED) { @@ -352,7 +357,7 @@ protected void implPut(PutTask task) throws Exception { request.method("PUT", HttpRequest.BodyPublishers.ofFile(tempFile.getPath())); try { - HttpResponse response = client.send(request.build(), HttpResponse.BodyHandlers.discarding()); + HttpResponse response = send(request.build(), HttpResponse.BodyHandlers.discarding()); if (response.statusCode() >= MULTIPLE_CHOICES) { throw new HttpTransporterException(response.statusCode()); } @@ -362,6 +367,16 @@ protected void implPut(PutTask task) throws Exception { } } + private HttpResponse send(HttpRequest request, HttpResponse.BodyHandler responseBodyHandler) + throws IOException, InterruptedException { + maxConcurrentRequests.acquire(); + try { + return client.send(request, responseBodyHandler); + } finally { + maxConcurrentRequests.release(); + } + } + @Override protected void implClose() { // no-op diff --git a/maven-resolver-transport-jdk-parent/maven-resolver-transport-jdk-11/src/main/java/org/eclipse/aether/transport/jdk/JdkTransporterConfigurationKeys.java b/maven-resolver-transport-jdk-parent/maven-resolver-transport-jdk-11/src/main/java/org/eclipse/aether/transport/jdk/JdkTransporterConfigurationKeys.java index 647aa093f..92bf1cade 100644 --- a/maven-resolver-transport-jdk-parent/maven-resolver-transport-jdk-11/src/main/java/org/eclipse/aether/transport/jdk/JdkTransporterConfigurationKeys.java +++ b/maven-resolver-transport-jdk-parent/maven-resolver-transport-jdk-11/src/main/java/org/eclipse/aether/transport/jdk/JdkTransporterConfigurationKeys.java @@ -43,4 +43,19 @@ private JdkTransporterConfigurationKeys() {} public static final String CONFIG_PROP_HTTP_VERSION = CONFIG_PROPS_PREFIX + "httpVersion"; public static final String DEFAULT_HTTP_VERSION = "HTTP_2"; + + /** + * The hard limit of maximum concurrent requests JDK transport can do. This is a workaround for the fact, that in + * HTTP/2 mode, JDK HttpClient initializes this value to Integer.MAX_VALUE (!) and lowers it on first response + * from the remote server (but it may be too late). See JDK bug + * JDK-8225647 for details. + * + * @configurationSource {@link RepositorySystemSession#getConfigProperties()} + * @configurationType {@link java.lang.Integer} + * @configurationDefaultValue {@link #DEFAULT_MAX_CONCURRENT_REQUESTS} + * @configurationRepoIdSuffix Yes + */ + public static final String CONFIG_PROP_MAX_CONCURRENT_REQUESTS = CONFIG_PROPS_PREFIX + "maxConcurrentRequests"; + + public static final int DEFAULT_MAX_CONCURRENT_REQUESTS = 100; } From 8efe49060f4e6421e05aa48be1d44ce5a7566a0b Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Wed, 17 Jan 2024 17:58:56 +0100 Subject: [PATCH 2/3] Doco --- src/site/markdown/configuration.md | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/src/site/markdown/configuration.md b/src/site/markdown/configuration.md index f50e797d8..2a4510e17 100644 --- a/src/site/markdown/configuration.md +++ b/src/site/markdown/configuration.md @@ -112,17 +112,18 @@ under the License. | 85. | `"aether.transport.http.userAgent"` | `java.lang.String` | The user agent that repository connectors should report to servers. | `"Aether"` | | No | Session Configuration | | 86. | `"aether.transport.https.securityMode"` | `java.lang.String` | The mode that sets HTTPS transport "security mode": to ignore any SSL errors (certificate validity checks, hostname verification). The default value is . | `"default"` | 1.9.6 | Yes | Session Configuration | | 87. | `"aether.transport.jdk.httpVersion"` | `java.lang.String` | Use string representation of HttpClient version enum "HTTP_2" or "HTTP_1_1" to set default HTTP protocol to use. | `"HTTP_2"` | 2.0.0 | Yes | Session Configuration | -| 88. | `"aether.transport.wagon.config"` | `java.lang.Object` | The configuration to use for the Wagon provider. | - | | Yes | Session Configuration | -| 89. | `"aether.transport.wagon.perms.dirMode"` | `java.lang.String` | Octal numerical notation of permissions to set for newly created directories. Only considered by certain Wagon providers. | - | | Yes | Session Configuration | -| 90. | `"aether.transport.wagon.perms.fileMode"` | `java.lang.String` | Octal numerical notation of permissions to set for newly created files. Only considered by certain Wagon providers. | - | | Yes | Session Configuration | -| 91. | `"aether.transport.wagon.perms.group"` | `java.lang.String` | Group which should own newly created directories/files. Only considered by certain Wagon providers. | - | | Yes | Session Configuration | -| 92. | `"aether.trustedChecksumsSource.sparseDirectory"` | `java.lang.Boolean` | Is checksum source enabled? | `false` | 1.9.0 | No | Session Configuration | -| 93. | `"aether.trustedChecksumsSource.sparseDirectory.basedir"` | `java.lang.String` | The basedir where checksums are. If relative, is resolved from local repository root. | `".checksums"` | 1.9.0 | No | Session Configuration | -| 94. | `"aether.trustedChecksumsSource.sparseDirectory.originAware"` | `java.lang.Boolean` | Is source origin aware? | `true` | 1.9.0 | No | Session Configuration | -| 95. | `"aether.trustedChecksumsSource.summaryFile"` | `java.lang.Boolean` | Is checksum source enabled? | `false` | 1.9.0 | No | Session Configuration | -| 96. | `"aether.trustedChecksumsSource.summaryFile.basedir"` | `java.lang.String` | The basedir where checksums are. If relative, is resolved from local repository root. | `".checksums"` | 1.9.0 | No | Session Configuration | -| 97. | `"aether.trustedChecksumsSource.summaryFile.originAware"` | `java.lang.Boolean` | Is source origin aware? | `true` | 1.9.0 | No | Session Configuration | -| 98. | `"aether.updateCheckManager.sessionState"` | `java.lang.String` | Manages the session state, i.e. influences if the same download requests to artifacts/metadata will happen multiple times within the same RepositorySystemSession. If "enabled" will enable the session state. If "bypass" will enable bypassing (i.e. store all artifact ids/metadata ids which have been updates but not evaluating those). All other values lead to disabling the session state completely. | `"enabled"` | | No | Session Configuration | +| 88. | `"aether.transport.jdk.maxConcurrentRequests"` | `java.lang.Integer` | The hard limit of maximum concurrent requests JDK transport can do. This is a workaround for the fact, that in HTTP/2 mode, JDK HttpClient initializes this value to Integer.MAX_VALUE (!) and lowers it on first response from the remote server (but it may be too late). See JDK bug JDK-8225647 for details. | `100` | 2.0.0 | Yes | Session Configuration | +| 89. | `"aether.transport.wagon.config"` | `java.lang.Object` | The configuration to use for the Wagon provider. | - | | Yes | Session Configuration | +| 90. | `"aether.transport.wagon.perms.dirMode"` | `java.lang.String` | Octal numerical notation of permissions to set for newly created directories. Only considered by certain Wagon providers. | - | | Yes | Session Configuration | +| 91. | `"aether.transport.wagon.perms.fileMode"` | `java.lang.String` | Octal numerical notation of permissions to set for newly created files. Only considered by certain Wagon providers. | - | | Yes | Session Configuration | +| 92. | `"aether.transport.wagon.perms.group"` | `java.lang.String` | Group which should own newly created directories/files. Only considered by certain Wagon providers. | - | | Yes | Session Configuration | +| 93. | `"aether.trustedChecksumsSource.sparseDirectory"` | `java.lang.Boolean` | Is checksum source enabled? | `false` | 1.9.0 | No | Session Configuration | +| 94. | `"aether.trustedChecksumsSource.sparseDirectory.basedir"` | `java.lang.String` | The basedir where checksums are. If relative, is resolved from local repository root. | `".checksums"` | 1.9.0 | No | Session Configuration | +| 95. | `"aether.trustedChecksumsSource.sparseDirectory.originAware"` | `java.lang.Boolean` | Is source origin aware? | `true` | 1.9.0 | No | Session Configuration | +| 96. | `"aether.trustedChecksumsSource.summaryFile"` | `java.lang.Boolean` | Is checksum source enabled? | `false` | 1.9.0 | No | Session Configuration | +| 97. | `"aether.trustedChecksumsSource.summaryFile.basedir"` | `java.lang.String` | The basedir where checksums are. If relative, is resolved from local repository root. | `".checksums"` | 1.9.0 | No | Session Configuration | +| 98. | `"aether.trustedChecksumsSource.summaryFile.originAware"` | `java.lang.Boolean` | Is source origin aware? | `true` | 1.9.0 | No | Session Configuration | +| 99. | `"aether.updateCheckManager.sessionState"` | `java.lang.String` | Manages the session state, i.e. influences if the same download requests to artifacts/metadata will happen multiple times within the same RepositorySystemSession. If "enabled" will enable the session state. If "bypass" will enable bypassing (i.e. store all artifact ids/metadata ids which have been updates but not evaluating those). All other values lead to disabling the session state completely. | `"enabled"` | | No | Session Configuration | All properties which have `yes` in the column `Supports Repo ID Suffix` can be optionally configured specifically for a repository id. In that case the configuration property needs to be suffixed with a period followed by the repository id of the repository to configure, e.g. `aether.connector.http.headers.central` for repository with id `central`. From a96bb61cffcda717ed0dcfa7078bb3c682b2a52d Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Wed, 17 Jan 2024 18:02:25 +0100 Subject: [PATCH 3/3] Make it per repoID configurable --- .../org/eclipse/aether/transport/jdk/JdkTransporter.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/maven-resolver-transport-jdk-parent/maven-resolver-transport-jdk-11/src/main/java/org/eclipse/aether/transport/jdk/JdkTransporter.java b/maven-resolver-transport-jdk-parent/maven-resolver-transport-jdk-11/src/main/java/org/eclipse/aether/transport/jdk/JdkTransporter.java index e87e6fbde..99688950b 100644 --- a/maven-resolver-transport-jdk-parent/maven-resolver-transport-jdk-11/src/main/java/org/eclipse/aether/transport/jdk/JdkTransporter.java +++ b/maven-resolver-transport-jdk-parent/maven-resolver-transport-jdk-11/src/main/java/org/eclipse/aether/transport/jdk/JdkTransporter.java @@ -182,8 +182,11 @@ final class JdkTransporter extends AbstractTransporter implements HttpTransporte } } - this.maxConcurrentRequests = new Semaphore( - ConfigUtils.getInteger(session, DEFAULT_MAX_CONCURRENT_REQUESTS, CONFIG_PROP_MAX_CONCURRENT_REQUESTS)); + this.maxConcurrentRequests = new Semaphore(ConfigUtils.getInteger( + session, + DEFAULT_MAX_CONCURRENT_REQUESTS, + CONFIG_PROP_MAX_CONCURRENT_REQUESTS + "." + repository.getId(), + CONFIG_PROP_MAX_CONCURRENT_REQUESTS)); this.headers = headers; this.client = getOrCreateClient(session, repository, javaVersion);