From 0d440cb5dba70285172972152b33a19139bb0813 Mon Sep 17 00:00:00 2001 From: Violeta Georgieva Date: Tue, 3 May 2022 16:57:09 +0300 Subject: [PATCH 01/42] Add API for providing SNI AsyncMapping (#2172) Fixes #2156 --- .../java/reactor/netty/tcp/SniProvider.java | 101 ++++++++++++------ .../java/reactor/netty/tcp/SslProvider.java | 60 +++++++++-- .../netty/http/server/HttpServerTests.java | 48 +++++++++ .../reactor/netty/tcp/SslProviderTests.java | 42 +++++--- 4 files changed, 195 insertions(+), 56 deletions(-) diff --git a/reactor-netty-core/src/main/java/reactor/netty/tcp/SniProvider.java b/reactor-netty-core/src/main/java/reactor/netty/tcp/SniProvider.java index 8ce994b25d..ca628f25ab 100644 --- a/reactor-netty-core/src/main/java/reactor/netty/tcp/SniProvider.java +++ b/reactor-netty-core/src/main/java/reactor/netty/tcp/SniProvider.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2020-2021 VMware, Inc. or its affiliates, All Rights Reserved. + * Copyright (c) 2020-2022 VMware, Inc. or its affiliates, All Rights Reserved. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -15,14 +15,19 @@ */ package reactor.netty.tcp; -import io.netty.buffer.ByteBufAllocator; import io.netty.channel.Channel; +import io.netty.channel.ChannelHandlerContext; import io.netty.channel.ChannelPipeline; -import io.netty.handler.ssl.SniHandler; -import io.netty.handler.ssl.SslContext; +import io.netty.handler.codec.DecoderException; +import io.netty.handler.ssl.AbstractSniHandler; import io.netty.handler.ssl.SslHandler; +import io.netty.util.AsyncMapping; import io.netty.util.DomainWildcardMappingBuilder; import io.netty.util.Mapping; +import io.netty.util.ReferenceCountUtil; +import io.netty.util.concurrent.Future; +import io.netty.util.concurrent.Promise; +import io.netty.util.internal.PlatformDependent; import reactor.netty.NettyPipeline; import java.util.Map; @@ -52,49 +57,81 @@ void addSniHandler(Channel channel, boolean sslDebug) { SslProvider.addSslReadHandler(pipeline, sslDebug); } - final Map confPerDomainName; - final SslProvider defaultSslProvider; + final AsyncMapping mappings; - SniProvider(Map confPerDomainName, SslProvider defaultSslProvider) { - this.confPerDomainName = confPerDomainName; - this.defaultSslProvider = defaultSslProvider; + SniProvider(AsyncMapping mappings) { + this.mappings = mappings; } - SniHandler newSniHandler() { - DomainWildcardMappingBuilder mappingsContextBuilder = - new DomainWildcardMappingBuilder<>(defaultSslProvider.getSslContext()); - confPerDomainName.forEach((s, sslProvider) -> mappingsContextBuilder.add(s, sslProvider.getSslContext())); + SniProvider(Map confPerDomainName, SslProvider defaultSslProvider) { DomainWildcardMappingBuilder mappingsSslProviderBuilder = new DomainWildcardMappingBuilder<>(defaultSslProvider); confPerDomainName.forEach(mappingsSslProviderBuilder::add); - return new AdvancedSniHandler(mappingsSslProviderBuilder.build(), defaultSslProvider, mappingsContextBuilder.build()); + this.mappings = new AsyncMappingAdapter(mappingsSslProviderBuilder.build()); + } + + SniHandler newSniHandler() { + return new SniHandler(mappings); + } + + static final class AsyncMappingAdapter implements AsyncMapping { + + final Mapping mapping; + + AsyncMappingAdapter(Mapping mapping) { + this.mapping = mapping; + } + + @Override + public Future map(String input, Promise promise) { + try { + return promise.setSuccess(mapping.map(input)); + } + catch (Throwable cause) { + return promise.setFailure(cause); + } + } } - static final class AdvancedSniHandler extends SniHandler { + static final class SniHandler extends AbstractSniHandler { - final Mapping confPerDomainName; - final SslProvider defaultSslProvider; + final AsyncMapping mappings; - AdvancedSniHandler( - Mapping confPerDomainName, - SslProvider defaultSslProvider, - Mapping mappings) { - super(mappings); - this.confPerDomainName = confPerDomainName; - this.defaultSslProvider = defaultSslProvider; + SniHandler(AsyncMapping mappings) { + this.mappings = mappings; } @Override - protected SslHandler newSslHandler(SslContext context, ByteBufAllocator allocator) { - SslHandler sslHandler = super.newSslHandler(context, allocator); - String hostName = hostname(); - if (hostName == null) { - defaultSslProvider.configure(sslHandler); + protected Future lookup(ChannelHandlerContext ctx, String hostname) { + return mappings.map(hostname, ctx.executor().newPromise()); + } + + @Override + protected void onLookupComplete(ChannelHandlerContext ctx, String hostname, Future future) { + if (!future.isSuccess()) { + final Throwable cause = future.cause(); + if (cause instanceof Error) { + throw (Error) cause; + } + throw new DecoderException("failed to get the SslContext for " + hostname, cause); + } + + SslProvider sslProvider = future.getNow(); + SslHandler sslHandler = null; + try { + sslHandler = sslProvider.getSslContext().newHandler(ctx.alloc()); + sslProvider.configure(sslHandler); + ctx.pipeline().replace(this, SslHandler.class.getName(), sslHandler); + sslHandler = null; + } + catch (Throwable cause) { + PlatformDependent.throwException(cause); } - else { - confPerDomainName.map(hostname()).configure(sslHandler); + finally { + if (sslHandler != null) { + ReferenceCountUtil.safeRelease(sslHandler.engine()); + } } - return sslHandler; } } } diff --git a/reactor-netty-core/src/main/java/reactor/netty/tcp/SslProvider.java b/reactor-netty-core/src/main/java/reactor/netty/tcp/SslProvider.java index 5443f418d9..fd4ff1d8aa 100644 --- a/reactor-netty-core/src/main/java/reactor/netty/tcp/SslProvider.java +++ b/reactor-netty-core/src/main/java/reactor/netty/tcp/SslProvider.java @@ -46,6 +46,7 @@ import io.netty.handler.ssl.SslHandler; import io.netty.handler.ssl.SslHandshakeCompletionEvent; import io.netty.handler.ssl.SupportedCipherSuiteFilter; +import io.netty.util.AsyncMapping; import reactor.core.Exceptions; import reactor.netty.NettyPipeline; import reactor.netty.ReactorNetty; @@ -176,7 +177,9 @@ public interface Builder { /** * Adds a mapping for the given domain name to an {@link SslProvider} builder. * If a mapping already exists, it will be overridden. - * Note: This configuration is applicable only when configuring the server. + *

Note: This method is a sync alternative of {@link #setSniAsyncMappings(AsyncMapping)}, + * which removes the async mappings. + *

Note: This configuration is applicable only when configuring the server. * * @param domainName the domain name, it may contain wildcard * @param sslProviderBuilder an {@link SslProvider} builder for building the {@link SslProvider} @@ -187,7 +190,9 @@ public interface Builder { /** * Adds the provided mappings of domain names to {@link SslProvider} builders to the existing mappings. * If a mapping already exists, it will be overridden. - * Note: This configuration is applicable only when configuring the server. + *

Note: This method is a sync alternative of {@link #setSniAsyncMappings(AsyncMapping)}, + * which removes the async mappings. + *

Note: This configuration is applicable only when configuring the server. * * @param confPerDomainName mappings of domain names to {@link SslProvider} builders * @return {@literal this} @@ -197,13 +202,27 @@ public interface Builder { /** * Sets the provided mappings of domain names to {@link SslProvider} builders. * The existing mappings will be removed. - * Note: This configuration is applicable only when configuring the server. + *

Note: This method is a sync alternative of {@link #setSniAsyncMappings(AsyncMapping)}, + * which removes the async mappings. + *

Note: This configuration is applicable only when configuring the server. * * @param confPerDomainName mappings of domain names to {@link SslProvider} builders * @return {@literal this} */ Builder setSniMappings(Map> confPerDomainName); + /** + * Sets the provided mappings of domain names to {@link SslProvider}. + *

Note: This method is an alternative of {@link #addSniMapping(String, Consumer)}, + * {@link #addSniMappings(Map)} and {@link #setSniMappings(Map)}. + *

Note: This configuration is applicable only when configuring the server. + * + * @param mappings mappings of domain names to {@link SslProvider} + * @return {@literal this} + * @since 1.0.19 + */ + Builder setSniAsyncMappings(AsyncMapping mappings); + /** * Sets the desired {@link SNIServerName}s. * Note: This configuration is applicable only when configuring the client. @@ -337,6 +356,8 @@ public interface ProtocolSslContextSpec { final Consumer handlerConfigurator; final int builderHashCode; final SniProvider sniProvider; + final Map confPerDomainName; + final AsyncMapping sniMappings; SslProvider(SslProvider.Build builder) { this.sslContextBuilder = builder.sslCtxBuilder; @@ -386,14 +407,19 @@ else if (builder.protocolSslContextSpec != null) { this.closeNotifyFlushTimeoutMillis = builder.closeNotifyFlushTimeoutMillis; this.closeNotifyReadTimeoutMillis = builder.closeNotifyReadTimeoutMillis; this.builderHashCode = builder.hashCode(); - if (!builder.confPerDomainName.isEmpty()) { + this.confPerDomainName = builder.confPerDomainName; + this.sniMappings = builder.sniMappings; + if (!confPerDomainName.isEmpty()) { if (this.type != null) { - this.sniProvider = updateAllSslProviderConfiguration(builder.confPerDomainName, this, type); + this.sniProvider = updateAllSslProviderConfiguration(confPerDomainName, this, type); } else { - this.sniProvider = new SniProvider(builder.confPerDomainName, this); + this.sniProvider = new SniProvider(confPerDomainName, this); } } + else if (sniMappings != null) { + this.sniProvider = new SniProvider(sniMappings); + } else { this.sniProvider = null; } @@ -416,6 +442,8 @@ else if (builder.protocolSslContextSpec != null) { this.closeNotifyFlushTimeoutMillis = from.closeNotifyFlushTimeoutMillis; this.closeNotifyReadTimeoutMillis = from.closeNotifyReadTimeoutMillis; this.builderHashCode = from.builderHashCode; + this.confPerDomainName = from.confPerDomainName; + this.sniMappings = from.sniMappings; this.sniProvider = from.sniProvider; } @@ -439,8 +467,15 @@ else if (builder.protocolSslContextSpec != null) { this.closeNotifyFlushTimeoutMillis = from.closeNotifyFlushTimeoutMillis; this.closeNotifyReadTimeoutMillis = from.closeNotifyReadTimeoutMillis; this.builderHashCode = from.builderHashCode; + this.confPerDomainName = from.confPerDomainName; + this.sniMappings = from.sniMappings; if (from.sniProvider != null) { - this.sniProvider = updateAllSslProviderConfiguration(from.sniProvider.confPerDomainName, this, type); + if (!confPerDomainName.isEmpty()) { + this.sniProvider = updateAllSslProviderConfiguration(confPerDomainName, this, type); + } + else { + this.sniProvider = new SniProvider(sniMappings); + } } else { this.sniProvider = null; @@ -613,6 +648,7 @@ static final class Build implements SslContextSpec, DefaultConfigurationSpec, Bu long closeNotifyReadTimeoutMillis; List serverNames; final Map confPerDomainName = new HashMap<>(); + AsyncMapping sniMappings; // SslContextSpec @@ -704,6 +740,7 @@ public final Builder closeNotifyReadTimeoutMillis(long closeNotifyReadTimeoutMil @Override public Builder addSniMapping(String domainName, Consumer sslProviderBuilder) { addInternal(domainName, sslProviderBuilder); + this.sniMappings = null; return this; } @@ -711,6 +748,7 @@ public Builder addSniMapping(String domainName, Consumer public Builder addSniMappings(Map> confPerDomainName) { Objects.requireNonNull(confPerDomainName); confPerDomainName.forEach(this::addInternal); + this.sniMappings = null; return this; } @@ -719,6 +757,14 @@ public Builder setSniMappings(Map> conf Objects.requireNonNull(confPerDomainName); this.confPerDomainName.clear(); confPerDomainName.forEach(this::addInternal); + this.sniMappings = null; + return this; + } + + @Override + public Builder setSniAsyncMappings(AsyncMapping mappings) { + this.sniMappings = Objects.requireNonNull(mappings); + this.confPerDomainName.clear(); return this; } diff --git a/reactor-netty-http/src/test/java/reactor/netty/http/server/HttpServerTests.java b/reactor-netty-http/src/test/java/reactor/netty/http/server/HttpServerTests.java index a2d7ca798a..440cef2488 100644 --- a/reactor-netty-http/src/test/java/reactor/netty/http/server/HttpServerTests.java +++ b/reactor-netty-http/src/test/java/reactor/netty/http/server/HttpServerTests.java @@ -117,6 +117,7 @@ import reactor.netty.http.client.PrematureCloseException; import reactor.netty.resources.ConnectionProvider; import reactor.netty.resources.LoopResources; +import reactor.netty.tcp.SslProvider; import reactor.netty.tcp.TcpClient; import reactor.netty.tcp.TcpServer; import reactor.netty.transport.TransportConfig; @@ -1976,6 +1977,53 @@ public void userEventTriggered(ChannelHandlerContext ctx, Object evt) { assertThat(hostname.get()).isEqualTo("test.com"); } + @Test + void testSniSupportAsyncMappings() throws Exception { + SelfSignedCertificate defaultCert = new SelfSignedCertificate("default"); + Http11SslContextSpec defaultSslContextBuilder = + Http11SslContextSpec.forServer(defaultCert.certificate(), defaultCert.privateKey()); + + SelfSignedCertificate testCert = new SelfSignedCertificate("test.com"); + Http11SslContextSpec testSslContextBuilder = + Http11SslContextSpec.forServer(testCert.certificate(), testCert.privateKey()); + SslProvider testSslProvider = SslProvider.builder().sslContext(testSslContextBuilder).build(); + + Http11SslContextSpec clientSslContextBuilder = + Http11SslContextSpec.forClient() + .configure(builder -> builder.trustManager(InsecureTrustManagerFactory.INSTANCE)); + + AtomicReference hostname = new AtomicReference<>(); + disposableServer = + createServer() + .secure(spec -> spec.sslContext(defaultSslContextBuilder) + .setSniAsyncMappings((input, promise) -> promise.setSuccess(testSslProvider))) + .doOnChannelInit((obs, channel, remoteAddress) -> + channel.pipeline() + .addAfter(NettyPipeline.SslHandler, "test", new ChannelInboundHandlerAdapter() { + @Override + public void userEventTriggered(ChannelHandlerContext ctx, Object evt) { + if (evt instanceof SniCompletionEvent) { + hostname.set(((SniCompletionEvent) evt).hostname()); + } + ctx.fireUserEventTriggered(evt); + } + })) + .handle((req, res) -> res.sendString(Mono.just("testSniSupport"))) + .bindNow(); + + createClient(disposableServer::address) + .secure(spec -> spec.sslContext(clientSslContextBuilder) + .serverNames(new SNIHostName("test.com"))) + .get() + .uri("/") + .responseContent() + .aggregate() + .block(Duration.ofSeconds(30)); + + assertThat(hostname.get()).isNotNull(); + assertThat(hostname.get()).isEqualTo("test.com"); + } + @Test void testIssue1286_HTTP11() throws Exception { doTestIssue1286(Function.identity(), Function.identity(), false, false); diff --git a/reactor-netty-http/src/test/java/reactor/netty/tcp/SslProviderTests.java b/reactor-netty-http/src/test/java/reactor/netty/tcp/SslProviderTests.java index d5d95c8346..7ed7afb16e 100644 --- a/reactor-netty-http/src/test/java/reactor/netty/tcp/SslProviderTests.java +++ b/reactor-netty-http/src/test/java/reactor/netty/tcp/SslProviderTests.java @@ -30,8 +30,7 @@ import io.netty.handler.ssl.SslHandler; import io.netty.handler.ssl.util.InsecureTrustManagerFactory; import io.netty.handler.ssl.util.SelfSignedCertificate; -import io.netty.util.DomainWildcardMappingBuilder; -import io.netty.util.Mapping; +import io.netty.util.concurrent.GlobalEventExecutor; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -245,12 +244,14 @@ void testAdd() { .addSniMapping("localhost", spec -> spec.sslContext(localhostSslContext)); SniProvider provider = builder.build().sniProvider; - assertThat(mappings(provider).map("localhost")).isSameAs(localhostSslContext); + assertThat(provider.mappings.map("localhost", GlobalEventExecutor.INSTANCE.newPromise()).getNow().sslContext) + .isSameAs(localhostSslContext); provider = builder.addSniMapping("localhost", spec -> spec.sslContext(anotherSslContext)) .build() .sniProvider; - assertThat(mappings(provider).map("localhost")).isSameAs(anotherSslContext); + assertThat(provider.mappings.map("localhost", GlobalEventExecutor.INSTANCE.newPromise()).getNow().sslContext) + .isSameAs(anotherSslContext); } @Test @@ -277,13 +278,16 @@ void testAddAll() { .addSniMappings(map); SniProvider provider = builder.build().sniProvider; - assertThat(mappings(provider).map("localhost")).isSameAs(localhostSslContext); + assertThat(provider.mappings.map("localhost", GlobalEventExecutor.INSTANCE.newPromise()).getNow().sslContext) + .isSameAs(localhostSslContext); map.put("another", spec -> spec.sslContext(anotherSslContext)); provider = builder.addSniMappings(map).build().sniProvider; - assertThat(mappings(provider).map("localhost")).isSameAs(localhostSslContext); - assertThat(mappings(provider).map("another")).isSameAs(anotherSslContext); + assertThat(provider.mappings.map("localhost", GlobalEventExecutor.INSTANCE.newPromise()).getNow().sslContext) + .isSameAs(localhostSslContext); + assertThat(provider.mappings.map("another", GlobalEventExecutor.INSTANCE.newPromise()).getNow().sslContext) + .isSameAs(anotherSslContext); } @Test @@ -306,14 +310,17 @@ void testSetAll() throws Exception { .setSniMappings(map); SniProvider provider = builder.build().sniProvider; - assertThat(mappings(provider).map("localhost")).isSameAs(localhostSslContext); + assertThat(provider.mappings.map("localhost", GlobalEventExecutor.INSTANCE.newPromise()).getNow().sslContext) + .isSameAs(localhostSslContext); map.clear(); map.put("another", spec -> spec.sslContext(anotherSslContext)); provider = builder.setSniMappings(map).build().sniProvider; - assertThat(mappings(provider).map("localhost")).isSameAs(defaultSslContext); - assertThat(mappings(provider).map("another")).isSameAs(anotherSslContext); + assertThat(provider.mappings.map("localhost", GlobalEventExecutor.INSTANCE.newPromise()).getNow().sslContext) + .isSameAs(defaultSslContext); + assertThat(provider.mappings.map("another", GlobalEventExecutor.INSTANCE.newPromise()).getNow().sslContext) + .isSameAs(anotherSslContext); } @Test @@ -324,6 +331,14 @@ void testSetAllBadValues() { .setSniMappings(null)); } + @Test + void testSetSniAsyncMappingsBadValues() { + assertThatExceptionOfType(NullPointerException.class) + .isThrownBy(() -> SslProvider.builder() + .sslContext(serverSslContextBuilder) + .setSniAsyncMappings(null)); + } + @Test void testServerNames() throws Exception { SslContext defaultSslContext = clientSslContextBuilder.sslContext(); @@ -351,11 +366,4 @@ void testServerNamesBadValues() throws Exception { .sslContext(defaultSslContext) .serverNames((SNIServerName[]) null)); } - - static Mapping mappings(SniProvider provider) { - DomainWildcardMappingBuilder mappingsBuilder = - new DomainWildcardMappingBuilder<>(provider.defaultSslProvider.getSslContext()); - provider.confPerDomainName.forEach((s, sslProvider) -> mappingsBuilder.add(s, sslProvider.getSslContext())); - return mappingsBuilder.build(); - } } From 752df461d7be0d01f8235fb035c5082b587ce09c Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 4 May 2022 12:42:50 +0300 Subject: [PATCH 02/42] Bump com.diffplug.spotless from 6.5.1 to 6.5.2 (#2176) Bumps com.diffplug.spotless from 6.5.1 to 6.5.2. --- updated-dependencies: - dependency-name: com.diffplug.spotless dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.gradle b/build.gradle index 800206df6e..95327e554b 100644 --- a/build.gradle +++ b/build.gradle @@ -28,7 +28,7 @@ buildscript { } plugins { - id "com.diffplug.spotless" version "6.5.1" + id "com.diffplug.spotless" version "6.5.2" id 'org.asciidoctor.jvm.convert' version '3.3.2' apply false id 'org.asciidoctor.jvm.pdf' version '3.3.2' apply false id 'com.google.osdetector' version '1.7.0' From 9bdc543fcffb512a7eb21ac7fa85af6553f4566a Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Thu, 5 May 2022 08:53:50 +0300 Subject: [PATCH 03/42] Bump de.undercouch.download from 5.0.5 to 5.1.0 (#2177) Bumps de.undercouch.download from 5.0.5 to 5.1.0. --- updated-dependencies: - dependency-name: de.undercouch.download dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.gradle b/build.gradle index 95327e554b..11bb9c5ee8 100644 --- a/build.gradle +++ b/build.gradle @@ -37,7 +37,7 @@ plugins { id 'com.github.johnrengelman.shadow' version '7.1.2' apply false //we now need version of Artifactory gradle plugin deployed on Maven Central, see above id 'me.champeau.gradle.japicmp' version '0.4.0' apply false - id 'de.undercouch.download' version '5.0.5' apply false + id 'de.undercouch.download' version '5.1.0' apply false id 'io.spring.javadoc' version '0.0.1' apply false id 'io.spring.javadoc-aggregate' version '0.0.1' apply false id 'biz.aQute.bnd.builder' version '6.2.0' apply false From 14fa39e77204d2aa53b41645326c3c9f8b57721e Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 9 May 2022 08:45:11 +0300 Subject: [PATCH 04/42] Bump braveVersion from 5.13.8 to 5.13.9 (#2179) Bumps `braveVersion` from 5.13.8 to 5.13.9. Updates `brave-instrumentation-http` from 5.13.8 to 5.13.9 Updates `brave-instrumentation-http-tests` from 5.13.8 to 5.13.9 --- updated-dependencies: - dependency-name: io.zipkin.brave:brave-instrumentation-http dependency-type: direct:production update-type: version-update:semver-patch - dependency-name: io.zipkin.brave:brave-instrumentation-http-tests dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.gradle b/build.gradle index 11bb9c5ee8..9b9fdaf70e 100644 --- a/build.gradle +++ b/build.gradle @@ -86,7 +86,7 @@ ext { //Metrics micrometerVersion = '1.5.0' //optional baseline: technically could work with 1.2.x, should work with any 1.3.x - braveVersion = '5.13.8' + braveVersion = '5.13.9' jsr305Version = '3.0.2' From 135fbcea63a54a9211724686721fb5aea3178eb5 Mon Sep 17 00:00:00 2001 From: Violeta Georgieva Date: Mon, 9 May 2022 10:55:38 +0300 Subject: [PATCH 05/42] Update to netty-tcnative-boringssl-static v2.0.52.Final (#2180) --- build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.gradle b/build.gradle index 9b9fdaf70e..566d3e5640 100644 --- a/build.gradle +++ b/build.gradle @@ -113,7 +113,7 @@ ext { awaitilityVersion = '4.2.0' hoverflyJavaVersion = '0.14.1' tomcatVersion = '9.0.62' - boringSslVersion = '2.0.51.Final' + boringSslVersion = '2.0.52.Final' junitVersion = '5.8.2' junitPlatformLauncherVersion = '1.8.2' mockitoVersion = '4.5.1' From 98033a14bb2c1b11ca0143f76fb11cd1d58a8eb2 Mon Sep 17 00:00:00 2001 From: Violeta Georgieva Date: Mon, 9 May 2022 17:13:48 +0300 Subject: [PATCH 06/42] Update to Netty v4.1.77.Final (#2181) --- .github/workflows/check_netty_snapshots.yml | 2 +- build.gradle | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/check_netty_snapshots.yml b/.github/workflows/check_netty_snapshots.yml index efbfc9ce3b..375be09491 100644 --- a/.github/workflows/check_netty_snapshots.yml +++ b/.github/workflows/check_netty_snapshots.yml @@ -29,4 +29,4 @@ jobs: distribution: 'temurin' java-version: '8' - name: Build with Gradle - run: ./gradlew clean check --no-daemon -PforceTransport=${{ matrix.transport }} -PforceNettyVersion='4.1.77.Final-SNAPSHOT' + run: ./gradlew clean check --no-daemon -PforceTransport=${{ matrix.transport }} -PforceNettyVersion='4.1.78.Final-SNAPSHOT' diff --git a/build.gradle b/build.gradle index 566d3e5640..c2a4d82ce3 100644 --- a/build.gradle +++ b/build.gradle @@ -95,7 +95,7 @@ ext { logbackVersion = '1.2.11' // Netty - nettyDefaultVersion = '4.1.76.Final' + nettyDefaultVersion = '4.1.77.Final' if (!project.hasProperty("forceNettyVersion")) { nettyVersion = nettyDefaultVersion } From 41db0104817c910294ad1cba2ed07049c4501f38 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Basl=C3=A9?= Date: Tue, 10 May 2022 10:52:50 +0200 Subject: [PATCH 07/42] [release] Prepare and release 1.0.19 --- README.md | 8 ++++---- gradle.properties | 12 ++++++------ 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/README.md b/README.md index c94e4a5053..8c03b32ab1 100644 --- a/README.md +++ b/README.md @@ -22,10 +22,10 @@ With `Gradle` from [repo.spring.io](https://repo.spring.io) or `Maven Central` r } dependencies { - //compile "io.projectreactor.netty:reactor-netty-core:1.0.19-SNAPSHOT" - compile "io.projectreactor.netty:reactor-netty-core:1.0.18" - //compile "io.projectreactor.netty:reactor-netty-http:1.0.19-SNAPSHOT" - compile "io.projectreactor.netty:reactor-netty-http:1.0.18" + //compile "io.projectreactor.netty:reactor-netty-core:1.0.20-SNAPSHOT" + compile "io.projectreactor.netty:reactor-netty-core:1.0.19" + //compile "io.projectreactor.netty:reactor-netty-http:1.0.20-SNAPSHOT" + compile "io.projectreactor.netty:reactor-netty-http:1.0.19" } ``` diff --git a/gradle.properties b/gradle.properties index c471ef0511..c437a701de 100644 --- a/gradle.properties +++ b/gradle.properties @@ -1,7 +1,7 @@ -reactorPoolVersion=0.2.9-SNAPSHOT -version=1.0.19-SNAPSHOT -reactorNettyQuicVersion=0.0.8-SNAPSHOT -reactorCoreVersion=3.4.18-SNAPSHOT -reactorAddonsVersion=3.4.9-SNAPSHOT +reactorPoolVersion=0.2.8 +version=1.0.19 +reactorNettyQuicVersion=0.0.8 +reactorCoreVersion=3.4.18 +reactorAddonsVersion=3.4.8 compatibleVersion=1.0.18 -bomVersion=2020.0.18 \ No newline at end of file +bomVersion=2020.0.19 \ No newline at end of file From 4af48e9f34abb930e39a27194ba194e59a7d393e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Basl=C3=A9?= Date: Tue, 10 May 2022 11:17:40 +0200 Subject: [PATCH 08/42] [release] Back to snapshots, next is 1.0.20-SNAPSHOT --- gradle.properties | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/gradle.properties b/gradle.properties index c437a701de..ebc98461e4 100644 --- a/gradle.properties +++ b/gradle.properties @@ -1,7 +1,7 @@ -reactorPoolVersion=0.2.8 -version=1.0.19 -reactorNettyQuicVersion=0.0.8 -reactorCoreVersion=3.4.18 -reactorAddonsVersion=3.4.8 -compatibleVersion=1.0.18 +reactorPoolVersion=0.2.9-SNAPSHOT +version=1.0.20-SNAPSHOT +reactorNettyQuicVersion=0.0.9-SNAPSHOT +reactorCoreVersion=3.4.19-SNAPSHOT +reactorAddonsVersion=3.4.9-SNAPSHOT +compatibleVersion=1.0.19 bomVersion=2020.0.19 \ No newline at end of file From 7d4821b6584c54829e80801607debd0dd1cdb2c1 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 11 May 2022 08:21:10 +0300 Subject: [PATCH 09/42] Bump com.diffplug.spotless from 6.5.2 to 6.6.0 (#2185) Bumps com.diffplug.spotless from 6.5.2 to 6.6.0. --- updated-dependencies: - dependency-name: com.diffplug.spotless dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.gradle b/build.gradle index c2a4d82ce3..c96fff1f04 100644 --- a/build.gradle +++ b/build.gradle @@ -28,7 +28,7 @@ buildscript { } plugins { - id "com.diffplug.spotless" version "6.5.2" + id "com.diffplug.spotless" version "6.6.0" id 'org.asciidoctor.jvm.convert' version '3.3.2' apply false id 'org.asciidoctor.jvm.pdf' version '3.3.2' apply false id 'com.google.osdetector' version '1.7.0' From a92cb8ff64fdd5d5d06b4ce1632825a33c7d0c79 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 11 May 2022 08:25:51 +0300 Subject: [PATCH 10/42] Bump netty-incubator-transport-native-io_uring (#2186) Bumps [netty-incubator-transport-native-io_uring](https://github.com/netty/netty-incubator-transport-io_uring) from 0.0.13.Final to 0.0.14.Final. - [Release notes](https://github.com/netty/netty-incubator-transport-io_uring/releases) - [Commits](https://github.com/netty/netty-incubator-transport-io_uring/compare/netty-incubator-transport-parent-io_uring-0.0.13.Final...netty-incubator-transport-parent-io_uring-0.0.14.Final) --- updated-dependencies: - dependency-name: io.netty.incubator:netty-incubator-transport-native-io_uring dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.gradle b/build.gradle index c96fff1f04..9ab483dddc 100644 --- a/build.gradle +++ b/build.gradle @@ -103,7 +103,7 @@ ext { nettyVersion = forceNettyVersion println "Netty version defined from command line: ${forceNettyVersion}" } - nettyIoUringVersion = '0.0.13.Final' + nettyIoUringVersion = '0.0.14.Final' nettyQuicVersion = '0.0.26.Final' // Testing From 05efec768d7acc7e07286502bc6cd41634a9e19e Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Thu, 12 May 2022 08:18:12 +0300 Subject: [PATCH 11/42] Bump org.gradle.test-retry from 1.3.2 to 1.4.0 (#2188) Bumps org.gradle.test-retry from 1.3.2 to 1.4.0. --- updated-dependencies: - dependency-name: org.gradle.test-retry dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.gradle b/build.gradle index 9ab483dddc..d97e45f819 100644 --- a/build.gradle +++ b/build.gradle @@ -32,7 +32,7 @@ plugins { id 'org.asciidoctor.jvm.convert' version '3.3.2' apply false id 'org.asciidoctor.jvm.pdf' version '3.3.2' apply false id 'com.google.osdetector' version '1.7.0' - id 'org.gradle.test-retry' version '1.3.2' + id 'org.gradle.test-retry' version '1.4.0' id 'io.spring.nohttp' version '0.0.10' id 'com.github.johnrengelman.shadow' version '7.1.2' apply false //we now need version of Artifactory gradle plugin deployed on Maven Central, see above From eda26d2baf24eb4f971bb17622dd68fc85f146a2 Mon Sep 17 00:00:00 2001 From: Violeta Georgieva Date: Thu, 12 May 2022 09:11:34 +0300 Subject: [PATCH 12/42] Update Netty QUIC Codec to v0.0.27.Final (#2189) --- build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.gradle b/build.gradle index d97e45f819..ac396f29a1 100644 --- a/build.gradle +++ b/build.gradle @@ -104,7 +104,7 @@ ext { println "Netty version defined from command line: ${forceNettyVersion}" } nettyIoUringVersion = '0.0.14.Final' - nettyQuicVersion = '0.0.26.Final' + nettyQuicVersion = '0.0.27.Final' // Testing jacksonDatabindVersion = '2.13.2.2' From 0e722680c9b68c7a38c2f2d595b5ead8f7c6c4fa Mon Sep 17 00:00:00 2001 From: Violeta Georgieva Date: Thu, 12 May 2022 12:17:53 +0300 Subject: [PATCH 13/42] Update reactor-netty-incubator-quic README.md --- reactor-netty-incubator-quic/README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/reactor-netty-incubator-quic/README.md b/reactor-netty-incubator-quic/README.md index 481e3e5825..c5bf366ae2 100644 --- a/reactor-netty-incubator-quic/README.md +++ b/reactor-netty-incubator-quic/README.md @@ -13,8 +13,8 @@ With `Gradle` from [repo.spring.io](https://repo.spring.io) or `Maven Central` r } dependencies { - //compile "io.projectreactor.netty.incubator:reactor-netty-incubator-quic:0.0.8-SNAPSHOT" - compile "io.projectreactor.netty.incubator:reactor-netty-incubator-quic:0.0.7" + //compile "io.projectreactor.netty.incubator:reactor-netty-incubator-quic:0.0.9-SNAPSHOT" + compile "io.projectreactor.netty.incubator:reactor-netty-incubator-quic:0.0.8" } ``` From 8444372ff75c6c457f411e33cd134d37b9000ddb Mon Sep 17 00:00:00 2001 From: Alexej Timonin Date: Thu, 12 May 2022 13:34:22 +0300 Subject: [PATCH 14/42] Add support for 303 redirect (#2184) --- .../java/reactor/netty/http/client/HttpClient.java | 2 +- .../reactor/netty/http/client/HttpClientConfig.java | 2 +- .../netty/http/client/HttpClientConnect.java | 6 +++++- .../netty/http/client/HttpClientOperations.java | 2 +- .../netty/http/client/RedirectClientException.java | 7 +++++-- .../netty/http/client/HttpClientOperationsTest.java | 5 +++-- .../reactor/netty/http/client/HttpRedirectTest.java | 13 +++++++++++++ 7 files changed, 29 insertions(+), 8 deletions(-) diff --git a/reactor-netty-http/src/main/java/reactor/netty/http/client/HttpClient.java b/reactor-netty-http/src/main/java/reactor/netty/http/client/HttpClient.java index 6631881d4c..0cb2da1a3a 100644 --- a/reactor-netty-http/src/main/java/reactor/netty/http/client/HttpClient.java +++ b/reactor-netty-http/src/main/java/reactor/netty/http/client/HttpClient.java @@ -905,7 +905,7 @@ public final HttpClient followRedirect(BiPredicateNote: The sensitive headers {@link #followRedirect(boolean, Consumer) followRedirect} * are removed from the initialized request when redirecting to a different domain, they can be re-added diff --git a/reactor-netty-http/src/main/java/reactor/netty/http/client/HttpClientConfig.java b/reactor-netty-http/src/main/java/reactor/netty/http/client/HttpClientConfig.java index 0b08f12cbd..3eeceabf3d 100644 --- a/reactor-netty-http/src/main/java/reactor/netty/http/client/HttpClientConfig.java +++ b/reactor-netty-http/src/main/java/reactor/netty/http/client/HttpClientConfig.java @@ -622,7 +622,7 @@ static Future openStream( return bootstrap.open(); } - static final Pattern FOLLOW_REDIRECT_CODES = Pattern.compile("30[1278]"); + static final Pattern FOLLOW_REDIRECT_CODES = Pattern.compile("30[12378]"); static final BiPredicate FOLLOW_REDIRECT_PREDICATE = (req, res) -> FOLLOW_REDIRECT_CODES.matcher(res.status() diff --git a/reactor-netty-http/src/main/java/reactor/netty/http/client/HttpClientConnect.java b/reactor-netty-http/src/main/java/reactor/netty/http/client/HttpClientConnect.java index f3fab1a9bb..615374ed6a 100644 --- a/reactor-netty-http/src/main/java/reactor/netty/http/client/HttpClientConnect.java +++ b/reactor-netty-http/src/main/java/reactor/netty/http/client/HttpClientConnect.java @@ -37,6 +37,7 @@ import io.netty.handler.codec.http.HttpHeaderNames; import io.netty.handler.codec.http.HttpHeaders; import io.netty.handler.codec.http.HttpMethod; +import io.netty.handler.codec.http.HttpResponseStatus; import io.netty.handler.codec.http.HttpUtil; import io.netty.handler.codec.http.HttpVersion; import io.netty.handler.ssl.SslClosedEngineException; @@ -446,7 +447,7 @@ public void onStateChange(Connection connection, State newState) { static final class HttpClientHandler extends SocketAddress implements Predicate, Supplier { - final HttpMethod method; + volatile HttpMethod method; final HttpHeaders defaultHeaders; final BiFunction> handler; @@ -675,6 +676,9 @@ void channel(HttpClientOperations ops) { public boolean test(Throwable throwable) { if (throwable instanceof RedirectClientException) { RedirectClientException re = (RedirectClientException) throwable; + if (HttpResponseStatus.SEE_OTHER.equals(re.status)) { + method = HttpMethod.GET; + } redirect(re.location); return true; } diff --git a/reactor-netty-http/src/main/java/reactor/netty/http/client/HttpClientOperations.java b/reactor-netty-http/src/main/java/reactor/netty/http/client/HttpClientOperations.java index 513e04d231..1edd53a500 100644 --- a/reactor-netty-http/src/main/java/reactor/netty/http/client/HttpClientOperations.java +++ b/reactor-netty-http/src/main/java/reactor/netty/http/client/HttpClientOperations.java @@ -738,7 +738,7 @@ final boolean notRedirected(HttpResponse response) { .entries() .toString()); } - redirecting = new RedirectClientException(response.headers()); + redirecting = new RedirectClientException(response.headers(), response.status()); return false; } return true; diff --git a/reactor-netty-http/src/main/java/reactor/netty/http/client/RedirectClientException.java b/reactor-netty-http/src/main/java/reactor/netty/http/client/RedirectClientException.java index 5bd7001e2e..21607cca64 100644 --- a/reactor-netty-http/src/main/java/reactor/netty/http/client/RedirectClientException.java +++ b/reactor-netty-http/src/main/java/reactor/netty/http/client/RedirectClientException.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2011-2021 VMware, Inc. or its affiliates, All Rights Reserved. + * Copyright (c) 2011-2022 VMware, Inc. or its affiliates, All Rights Reserved. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -19,6 +19,7 @@ import io.netty.handler.codec.http.HttpHeaderNames; import io.netty.handler.codec.http.HttpHeaders; +import io.netty.handler.codec.http.HttpResponseStatus; /** * An error for signalling that an error occurred during a communication over HTTP version @@ -27,9 +28,11 @@ final class RedirectClientException extends RuntimeException { final String location; + final HttpResponseStatus status; - RedirectClientException(HttpHeaders headers) { + RedirectClientException(HttpHeaders headers, HttpResponseStatus status) { location = Objects.requireNonNull(headers.get(HttpHeaderNames.LOCATION)); + this.status = Objects.requireNonNull(status); } @Override diff --git a/reactor-netty-http/src/test/java/reactor/netty/http/client/HttpClientOperationsTest.java b/reactor-netty-http/src/test/java/reactor/netty/http/client/HttpClientOperationsTest.java index 25eca24cc9..c126a4601d 100644 --- a/reactor-netty-http/src/test/java/reactor/netty/http/client/HttpClientOperationsTest.java +++ b/reactor-netty-http/src/test/java/reactor/netty/http/client/HttpClientOperationsTest.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2017-2021 VMware, Inc. or its affiliates, All Rights Reserved. + * Copyright (c) 2017-2022 VMware, Inc. or its affiliates, All Rights Reserved. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -145,7 +145,8 @@ void testConstructorWithProvidedReplacement() { ops1.followRedirectPredicate((req, res) -> true); ops1.started = true; ops1.retrying = true; - ops1.redirecting = new RedirectClientException(new DefaultHttpHeaders().add(HttpHeaderNames.LOCATION, "/")); + ops1.redirecting = new RedirectClientException(new DefaultHttpHeaders().add(HttpHeaderNames.LOCATION, "/"), + HttpResponseStatus.MOVED_PERMANENTLY); HttpClientOperations ops2 = new HttpClientOperations(ops1); diff --git a/reactor-netty-http/src/test/java/reactor/netty/http/client/HttpRedirectTest.java b/reactor-netty-http/src/test/java/reactor/netty/http/client/HttpRedirectTest.java index c34afa4327..5feed17da5 100644 --- a/reactor-netty-http/src/test/java/reactor/netty/http/client/HttpRedirectTest.java +++ b/reactor-netty-http/src/test/java/reactor/netty/http/client/HttpRedirectTest.java @@ -339,6 +339,9 @@ void testIssue522() { .get("/302", (req, res) -> res.status(302) .header(HttpHeaderNames.LOCATION, "http://localhost:" + serverPort + "/redirect")) + .post("/303", (req, res) -> + res.status(303) + .header(HttpHeaderNames.LOCATION, "http://localhost:" + serverPort + "/redirect")) .get("/304", (req, res) -> res.status(304)) .get("/307", (req, res) -> res.status(307) @@ -375,6 +378,16 @@ void testIssue522() { .expectComplete() .verify(Duration.ofSeconds(30)); + StepVerifier.create(client.followRedirect(true) + .post() + .uri("/303") + .responseContent() + .aggregate() + .asString()) + .expectNextMatches("OK"::equals) + .expectComplete() + .verify(Duration.ofSeconds(30)); + StepVerifier.create(client.followRedirect(true) .get() .uri("/307") From 522daa2738456c2d028f727ca88026398a086071 Mon Sep 17 00:00:00 2001 From: Violeta Georgieva Date: Thu, 12 May 2022 13:45:37 +0300 Subject: [PATCH 15/42] Update the reference documentation for auto-redirect support Related to #2184 --- docs/asciidoc/http-client.adoc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/asciidoc/http-client.adoc b/docs/asciidoc/http-client.adoc index e8e36be2d2..25cf772744 100644 --- a/docs/asciidoc/http-client.adoc +++ b/docs/asciidoc/http-client.adoc @@ -131,7 +131,8 @@ You can configure the `HTTP` client to enable auto-redirect support. Reactor Netty provides two different strategies for auto-redirect support: -* `followRedirect(boolean)`: Specifies whether HTTP auto-redirect support is enabled for statuses `301|302|307|308`. +* `followRedirect(boolean)`: Specifies whether HTTP auto-redirect support is enabled for statuses `301|302|303|307|308`. +When it is `303` status code, `GET` method is used for the redirect. * `followRedirect(BiPredicate)`: Enables auto-redirect support if the supplied predicate matches. From 433b9b2d0ee1c687e1cf88ffbc51800380a46c01 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Fri, 13 May 2022 07:54:54 +0300 Subject: [PATCH 16/42] Bump build-info-extractor-gradle from 4.28.2 to 4.28.3 (#2190) Bumps build-info-extractor-gradle from 4.28.2 to 4.28.3. --- updated-dependencies: - dependency-name: org.jfrog.buildinfo:build-info-extractor-gradle dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.gradle b/build.gradle index ac396f29a1..8836bcff9c 100644 --- a/build.gradle +++ b/build.gradle @@ -23,7 +23,7 @@ buildscript { maven { url "https://repo.spring.io/plugins-release" } } dependencies { - classpath 'org.jfrog.buildinfo:build-info-extractor-gradle:4.28.2' //applied in individual submodules + classpath 'org.jfrog.buildinfo:build-info-extractor-gradle:4.28.3' //applied in individual submodules } } From 6503da8d84fe0203e1ffdac06109f32140fd8ccc Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 16 May 2022 07:45:54 +0300 Subject: [PATCH 17/42] Bump jackson-databind from 2.13.2.2 to 2.13.3 (#2191) Bumps [jackson-databind](https://github.com/FasterXML/jackson) from 2.13.2.2 to 2.13.3. - [Release notes](https://github.com/FasterXML/jackson/releases) - [Commits](https://github.com/FasterXML/jackson/commits) --- updated-dependencies: - dependency-name: com.fasterxml.jackson.core:jackson-databind dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.gradle b/build.gradle index 8836bcff9c..6b7978c1ad 100644 --- a/build.gradle +++ b/build.gradle @@ -107,7 +107,7 @@ ext { nettyQuicVersion = '0.0.27.Final' // Testing - jacksonDatabindVersion = '2.13.2.2' + jacksonDatabindVersion = '2.13.3' testAddonVersion = reactorCoreVersion assertJVersion = '3.22.0' awaitilityVersion = '4.2.0' From e4ea2ff659108d1b5e07164974bf32896471d623 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 16 May 2022 08:17:46 +0300 Subject: [PATCH 18/42] Bump com.diffplug.spotless from 6.6.0 to 6.6.1 (#2192) Bumps com.diffplug.spotless from 6.6.0 to 6.6.1. --- updated-dependencies: - dependency-name: com.diffplug.spotless dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.gradle b/build.gradle index 6b7978c1ad..0ee468a647 100644 --- a/build.gradle +++ b/build.gradle @@ -28,7 +28,7 @@ buildscript { } plugins { - id "com.diffplug.spotless" version "6.6.0" + id "com.diffplug.spotless" version "6.6.1" id 'org.asciidoctor.jvm.convert' version '3.3.2' apply false id 'org.asciidoctor.jvm.pdf' version '3.3.2' apply false id 'com.google.osdetector' version '1.7.0' From 600c38c1166063032e026457d858f2ed652730d0 Mon Sep 17 00:00:00 2001 From: Violeta Georgieva Date: Mon, 16 May 2022 19:57:03 +0300 Subject: [PATCH 19/42] [tests] Add bouncycastle dependency (#2195) Add bouncycastle dependency for noMicrometer tests similar to the standard tests. --- reactor-netty-http/build.gradle | 1 + 1 file changed, 1 insertion(+) diff --git a/reactor-netty-http/build.gradle b/reactor-netty-http/build.gradle index 5ead618e81..4b758ba6cc 100644 --- a/reactor-netty-http/build.gradle +++ b/reactor-netty-http/build.gradle @@ -149,6 +149,7 @@ dependencies { // https://github.com/netty/netty/issues/10317 // Necessary for generating SelfSignedCertificate on Java version >= 15 testRuntimeOnly "org.bouncycastle:bcpkix-jdk15on:$bouncycastleVersion" + noMicrometerTestRuntimeOnly "org.bouncycastle:bcpkix-jdk15on:$bouncycastleVersion" } // noMicrometerTest sourceSet (must not include Micrometer) From 0d226d94a8727c9929c532ceca49119917afbd71 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 17 May 2022 09:37:10 +0300 Subject: [PATCH 20/42] Bump tomcat-embed-core from 9.0.62 to 9.0.63 (#2197) Bumps tomcat-embed-core from 9.0.62 to 9.0.63. --- updated-dependencies: - dependency-name: org.apache.tomcat.embed:tomcat-embed-core dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.gradle b/build.gradle index 0ee468a647..622bc66177 100644 --- a/build.gradle +++ b/build.gradle @@ -112,7 +112,7 @@ ext { assertJVersion = '3.22.0' awaitilityVersion = '4.2.0' hoverflyJavaVersion = '0.14.1' - tomcatVersion = '9.0.62' + tomcatVersion = '9.0.63' boringSslVersion = '2.0.52.Final' junitVersion = '5.8.2' junitPlatformLauncherVersion = '1.8.2' From ed94c5632c058a2b14c02c29a33eb6537754cca5 Mon Sep 17 00:00:00 2001 From: Violeta Georgieva Date: Tue, 17 May 2022 11:03:46 +0300 Subject: [PATCH 21/42] [tests] Add SocketException to the list with possible exceptions (#2198) `SocketException` with message `Connection reset` may be thrown when running the tests with Java 17 --- .../test/java/reactor/netty/http/client/HttpClientTest.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/reactor-netty-http/src/test/java/reactor/netty/http/client/HttpClientTest.java b/reactor-netty-http/src/test/java/reactor/netty/http/client/HttpClientTest.java index c838cf0eda..5c8aa2182f 100644 --- a/reactor-netty-http/src/test/java/reactor/netty/http/client/HttpClientTest.java +++ b/reactor-netty-http/src/test/java/reactor/netty/http/client/HttpClientTest.java @@ -1316,7 +1316,8 @@ private void doTestRetry(boolean retryDisabled, boolean expectRetry) throws Exce error.set(t); return t.getMessage() != null && (t.getMessage().contains("Connection reset by peer") || - t.getMessage().contains("readAddress(..)") || // https://github.com/reactor/reactor-netty/issues/1673 + t.getMessage().contains("Connection reset") || + t.getMessage().contains("readAddress(..)") || // https://github.com/reactor/reactor-netty/issues/1673 t.getMessage().contains("Connection prematurely closed BEFORE response")); }) .verify(Duration.ofSeconds(30)); From 78eadd273254b27c3662d8cdba30f859106b07f8 Mon Sep 17 00:00:00 2001 From: Violeta Georgieva Date: Tue, 17 May 2022 16:37:20 +0300 Subject: [PATCH 22/42] Ensure DefaultHttpDataFactory.requestFileDeleteMap's items are cleaned on terminate (#2201) Fixes #2183 --- .../http/client/HttpClientOperations.java | 32 ++++++++---- .../test/java/reactor/netty/TomcatServer.java | 11 ++-- .../http/client/HttpClientWithTomcatTest.java | 51 ++++++++++++++----- 3 files changed, 67 insertions(+), 27 deletions(-) diff --git a/reactor-netty-http/src/main/java/reactor/netty/http/client/HttpClientOperations.java b/reactor-netty-http/src/main/java/reactor/netty/http/client/HttpClientOperations.java index 1edd53a500..9577c82f21 100644 --- a/reactor-netty-http/src/main/java/reactor/netty/http/client/HttpClientOperations.java +++ b/reactor-netty-http/src/main/java/reactor/netty/http/client/HttpClientOperations.java @@ -903,27 +903,39 @@ void _subscribe(CoreSubscriber s) { ChannelFuture f = parent.channel() .writeAndFlush(r); - Flux tail = encoder.progressSink.asFlux().onBackpressureLatest(); + if (encoder.isChunked()) { + Flux tail = encoder.progressSink.asFlux().onBackpressureLatest(); - if (encoder.cleanOnTerminate) { - tail = tail.doOnCancel(encoder) - .doAfterTerminate(encoder); - } + if (encoder.cleanOnTerminate) { + tail = tail.doOnCancel(encoder) + .doAfterTerminate(encoder); + } - if (encoder.isChunked()) { if (progressCallback != null) { progressCallback.accept(tail); } + else { + tail.subscribe(); + } //"FutureReturnValueIgnored" this is deliberate parent.channel() .writeAndFlush(encoder); } else { + Mono mono = FutureMono.from(f); + + if (encoder.cleanOnTerminate) { + mono = mono.doOnCancel(encoder) + .doAfterTerminate(encoder); + } + if (progressCallback != null) { - progressCallback.accept(FutureMono.from(f) - .cast(Long.class) - .switchIfEmpty(Mono.just(encoder.length())) - .flux()); + progressCallback.accept(mono.cast(Long.class) + .switchIfEmpty(Mono.just(encoder.length())) + .flux()); + } + else { + mono.subscribe(); } } s.onComplete(); diff --git a/reactor-netty-http/src/test/java/reactor/netty/TomcatServer.java b/reactor-netty-http/src/test/java/reactor/netty/TomcatServer.java index f80daf853d..4da3bc9a23 100644 --- a/reactor-netty-http/src/test/java/reactor/netty/TomcatServer.java +++ b/reactor-netty-http/src/test/java/reactor/netty/TomcatServer.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2019-2021 VMware, Inc. or its affiliates, All Rights Reserved. + * Copyright (c) 2019-2022 VMware, Inc. or its affiliates, All Rights Reserved. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -132,8 +132,13 @@ protected void service(HttpServletRequest req, HttpServletResponse resp) throws StringBuilder builder = new StringBuilder(); - Collection parts = req.getParts(); - parts.forEach(p -> builder.append(p.getName()).append(' ')); + if ("application/x-www-form-urlencoded".equals(req.getHeader("content-type"))) { + builder.append(req.getReader().readLine()); + } + else { + Collection parts = req.getParts(); + parts.forEach(p -> builder.append(p.getName()).append(' ')); + } writer.print(builder); writer.flush(); diff --git a/reactor-netty-http/src/test/java/reactor/netty/http/client/HttpClientWithTomcatTest.java b/reactor-netty-http/src/test/java/reactor/netty/http/client/HttpClientWithTomcatTest.java index c53d4fa758..c77cc17fb3 100644 --- a/reactor-netty-http/src/test/java/reactor/netty/http/client/HttpClientWithTomcatTest.java +++ b/reactor-netty-http/src/test/java/reactor/netty/http/client/HttpClientWithTomcatTest.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2019-2021 VMware, Inc. or its affiliates, All Rights Reserved. + * Copyright (c) 2019-2022 VMware, Inc. or its affiliates, All Rights Reserved. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -20,7 +20,9 @@ import io.netty.handler.codec.http.HttpHeaderValues; import io.netty.handler.codec.http.HttpHeaders; import io.netty.handler.codec.http.HttpMethod; +import io.netty.handler.codec.http.HttpRequest; import io.netty.handler.codec.http.HttpResponseStatus; +import io.netty.handler.codec.http.multipart.HttpData; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; @@ -33,16 +35,21 @@ import reactor.util.function.Tuples; import java.io.InputStream; +import java.lang.reflect.Field; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; import java.time.Duration; +import java.util.List; +import java.util.Map; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicReference; +import java.util.function.BiConsumer; import static org.assertj.core.api.Assertions.assertThat; +import static reactor.netty.http.client.HttpClientOperations.SendForm.DEFAULT_FACTORY; /** * @author Violeta Georgieva @@ -86,7 +93,25 @@ void nettyNetChannelAcceptsNettyChannelHandlers() throws Exception { } @Test - void postUpload() throws Exception { + void postUploadWithMultipart() throws Exception { + Path file = Paths.get(getClass().getResource("/smallFile.txt").toURI()); + try (InputStream f = Files.newInputStream(file)) { + doTestPostUpload((req, form) -> form.multipart(true) + .file("test", f) + .attr("attr1", "attr2") + .file("test2", f), + "test attr1 test2 "); + } + } + + @Test + void postUploadNoMultipart() throws Exception { + doTestPostUpload((req, form) -> form.multipart(false).attr("attr1", "attr2"), "attr1=attr2"); + } + + @SuppressWarnings("unchecked") + private void doTestPostUpload(BiConsumer formCallback, + String expectedResponse) throws Exception { HttpClient client = HttpClient.create() .host("localhost") @@ -94,21 +119,19 @@ void postUpload() throws Exception { .wiretap(true); Tuple2 res; - Path file = Paths.get(getClass().getResource("/smallFile.txt").toURI()); - try (InputStream f = Files.newInputStream(file)) { - res = client.post() - .uri("/multipart") - .sendForm((req, form) -> form.multipart(true) - .file("test", f) - .attr("attr1", "attr2") - .file("test2", f)) - .responseSingle((r, buf) -> buf.asString().map(s -> Tuples.of(r.status().code(), s))) - .block(Duration.ofSeconds(30)); - } + res = client.post() + .uri("/multipart") + .sendForm(formCallback) + .responseSingle((r, buf) -> buf.asString().map(s -> Tuples.of(r.status().code(), s))) + .block(Duration.ofSeconds(30)); assertThat(res).as("response").isNotNull(); assertThat(res.getT1()).as("status code").isEqualTo(200); - assertThat(res.getT2()).as("response body reflecting request").contains("test attr1 test2 "); + assertThat(res.getT2()).as("response body reflecting request").contains(expectedResponse); + + Field field = DEFAULT_FACTORY.getClass().getDeclaredField("requestFileDeleteMap"); + field.setAccessible(true); + assertThat((Map>) field.get(DEFAULT_FACTORY)).isEmpty(); } @Test From eb1b478f27a11112cec895d7aaaa31e03226621d Mon Sep 17 00:00:00 2001 From: Violeta Georgieva Date: Wed, 18 May 2022 10:07:49 +0300 Subject: [PATCH 23/42] Configure dependabot to update GitHub Actions versions (#2205) --- .github/dependabot.yml | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/.github/dependabot.yml b/.github/dependabot.yml index d09ba50356..071bce0d0a 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -18,3 +18,14 @@ updates: versions: - ">= 10.0.a" rebase-strategy: disabled +- package-ecosystem: github-actions + directory: "/" + schedule: + interval: daily + open-pull-requests-limit: 10 + assignees: + - violetagg + target-branch: "1.0.x" + labels: + - type/dependency-upgrade + rebase-strategy: disabled From 13110589b3927891b734fd7cd0a2afdcb4f0c2d8 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 18 May 2022 10:43:51 +0300 Subject: [PATCH 24/42] Bump actions/setup-java from 2 to 3 (#2206) Bumps [actions/setup-java](https://github.com/actions/setup-java) from 2 to 3. - [Release notes](https://github.com/actions/setup-java/releases) - [Commits](https://github.com/actions/setup-java/compare/v2...v3) --- updated-dependencies: - dependency-name: actions/setup-java dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- .github/workflows/check_netty_snapshots.yml | 2 +- .github/workflows/check_transport.yml | 4 ++-- .github/workflows/publish.yml | 8 ++++---- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/.github/workflows/check_netty_snapshots.yml b/.github/workflows/check_netty_snapshots.yml index 375be09491..cabdac4c3c 100644 --- a/.github/workflows/check_netty_snapshots.yml +++ b/.github/workflows/check_netty_snapshots.yml @@ -24,7 +24,7 @@ jobs: steps: - uses: actions/checkout@v2 - name: Set up JDK 1.8 - uses: actions/setup-java@v2 + uses: actions/setup-java@v3 with: distribution: 'temurin' java-version: '8' diff --git a/.github/workflows/check_transport.yml b/.github/workflows/check_transport.yml index 8182939bbf..62e00c6b7e 100644 --- a/.github/workflows/check_transport.yml +++ b/.github/workflows/check_transport.yml @@ -11,7 +11,7 @@ jobs: - uses: actions/checkout@v2 with: fetch-depth: 0 #needed by spotless - - uses: actions/setup-java@v2 + - uses: actions/setup-java@v3 with: distribution: 'temurin' java-version: 8 @@ -50,7 +50,7 @@ jobs: - uses: actions/checkout@v2 - uses: gradle/wrapper-validation-action@v1 - name: Set up JDK 1.8 - uses: actions/setup-java@v2 + uses: actions/setup-java@v3 with: distribution: 'temurin' java-version: '8' diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index 65bab92dd2..854b97f285 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -19,7 +19,7 @@ jobs: steps: - uses: actions/checkout@v2 - name: setup java - uses: actions/setup-java@v2 + uses: actions/setup-java@v3 with: distribution: 'temurin' java-version: '8' @@ -42,7 +42,7 @@ jobs: environment: snapshots steps: - uses: actions/checkout@v2 - - uses: actions/setup-java@v2 + - uses: actions/setup-java@v3 with: distribution: 'temurin' java-version: '8' @@ -62,7 +62,7 @@ jobs: environment: releases steps: - uses: actions/checkout@v2 - - uses: actions/setup-java@v2 + - uses: actions/setup-java@v3 with: distribution: 'temurin' java-version: '8' @@ -84,7 +84,7 @@ jobs: environment: releases steps: - uses: actions/checkout@v2 - - uses: actions/setup-java@v2 + - uses: actions/setup-java@v3 with: distribution: 'temurin' java-version: '8' From 0b29b6dcf56957fdd146429035b80bee0f03f1e7 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 18 May 2022 11:17:33 +0300 Subject: [PATCH 25/42] Bump actions/checkout from 2 to 3 (#2207) Bumps [actions/checkout](https://github.com/actions/checkout) from 2 to 3. - [Release notes](https://github.com/actions/checkout/releases) - [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md) - [Commits](https://github.com/actions/checkout/compare/v2...v3) --- updated-dependencies: - dependency-name: actions/checkout dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- .github/workflows/check_netty_snapshots.yml | 2 +- .github/workflows/check_transport.yml | 4 ++-- .github/workflows/codeql-analysis.yml | 2 +- .github/workflows/publish.yml | 12 ++++++------ 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/.github/workflows/check_netty_snapshots.yml b/.github/workflows/check_netty_snapshots.yml index cabdac4c3c..fc7b20c031 100644 --- a/.github/workflows/check_netty_snapshots.yml +++ b/.github/workflows/check_netty_snapshots.yml @@ -22,7 +22,7 @@ jobs: transport: native steps: - - uses: actions/checkout@v2 + - uses: actions/checkout@v3 - name: Set up JDK 1.8 uses: actions/setup-java@v3 with: diff --git a/.github/workflows/check_transport.yml b/.github/workflows/check_transport.yml index 62e00c6b7e..1f2b7e3d63 100644 --- a/.github/workflows/check_transport.yml +++ b/.github/workflows/check_transport.yml @@ -8,7 +8,7 @@ jobs: name: preliminary sanity checks runs-on: ubuntu-20.04 steps: - - uses: actions/checkout@v2 + - uses: actions/checkout@v3 with: fetch-depth: 0 #needed by spotless - uses: actions/setup-java@v3 @@ -47,7 +47,7 @@ jobs: - os: windows-2019 transport: native steps: - - uses: actions/checkout@v2 + - uses: actions/checkout@v3 - uses: gradle/wrapper-validation-action@v1 - name: Set up JDK 1.8 uses: actions/setup-java@v3 diff --git a/.github/workflows/codeql-analysis.yml b/.github/workflows/codeql-analysis.yml index a5b53acba1..626c735e83 100644 --- a/.github/workflows/codeql-analysis.yml +++ b/.github/workflows/codeql-analysis.yml @@ -38,7 +38,7 @@ jobs: steps: - name: Checkout repository - uses: actions/checkout@v2 + uses: actions/checkout@v3 # Initializes the CodeQL tools for scanning. - name: Initialize CodeQL diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index 854b97f285..77b391b79e 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -17,7 +17,7 @@ jobs: versionType: ${{ steps.version.outputs.versionType }} fullVersion: ${{ steps.version.outputs.fullVersion }} steps: - - uses: actions/checkout@v2 + - uses: actions/checkout@v3 - name: setup java uses: actions/setup-java@v3 with: @@ -41,7 +41,7 @@ jobs: if: needs.prepare.outputs.versionType == 'SNAPSHOT' environment: snapshots steps: - - uses: actions/checkout@v2 + - uses: actions/checkout@v3 - uses: actions/setup-java@v3 with: distribution: 'temurin' @@ -61,7 +61,7 @@ jobs: if: needs.prepare.outputs.versionType == 'MILESTONE' environment: releases steps: - - uses: actions/checkout@v2 + - uses: actions/checkout@v3 - uses: actions/setup-java@v3 with: distribution: 'temurin' @@ -83,7 +83,7 @@ jobs: if: needs.prepare.outputs.versionType == 'RELEASE' environment: releases steps: - - uses: actions/checkout@v2 + - uses: actions/checkout@v3 - uses: actions/setup-java@v3 with: distribution: 'temurin' @@ -106,7 +106,7 @@ jobs: permissions: contents: write steps: - - uses: actions/checkout@v2 + - uses: actions/checkout@v3 - name: tag run: | git config --local user.name 'reactorbot' @@ -121,7 +121,7 @@ jobs: permissions: contents: write steps: - - uses: actions/checkout@v2 + - uses: actions/checkout@v3 - name: tag run: | git config --local user.name 'reactorbot' From be2765a10f61446f8261e2677f0c0427a181a2ee Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 18 May 2022 11:22:16 +0300 Subject: [PATCH 26/42] Bump github/codeql-action from 1 to 2 (#2208) Bumps [github/codeql-action](https://github.com/github/codeql-action) from 1 to 2. - [Release notes](https://github.com/github/codeql-action/releases) - [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md) - [Commits](https://github.com/github/codeql-action/compare/v1...v2) --- updated-dependencies: - dependency-name: github/codeql-action dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- .github/workflows/codeql-analysis.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/codeql-analysis.yml b/.github/workflows/codeql-analysis.yml index 626c735e83..94be423e99 100644 --- a/.github/workflows/codeql-analysis.yml +++ b/.github/workflows/codeql-analysis.yml @@ -42,7 +42,7 @@ jobs: # Initializes the CodeQL tools for scanning. - name: Initialize CodeQL - uses: github/codeql-action/init@v1 + uses: github/codeql-action/init@v2 with: languages: ${{ matrix.language }} # If you wish to specify custom queries, you can do so here or in a config file. @@ -53,7 +53,7 @@ jobs: # Autobuild attempts to build any compiled languages (C/C++, C#, or Java). # If this step fails, then you should remove it and run the build manually (see below) - name: Autobuild - uses: github/codeql-action/autobuild@v1 + uses: github/codeql-action/autobuild@v2 # ℹ️ Command-line programs to run using the OS shell. # 📚 https://git.io/JvXDl @@ -67,4 +67,4 @@ jobs: # make release - name: Perform CodeQL Analysis - uses: github/codeql-action/analyze@v1 + uses: github/codeql-action/analyze@v2 From 30bbdc94d3e0f877c7052c2b28e35b35340f9ca7 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 23 May 2022 09:29:16 +0300 Subject: [PATCH 27/42] Bump hoverfly-java-junit5 from 0.14.1 to 0.14.2 (#2236) Bumps [hoverfly-java-junit5](https://github.com/SpectoLabs/hoverfly-java) from 0.14.1 to 0.14.2. - [Release notes](https://github.com/SpectoLabs/hoverfly-java/releases) - [Commits](https://github.com/SpectoLabs/hoverfly-java/compare/0.14.1...0.14.2) --- updated-dependencies: - dependency-name: io.specto:hoverfly-java-junit5 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.gradle b/build.gradle index 622bc66177..dfd8514e21 100644 --- a/build.gradle +++ b/build.gradle @@ -111,7 +111,7 @@ ext { testAddonVersion = reactorCoreVersion assertJVersion = '3.22.0' awaitilityVersion = '4.2.0' - hoverflyJavaVersion = '0.14.1' + hoverflyJavaVersion = '0.14.2' tomcatVersion = '9.0.63' boringSslVersion = '2.0.52.Final' junitVersion = '5.8.2' From 7d4587257e668dcc805716ecf09d790887d91b5e Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 30 May 2022 07:30:05 +0300 Subject: [PATCH 28/42] Bump mockito-core from 4.5.1 to 4.6.0 (#2255) Bumps [mockito-core](https://github.com/mockito/mockito) from 4.5.1 to 4.6.0. - [Release notes](https://github.com/mockito/mockito/releases) - [Commits](https://github.com/mockito/mockito/compare/v4.5.1...v4.6.0) --- updated-dependencies: - dependency-name: org.mockito:mockito-core dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.gradle b/build.gradle index dfd8514e21..7e00026eb6 100644 --- a/build.gradle +++ b/build.gradle @@ -116,7 +116,7 @@ ext { boringSslVersion = '2.0.52.Final' junitVersion = '5.8.2' junitPlatformLauncherVersion = '1.8.2' - mockitoVersion = '4.5.1' + mockitoVersion = '4.6.0' blockHoundVersion = '1.0.6.RELEASE' bouncycastleVersion = '1.70' From 81895a34988a686106ed481f20fe031af6830c46 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 1 Jun 2022 07:43:53 +0300 Subject: [PATCH 29/42] Bump assertj-core from 3.22.0 to 3.23.1 (#2258) Bumps [assertj-core](https://github.com/assertj/assertj-core) from 3.22.0 to 3.23.1. - [Release notes](https://github.com/assertj/assertj-core/releases) - [Commits](https://github.com/assertj/assertj-core/compare/assertj-core-3.22.0...assertj-core-3.23.1) --- updated-dependencies: - dependency-name: org.assertj:assertj-core dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.gradle b/build.gradle index 7e00026eb6..b3a511a64c 100644 --- a/build.gradle +++ b/build.gradle @@ -109,7 +109,7 @@ ext { // Testing jacksonDatabindVersion = '2.13.3' testAddonVersion = reactorCoreVersion - assertJVersion = '3.22.0' + assertJVersion = '3.23.1' awaitilityVersion = '4.2.0' hoverflyJavaVersion = '0.14.2' tomcatVersion = '9.0.63' From 3cff6973fee56a5d9d3a9f9b6319dfce1e319ace Mon Sep 17 00:00:00 2001 From: Tyler Jones Date: Thu, 2 Jun 2022 07:49:13 -0600 Subject: [PATCH 30/42] Update faq for connection closed BEFORE error (#2263) `max-keep-alive-requests` might be a reason for connection close Co-authored-by: Violeta Georgieva --- docs/asciidoc/faq.adoc | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/asciidoc/faq.adoc b/docs/asciidoc/faq.adoc index dde767fda0..200c76b67e 100644 --- a/docs/asciidoc/faq.adoc +++ b/docs/asciidoc/faq.adoc @@ -136,6 +136,7 @@ Issues related to TCP keep-alive configuration on various load balancers were re ** limit for buffering data in memory ** multipart exceeds the max file size limit ** bad request +* Check the server.tomcat.max-keep-alive-requests property. Consider checking <>. The section describes various timeout configuration options that are available for Reactor Netty clients. Configuring a proper timeout may improve or solve issues in the communication process. From 1c4be66c325209f9d6da6f02bf3db86e7ceecdb3 Mon Sep 17 00:00:00 2001 From: Violeta Georgieva Date: Thu, 2 Jun 2022 16:54:33 +0300 Subject: [PATCH 31/42] Polish --- docs/asciidoc/faq.adoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/asciidoc/faq.adoc b/docs/asciidoc/faq.adoc index 200c76b67e..02bc9d86c6 100644 --- a/docs/asciidoc/faq.adoc +++ b/docs/asciidoc/faq.adoc @@ -136,7 +136,7 @@ Issues related to TCP keep-alive configuration on various load balancers were re ** limit for buffering data in memory ** multipart exceeds the max file size limit ** bad request -* Check the server.tomcat.max-keep-alive-requests property. +** max keep alive requests (the connection is closed when the requests reach the configured maximum number) Consider checking <>. The section describes various timeout configuration options that are available for Reactor Netty clients. Configuring a proper timeout may improve or solve issues in the communication process. From e28067f5ce03ddff3ee7fdcce7db47c14687bde3 Mon Sep 17 00:00:00 2001 From: Violeta Georgieva Date: Thu, 2 Jun 2022 19:48:10 +0300 Subject: [PATCH 32/42] Ensure a custom factory can be used with HttpClient#sendForm (#2265) HttpClient#sendForm API uses BiConsumer and not BiFunction. The user cannot change the encoder in the callback. When a custom factory is provided, apply the changes when the control is again in Reactor Netty. Fixes #2259 --- .../netty/http/client/HttpClientFormEncoder.java | 6 ++++-- .../netty/http/client/HttpClientWithTomcatTest.java | 11 +++++++++++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/reactor-netty-http/src/main/java/reactor/netty/http/client/HttpClientFormEncoder.java b/reactor-netty-http/src/main/java/reactor/netty/http/client/HttpClientFormEncoder.java index b1916bb1da..34830fb8d1 100644 --- a/reactor-netty-http/src/main/java/reactor/netty/http/client/HttpClientFormEncoder.java +++ b/reactor-netty-http/src/main/java/reactor/netty/http/client/HttpClientFormEncoder.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2011-2021 VMware, Inc. or its affiliates, All Rights Reserved. + * Copyright (c) 2011-2022 VMware, Inc. or its affiliates, All Rights Reserved. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -131,7 +131,7 @@ public HttpClientForm factory(HttpDataFactory factory) { } this.newFactory = Objects.requireNonNull(factory, "factory"); this.needNewEncoder = true; - return applyChanges(request); + return this; } @Override @@ -317,6 +317,8 @@ final HttpClientFormEncoder applyChanges(HttpRequest request) { encoder.setBodyHttpDatas(getBodyListAttributes()); + needNewEncoder = false; + return encoder; } catch (ErrorDataEncoderException ee) { diff --git a/reactor-netty-http/src/test/java/reactor/netty/http/client/HttpClientWithTomcatTest.java b/reactor-netty-http/src/test/java/reactor/netty/http/client/HttpClientWithTomcatTest.java index c77cc17fb3..a4ff369b9c 100644 --- a/reactor-netty-http/src/test/java/reactor/netty/http/client/HttpClientWithTomcatTest.java +++ b/reactor-netty-http/src/test/java/reactor/netty/http/client/HttpClientWithTomcatTest.java @@ -22,6 +22,7 @@ import io.netty.handler.codec.http.HttpMethod; import io.netty.handler.codec.http.HttpRequest; import io.netty.handler.codec.http.HttpResponseStatus; +import io.netty.handler.codec.http.multipart.DefaultHttpDataFactory; import io.netty.handler.codec.http.multipart.HttpData; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; @@ -109,6 +110,16 @@ void postUploadNoMultipart() throws Exception { doTestPostUpload((req, form) -> form.multipart(false).attr("attr1", "attr2"), "attr1=attr2"); } + @Test + void postUploadNoMultipartWithCustomFactory() throws Exception { + doTestPostUpload((req, form) -> { + DefaultHttpDataFactory customFactory = new DefaultHttpDataFactory(DefaultHttpDataFactory.MINSIZE); + form.factory(customFactory) + .multipart(false) + .attr("attr1", "attr2"); + }, "attr1=attr2"); + } + @SuppressWarnings("unchecked") private void doTestPostUpload(BiConsumer formCallback, String expectedResponse) throws Exception { From d4825cccd72c686130a8c7fa75e3cdf08c9e2dec Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Fri, 3 Jun 2022 09:10:39 +0300 Subject: [PATCH 33/42] Bump mockito-core from 4.6.0 to 4.6.1 (#2266) Bumps [mockito-core](https://github.com/mockito/mockito) from 4.6.0 to 4.6.1. - [Release notes](https://github.com/mockito/mockito/releases) - [Commits](https://github.com/mockito/mockito/compare/v4.6.0...v4.6.1) --- updated-dependencies: - dependency-name: org.mockito:mockito-core dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.gradle b/build.gradle index b3a511a64c..45d061d4da 100644 --- a/build.gradle +++ b/build.gradle @@ -116,7 +116,7 @@ ext { boringSslVersion = '2.0.52.Final' junitVersion = '5.8.2' junitPlatformLauncherVersion = '1.8.2' - mockitoVersion = '4.6.0' + mockitoVersion = '4.6.1' blockHoundVersion = '1.0.6.RELEASE' bouncycastleVersion = '1.70' From aa1f7756784ca4653d1b29e53e9fe2bfc217fd0e Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 6 Jun 2022 08:34:04 +0300 Subject: [PATCH 34/42] Bump com.diffplug.spotless from 6.6.1 to 6.7.0 (#2267) Bumps com.diffplug.spotless from 6.6.1 to 6.7.0. --- updated-dependencies: - dependency-name: com.diffplug.spotless dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.gradle b/build.gradle index 45d061d4da..f407e25f27 100644 --- a/build.gradle +++ b/build.gradle @@ -28,7 +28,7 @@ buildscript { } plugins { - id "com.diffplug.spotless" version "6.6.1" + id "com.diffplug.spotless" version "6.7.0" id 'org.asciidoctor.jvm.convert' version '3.3.2' apply false id 'org.asciidoctor.jvm.pdf' version '3.3.2' apply false id 'com.google.osdetector' version '1.7.0' From d9f8401e1270b4b02d64da9dbd16e0db8257672f Mon Sep 17 00:00:00 2001 From: Violeta Georgieva Date: Tue, 7 Jun 2022 16:03:45 +0300 Subject: [PATCH 35/42] Update Reactive Streams javadoc to version 1.0.4 (#2268) --- build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.gradle b/build.gradle index f407e25f27..599f0d9e11 100644 --- a/build.gradle +++ b/build.gradle @@ -128,7 +128,7 @@ ext { println "JDK Javadoc link for this build is ${jdkJavadoc}" javadocLinks = [jdkJavadoc, "https://fasterxml.github.io/jackson-databind/javadoc/2.5/", - "https://www.reactive-streams.org/reactive-streams-1.0.3-javadoc/", + "https://www.reactive-streams.org/reactive-streams-1.0.4-javadoc/", "https://projectreactor.io/docs/core/release/api/", "https://netty.io/4.1/api/", "https://projectreactor.io/docs/netty/release/api/",] as String[] From 738f5df371f50c243d16e29cea7bdcf61883d8d6 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 8 Jun 2022 08:58:41 +0300 Subject: [PATCH 36/42] Bump biz.aQute.bnd.builder from 6.2.0 to 6.3.1 (#2269) Bumps biz.aQute.bnd.builder from 6.2.0 to 6.3.1. --- updated-dependencies: - dependency-name: biz.aQute.bnd.builder dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.gradle b/build.gradle index 599f0d9e11..5fec14e17f 100644 --- a/build.gradle +++ b/build.gradle @@ -40,7 +40,7 @@ plugins { id 'de.undercouch.download' version '5.1.0' apply false id 'io.spring.javadoc' version '0.0.1' apply false id 'io.spring.javadoc-aggregate' version '0.0.1' apply false - id 'biz.aQute.bnd.builder' version '6.2.0' apply false + id 'biz.aQute.bnd.builder' version '6.3.1' apply false } description = 'Reactive Streams Netty driver' From f5e79b7ac647905675b24d2dc11fbd92e6b863d8 Mon Sep 17 00:00:00 2001 From: Violeta Georgieva Date: Mon, 30 May 2022 12:22:34 +0300 Subject: [PATCH 37/42] Http2Pool handles the lifecycle of the cache with connections (#2257) Cache Http2FrameCodec/Http2MultiplexHandler/H2CUpgradeHandler context. Obtain the negotiated application level protocol once. Related to #2151 and #2262 --- .../resources/PooledConnectionProvider.java | 10 +- .../reactor/netty/http/HttpResources.java | 10 + .../http/client/Http2ConnectionProvider.java | 34 +-- .../reactor/netty/http/client/Http2Pool.java | 242 ++++++++++++------ ...Http2ConnectionProviderMeterRegistrar.java | 17 +- .../netty/http/client/Http2PoolTest.java | 138 ++++++---- .../DefaultPooledConnectionProviderTest.java | 120 +++++++-- ...dConnectionProviderDefaultMetricsTest.java | 8 +- 8 files changed, 408 insertions(+), 171 deletions(-) diff --git a/reactor-netty-core/src/main/java/reactor/netty/resources/PooledConnectionProvider.java b/reactor-netty-core/src/main/java/reactor/netty/resources/PooledConnectionProvider.java index a19816e512..25947313e2 100644 --- a/reactor-netty-core/src/main/java/reactor/netty/resources/PooledConnectionProvider.java +++ b/reactor-netty-core/src/main/java/reactor/netty/resources/PooledConnectionProvider.java @@ -89,6 +89,7 @@ public abstract class PooledConnectionProvider implements final Duration poolInactivity; final Duration disposeTimeout; final Map maxConnections = new HashMap<>(); + Mono onDispose; protected PooledConnectionProvider(Builder builder) { this(builder, null); @@ -106,6 +107,7 @@ protected PooledConnectionProvider(Builder builder) { poolFactoryPerRemoteHost.put(entry.getKey(), new PoolFactory<>(entry.getValue(), builder.disposeTimeout)); maxConnections.put(entry.getKey(), entry.getValue().maxConnections); } + this.onDispose = Mono.empty(); scheduleInactivePoolsDisposal(); } @@ -190,10 +192,10 @@ public final Mono disposeLater() { }) .collect(Collectors.toList()); if (pools.isEmpty()) { - return Mono.empty(); + return onDispose; } channelPools.clear(); - return Mono.when(pools); + return onDispose.and(Mono.when(pools)); }); } @@ -243,6 +245,10 @@ public String name() { return name; } + public void onDispose(Mono disposeMono) { + onDispose = onDispose.and(disposeMono); + } + protected abstract CoreSubscriber> createDisposableAcquire( TransportConfig config, ConnectionObserver connectionObserver, diff --git a/reactor-netty-http/src/main/java/reactor/netty/http/HttpResources.java b/reactor-netty-http/src/main/java/reactor/netty/http/HttpResources.java index 801dcac824..a94268145b 100644 --- a/reactor-netty-http/src/main/java/reactor/netty/http/HttpResources.java +++ b/reactor-netty-http/src/main/java/reactor/netty/http/HttpResources.java @@ -15,6 +15,7 @@ */ package reactor.netty.http; +import java.net.SocketAddress; import java.time.Duration; import java.util.concurrent.atomic.AtomicReference; import java.util.function.BiFunction; @@ -147,6 +148,15 @@ public static HttpResources set(LoopResources loops) { http2ConnectionProvider = new AtomicReference<>(); } + @Override + public void disposeWhen(SocketAddress remoteAddress) { + ConnectionProvider provider = http2ConnectionProvider.get(); + if (provider != null) { + provider.disposeWhen(remoteAddress); + } + super.disposeWhen(remoteAddress); + } + @Override public AddressResolverGroup getOrCreateDefaultResolver() { return super.getOrCreateDefaultResolver(); diff --git a/reactor-netty-http/src/main/java/reactor/netty/http/client/Http2ConnectionProvider.java b/reactor-netty-http/src/main/java/reactor/netty/http/client/Http2ConnectionProvider.java index cf76c73902..f8cb3eb3b4 100644 --- a/reactor-netty-http/src/main/java/reactor/netty/http/client/Http2ConnectionProvider.java +++ b/reactor-netty-http/src/main/java/reactor/netty/http/client/Http2ConnectionProvider.java @@ -16,13 +16,12 @@ package reactor.netty.http.client; import io.netty.channel.Channel; -import io.netty.channel.ChannelPipeline; +import io.netty.channel.ChannelHandlerContext; import io.netty.handler.codec.http2.Http2Connection; import io.netty.handler.codec.http2.Http2FrameCodec; import io.netty.handler.codec.http2.Http2LocalFlowController; import io.netty.handler.codec.http2.Http2StreamChannel; import io.netty.handler.ssl.ApplicationProtocolNames; -import io.netty.handler.ssl.SslHandler; import io.netty.resolver.AddressResolverGroup; import io.netty.util.AttributeKey; import io.netty.util.concurrent.Future; @@ -37,7 +36,6 @@ import reactor.core.publisher.Operators; import reactor.netty.Connection; import reactor.netty.ConnectionObserver; -import reactor.netty.NettyPipeline; import reactor.netty.channel.ChannelMetricsRecorder; import reactor.netty.channel.ChannelOperations; import reactor.netty.resources.ConnectionProvider; @@ -76,6 +74,9 @@ final class Http2ConnectionProvider extends PooledConnectionProvider Http2ConnectionProvider(ConnectionProvider parent) { super(initConfiguration(parent)); this.parent = parent; + if (parent instanceof PooledConnectionProvider) { + ((PooledConnectionProvider) parent).onDispose(disposeLater()); + } } static Builder initConfiguration(ConnectionProvider parent) { @@ -332,11 +333,12 @@ public void onUncaughtException(Connection connection, Throwable error) { @Override public void operationComplete(Future future) { Channel channel = pooledRef.poolable().channel(); - Http2FrameCodec frameCodec = channel.pipeline().get(Http2FrameCodec.class); + ChannelHandlerContext frameCodec = ((Http2Pool.Http2PooledRef) pooledRef).slot.http2FrameCodecCtx(); if (future.isSuccess()) { Http2StreamChannel ch = future.getNow(); - if (!channel.isActive() || frameCodec == null || !frameCodec.connection().local().canOpenStream()) { + if (!channel.isActive() || frameCodec == null || + !((Http2FrameCodec) frameCodec.handler()).connection().local().canOpenStream()) { invalidate(this); if (!retried) { if (log.isDebugEnabled()) { @@ -358,8 +360,8 @@ public void operationComplete(Future future) { sink.success(ops); } - Http2Connection.Endpoint localEndpoint = frameCodec.connection().local(); if (log.isDebugEnabled()) { + Http2Connection.Endpoint localEndpoint = ((Http2FrameCodec) frameCodec.handler()).connection().local(); logStreamsState(ch, localEndpoint, "Stream opened"); } } @@ -372,8 +374,8 @@ public void operationComplete(Future future) { boolean isH2cUpgrade() { Channel channel = pooledRef.poolable().channel(); - if (channel.pipeline().get(NettyPipeline.H2CUpgradeHandler) != null && - channel.pipeline().get(NettyPipeline.H2MultiplexHandler) == null) { + if (((Http2Pool.Http2PooledRef) pooledRef).slot.h2cUpgradeHandlerCtx() != null && + ((Http2Pool.Http2PooledRef) pooledRef).slot.http2MultiplexHandlerCtx() == null) { ChannelOperations ops = ChannelOperations.get(channel); if (ops != null) { sink.success(ops); @@ -385,11 +387,9 @@ boolean isH2cUpgrade() { boolean notHttp2() { Channel channel = pooledRef.poolable().channel(); - ChannelPipeline pipeline = channel.pipeline(); - SslHandler handler = pipeline.get(SslHandler.class); - if (handler != null) { - String protocol = handler.applicationProtocol() != null ? handler.applicationProtocol() : ApplicationProtocolNames.HTTP_1_1; - if (ApplicationProtocolNames.HTTP_1_1.equals(protocol)) { + String applicationProtocol = ((Http2Pool.Http2PooledRef) pooledRef).slot.applicationProtocol; + if (applicationProtocol != null) { + if (ApplicationProtocolNames.HTTP_1_1.equals(applicationProtocol)) { // No information for the negotiated application-level protocol, // or it is HTTP/1.1, continue as an HTTP/1.1 request // and remove the connection from this pool. @@ -400,15 +400,15 @@ boolean notHttp2() { return true; } } - else if (!ApplicationProtocolNames.HTTP_2.equals(handler.applicationProtocol())) { + else if (!ApplicationProtocolNames.HTTP_2.equals(applicationProtocol)) { channel.attr(OWNER).set(null); invalidate(this); - sink.error(new IOException("Unknown protocol [" + protocol + "].")); + sink.error(new IOException("Unknown protocol [" + applicationProtocol + "].")); return true; } } - else if (pipeline.get(NettyPipeline.H2CUpgradeHandler) == null && - pipeline.get(NettyPipeline.H2MultiplexHandler) == null) { + else if (((Http2Pool.Http2PooledRef) pooledRef).slot.h2cUpgradeHandlerCtx() == null && + ((Http2Pool.Http2PooledRef) pooledRef).slot.http2MultiplexHandlerCtx() == null) { // It is not H2. There are no handlers for H2C upgrade/H2C prior-knowledge, // continue as an HTTP/1.1 request and remove the connection from this pool. ChannelOperations ops = ChannelOperations.get(channel); diff --git a/reactor-netty-http/src/main/java/reactor/netty/http/client/Http2Pool.java b/reactor-netty-http/src/main/java/reactor/netty/http/client/Http2Pool.java index a86dd87f3b..8162e5df18 100644 --- a/reactor-netty-http/src/main/java/reactor/netty/http/client/Http2Pool.java +++ b/reactor-netty-http/src/main/java/reactor/netty/http/client/Http2Pool.java @@ -24,9 +24,14 @@ import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicIntegerFieldUpdater; import java.util.concurrent.atomic.AtomicReferenceFieldUpdater; +import java.util.function.Function; +import io.netty.channel.ChannelHandlerContext; import io.netty.handler.codec.http2.Http2FrameCodec; import io.netty.handler.codec.http2.Http2MultiplexHandler; +import io.netty.handler.ssl.ApplicationProtocolNames; +import io.netty.handler.ssl.SslHandler; +import org.reactivestreams.Publisher; import org.reactivestreams.Subscription; import reactor.core.CoreSubscriber; import reactor.core.Disposable; @@ -36,8 +41,8 @@ import reactor.core.publisher.Operators; import reactor.core.scheduler.Schedulers; import reactor.netty.Connection; -import reactor.netty.ConnectionObserver; -import reactor.netty.channel.ChannelOperations; +import reactor.netty.FutureMono; +import reactor.netty.NettyPipeline; import reactor.netty.internal.shaded.reactor.pool.InstrumentedPool; import reactor.netty.internal.shaded.reactor.pool.PoolAcquirePendingLimitException; import reactor.netty.internal.shaded.reactor.pool.PoolAcquireTimeoutException; @@ -59,7 +64,6 @@ *

    *
  • The connection is closed.
    • *
  • The connection has reached its life time and there are no active streams.
    • - *
  • The connection has no active streams.
    • *
  • When the client is in one of the two modes: 1) H2 and HTTP/1.1 or 2) H2C and HTTP/1.1, * and the negotiated protocol is HTTP/1.1.
    • *
@@ -75,9 +79,9 @@ *

* This pool always invalidate the {@link PooledRef}, there is no release functionality. *

    - *
  • {@link PoolMetrics#acquiredSize()} and {@link PoolMetrics#allocatedSize()} always return the number of - * the active streams from all connections currently in the pool.
    • - *
  • {@link PoolMetrics#idleSize()} always returns {@code 0}.
    • + *
  • {@link PoolMetrics#acquiredSize()}, {@link PoolMetrics#allocatedSize()} and {@link PoolMetrics#idleSize()} + * always return the number of the cached connections.
    • + *
  • {@link Http2Pool#activeStreams()} always return the active streams from all connections currently in the pool.
    • *
*

* Configurations that are not applicable @@ -114,6 +118,10 @@ final class Http2Pool implements InstrumentedPool, InstrumentedPool. static final AtomicReferenceFieldUpdater CONNECTIONS = AtomicReferenceFieldUpdater.newUpdater(Http2Pool.class, ConcurrentLinkedQueue.class, "connections"); + volatile int idleSize; + private static final AtomicIntegerFieldUpdater IDLE_SIZE = + AtomicIntegerFieldUpdater.newUpdater(Http2Pool.class, "idleSize"); + /** * Pending borrowers queue. Never invoke directly the poll/add/remove methods and instead of that, * use addPending/pollPending/removePending methods which take care of maintaining the pending queue size. @@ -171,12 +179,12 @@ public Mono> acquire(Duration timeout) { @Override public int acquiredSize() { - return acquired; + return allocatedSize() - idleSize(); } @Override public int allocatedSize() { - return acquired; + return poolConfig.allocationStrategy().permitGranted(); } @Override @@ -197,10 +205,19 @@ public Mono disposeLater() { p.fail(new PoolShutdownException()); } - // the last stream on that connection will release the connection to the parent pool - // the structure should not contain connections with 0 streams as the last stream on that connection - // always removes the connection from this pool - CONNECTIONS.getAndSet(this, null); + @SuppressWarnings("unchecked") + ConcurrentLinkedQueue slots = CONNECTIONS.getAndSet(this, null); + if (slots != null) { + Mono closeMonos = Mono.empty(); + while (!slots.isEmpty()) { + Slot slot = pollSlot(slots); + if (slot != null) { + slot.invalidate(); + closeMonos = closeMonos.and(DEFAULT_DESTROY_HANDLER.apply(slot.connection)); + } + } + return closeMonos; + } } return Mono.empty(); }); @@ -218,7 +235,7 @@ public int getMaxPendingAcquireSize() { @Override public int idleSize() { - return 0; + return idleSize; } @Override @@ -253,6 +270,10 @@ public Mono warmup() { return Mono.just(0); } + int activeStreams() { + return acquired; + } + void cancelAcquire(Borrower borrower) { if (!isDisposed()) { ConcurrentLinkedDeque q = pending; @@ -260,15 +281,32 @@ void cancelAcquire(Borrower borrower) { } } + @SuppressWarnings("FutureReturnValueIgnored") Mono destroyPoolable(Http2PooledRef ref) { + assert ref.slot.connection.channel().eventLoop().inEventLoop(); Mono mono = Mono.empty(); try { + // By default, check the connection for removal on acquire and invalidate (only if there are no active streams) if (ref.slot.decrementConcurrencyAndGet() == 0) { - ref.slot.invalidate(); - Connection connection = ref.poolable(); - Http2FrameCodec frameCodec = connection.channel().pipeline().get(Http2FrameCodec.class); - if (frameCodec != null) { - releaseConnection(connection); + // not HTTP/2 request + if (ref.slot.http2FrameCodecCtx() == null) { + ref.slot.invalidate(); + removeSlot(ref.slot); + } + // If there is eviction in background, the background process will remove this connection + else if (poolConfig.evictInBackgroundInterval().isZero()) { + // not active + if (!ref.poolable().channel().isActive()) { + ref.slot.invalidate(); + removeSlot(ref.slot); + } + // max life reached + else if (maxLifeReached(ref.slot)) { + //"FutureReturnValueIgnored" this is deliberate + ref.slot.connection.channel().close(); + ref.slot.invalidate(); + removeSlot(ref.slot); + } } } } @@ -315,27 +353,22 @@ void drainLoop() { if (slot != null) { Borrower borrower = pollPending(borrowers, true); if (borrower == null) { - resources.offer(slot); + offerSlot(resources, slot); continue; } if (isDisposed()) { borrower.fail(new PoolShutdownException()); return; } - if (slot.incrementConcurrencyAndGet() > 1) { - borrower.stopPendingCountdown(); - if (log.isDebugEnabled()) { - log.debug(format(slot.connection.channel(), "Channel activated")); - } - ACQUIRED.incrementAndGet(this); - // we are ready here, the connection can be used for opening another stream - slot.deactivate(); - poolConfig.acquisitionScheduler().schedule(() -> borrower.deliver(new Http2PooledRef(slot))); - } - else { - addPending(borrowers, borrower, true); - continue; + borrower.stopPendingCountdown(); + if (log.isDebugEnabled()) { + log.debug(format(slot.connection.channel(), "Channel activated")); } + ACQUIRED.incrementAndGet(this); + slot.connection.channel().eventLoop().execute(() -> { + borrower.deliver(new Http2PooledRef(slot)); + drain(); + }); } else { int permits = poolConfig.allocationStrategy().getPermits(1); @@ -372,8 +405,6 @@ void drainLoop() { log.debug(format(newInstance.channel(), "Channel activated")); } ACQUIRED.incrementAndGet(this); - newSlot.incrementConcurrencyAndGet(); - newSlot.deactivate(); borrower.deliver(new Http2PooledRef(newSlot)); } else if (sig.isOnError()) { @@ -398,15 +429,16 @@ else if (sig.isOnError()) { } @Nullable + @SuppressWarnings("FutureReturnValueIgnored") Slot findConnection(ConcurrentLinkedQueue resources) { - int resourcesCount = resources.size(); + int resourcesCount = idleSize; while (resourcesCount > 0) { // There are connections in the queue resourcesCount--; // get the connection - Slot slot = resources.poll(); + Slot slot = pollSlot(resources); if (slot == null) { continue; } @@ -418,38 +450,40 @@ Slot findConnection(ConcurrentLinkedQueue resources) { log.debug(format(slot.connection.channel(), "Channel is closed, {} active streams"), slot.concurrency()); } - resources.offer(slot); + offerSlot(resources, slot); } else { if (log.isDebugEnabled()) { log.debug(format(slot.connection.channel(), "Channel is closed, remove from pool")); } - resources.remove(slot); + slot.invalidate(); } continue; } // check that the connection's max lifetime has not been reached - if (maxLifeTime != -1 && slot.lifeTime() >= maxLifeTime) { + if (maxLifeReached(slot)) { if (slot.concurrency() > 0) { if (log.isDebugEnabled()) { log.debug(format(slot.connection.channel(), "Max life time is reached, {} active streams"), slot.concurrency()); } - resources.offer(slot); + offerSlot(resources, slot); } else { if (log.isDebugEnabled()) { log.debug(format(slot.connection.channel(), "Max life time is reached, remove from pool")); } - resources.remove(slot); + //"FutureReturnValueIgnored" this is deliberate + slot.connection.channel().close(); + slot.invalidate(); } continue; } // check that the connection's max active streams has not been reached if (!slot.canOpenStream()) { - resources.offer(slot); + offerSlot(resources, slot); if (log.isDebugEnabled()) { log.debug(format(slot.connection.channel(), "Max active streams is reached")); } @@ -462,6 +496,10 @@ Slot findConnection(ConcurrentLinkedQueue resources) { return null; } + boolean maxLifeReached(Slot slot) { + return maxLifeTime != -1 && slot.lifeTime() >= maxLifeTime; + } + void pendingAcquireLimitReached(Borrower borrower, int maxPending) { if (maxPending == 0) { borrower.fail(new PoolAcquirePendingLimitException(0, @@ -530,33 +568,40 @@ int addPending(ConcurrentLinkedDeque borrowers, Borrower borrower, boo return PENDING_SIZE.incrementAndGet(this); } - static boolean offerSlot(Slot slot) { - @SuppressWarnings("unchecked") - ConcurrentLinkedQueue q = CONNECTIONS.get(slot.pool); - return q != null && q.offer(slot); + void offerSlot(@Nullable ConcurrentLinkedQueue slots, Slot slot) { + if (slots != null && slots.offer(slot)) { + IDLE_SIZE.incrementAndGet(this); + } } - static void releaseConnection(Connection connection) { - ChannelOperations ops = connection.as(ChannelOperations.class); - if (ops != null) { - ops.listener().onStateChange(ops, ConnectionObserver.State.DISCONNECTING); - } - else if (connection instanceof ConnectionObserver) { - ((ConnectionObserver) connection).onStateChange(connection, ConnectionObserver.State.DISCONNECTING); + @Nullable + Slot pollSlot(@Nullable ConcurrentLinkedQueue slots) { + if (slots == null) { + return null; } - else { - connection.dispose(); + Slot slot = slots.poll(); + if (slot != null) { + IDLE_SIZE.decrementAndGet(this); } + return slot; } - static void removeSlot(Slot slot) { + void removeSlot(Slot slot) { @SuppressWarnings("unchecked") ConcurrentLinkedQueue q = CONNECTIONS.get(slot.pool); - if (q != null) { - q.remove(slot); + if (q != null && q.remove(slot)) { + IDLE_SIZE.decrementAndGet(this); } } + static final Function> DEFAULT_DESTROY_HANDLER = + connection -> { + if (!connection.channel().isActive()) { + return Mono.empty(); + } + return FutureMono.from(connection.channel().close()); + }; + static final class Borrower extends AtomicBoolean implements Scannable, Subscription, Runnable { static final Disposable TIMEOUT_DISPOSED = Disposables.disposed(); @@ -589,7 +634,10 @@ Context currentContext() { @Override public void request(long n) { if (Operators.validate(n)) { - if (!acquireTimeout.isZero()) { + // Cannot rely on idleSize because there might be idle connections but not suitable for use + int permits = pool.poolConfig.allocationStrategy().estimatePermitCount(); + int pending = pool.pendingSize; + if (!acquireTimeout.isZero() && permits <= pending) { timeoutTask = Schedulers.parallel().schedule(this, acquireTimeout.toMillis(), TimeUnit.MILLISECONDS); } pool.doAcquire(this); @@ -627,7 +675,9 @@ public String toString() { } void deliver(Http2PooledRef poolSlot) { - stopPendingCountdown(); + assert poolSlot.slot.connection.channel().eventLoop().inEventLoop(); + poolSlot.slot.incrementConcurrencyAndGet(); + poolSlot.slot.deactivate(); if (get()) { //CANCELLED or timeout reached poolSlot.invalidate().subscribe(aVoid -> {}, e -> Operators.onErrorDropped(e, Context.empty())); @@ -737,7 +787,7 @@ public String toString() { } } - static final class Slot { + static final class Slot extends AtomicBoolean { volatile int concurrency; static final AtomicIntegerFieldUpdater CONCURRENCY = @@ -746,18 +796,30 @@ static final class Slot { final Connection connection; final long creationTimestamp; final Http2Pool pool; + final String applicationProtocol; + + volatile ChannelHandlerContext http2FrameCodecCtx; + volatile ChannelHandlerContext http2MultiplexHandlerCtx; + volatile ChannelHandlerContext h2cUpgradeHandlerCtx; Slot(Http2Pool pool, Connection connection) { this.connection = connection; this.creationTimestamp = pool.clock.millis(); this.pool = pool; + SslHandler handler = connection.channel().pipeline().get(SslHandler.class); + if (handler != null) { + this.applicationProtocol = handler.applicationProtocol() != null ? + handler.applicationProtocol() : ApplicationProtocolNames.HTTP_1_1; + } + else { + this.applicationProtocol = null; + } } boolean canOpenStream() { - Http2FrameCodec frameCodec = connection.channel().pipeline().get(Http2FrameCodec.class); - Http2MultiplexHandler multiplexHandler = connection.channel().pipeline().get(Http2MultiplexHandler.class); - if (frameCodec != null && multiplexHandler != null) { - int maxActiveStreams = frameCodec.connection().local().maxActiveStreams(); + ChannelHandlerContext frameCodec = http2FrameCodecCtx(); + if (frameCodec != null && http2MultiplexHandlerCtx() != null) { + int maxActiveStreams = ((Http2FrameCodec) frameCodec.handler()).connection().local().maxActiveStreams(); int concurrency = this.concurrency; return concurrency < maxActiveStreams; } @@ -772,23 +834,59 @@ void deactivate() { if (log.isDebugEnabled()) { log.debug(format(connection.channel(), "Channel deactivated")); } - offerSlot(this); + @SuppressWarnings("unchecked") + ConcurrentLinkedQueue slots = CONNECTIONS.get(pool); + pool.offerSlot(slots, this); } int decrementConcurrencyAndGet() { return CONCURRENCY.decrementAndGet(this); } - int incrementConcurrencyAndGet() { - return CONCURRENCY.incrementAndGet(this); + @Nullable + ChannelHandlerContext http2FrameCodecCtx() { + ChannelHandlerContext ctx = http2FrameCodecCtx; + if (ctx != null && !ctx.isRemoved()) { + return ctx; + } + ctx = connection.channel().pipeline().context(Http2FrameCodec.class); + http2FrameCodecCtx = ctx; + return ctx; + } + + @Nullable + ChannelHandlerContext http2MultiplexHandlerCtx() { + ChannelHandlerContext ctx = http2MultiplexHandlerCtx; + if (ctx != null && !ctx.isRemoved()) { + return ctx; + } + ctx = connection.channel().pipeline().context(Http2MultiplexHandler.class); + http2MultiplexHandlerCtx = ctx; + return ctx; + } + + @Nullable + ChannelHandlerContext h2cUpgradeHandlerCtx() { + ChannelHandlerContext ctx = h2cUpgradeHandlerCtx; + if (ctx != null && !ctx.isRemoved()) { + return ctx; + } + ctx = connection.channel().pipeline().context(NettyPipeline.H2CUpgradeHandler); + h2cUpgradeHandlerCtx = ctx; + return ctx; + } + + void incrementConcurrencyAndGet() { + CONCURRENCY.incrementAndGet(this); } void invalidate() { - if (log.isDebugEnabled()) { - log.debug(format(connection.channel(), "Channel removed from pool")); + if (compareAndSet(false, true)) { + if (log.isDebugEnabled()) { + log.debug(format(connection.channel(), "Channel removed from pool")); + } + pool.poolConfig.allocationStrategy().returnPermits(1); } - pool.poolConfig.allocationStrategy().returnPermits(1); - removeSlot(this); } long lifeTime() { diff --git a/reactor-netty-http/src/main/java/reactor/netty/http/client/MicrometerHttp2ConnectionProviderMeterRegistrar.java b/reactor-netty-http/src/main/java/reactor/netty/http/client/MicrometerHttp2ConnectionProviderMeterRegistrar.java index 08bd8ee6af..3543fe1608 100644 --- a/reactor-netty-http/src/main/java/reactor/netty/http/client/MicrometerHttp2ConnectionProviderMeterRegistrar.java +++ b/reactor-netty-http/src/main/java/reactor/netty/http/client/MicrometerHttp2ConnectionProviderMeterRegistrar.java @@ -22,16 +22,21 @@ import java.net.SocketAddress; +import static reactor.netty.Metrics.ACTIVE_CONNECTIONS; import static reactor.netty.Metrics.ACTIVE_STREAMS; import static reactor.netty.Metrics.CONNECTION_PROVIDER_PREFIX; import static reactor.netty.Metrics.ID; +import static reactor.netty.Metrics.IDLE_CONNECTIONS; import static reactor.netty.Metrics.NAME; import static reactor.netty.Metrics.PENDING_STREAMS; import static reactor.netty.Metrics.REGISTRY; import static reactor.netty.Metrics.REMOTE_ADDRESS; final class MicrometerHttp2ConnectionProviderMeterRegistrar { + static final String ACTIVE_CONNECTIONS_DESCRIPTION = + "The number of the connections that have been successfully acquired and are in active use"; static final String ACTIVE_STREAMS_DESCRIPTION = "The number of the active HTTP/2 streams"; + static final String IDLE_CONNECTIONS_DESCRIPTION = "The number of the idle connections"; static final String PENDING_STREAMS_DESCRIPTION = "The number of requests that are waiting for opening HTTP/2 stream"; @@ -45,11 +50,21 @@ void registerMetrics(String poolName, String id, SocketAddress remoteAddress, In String addressAsString = Metrics.formatSocketAddress(remoteAddress); Tags tags = Tags.of(ID, id, REMOTE_ADDRESS, addressAsString, NAME, poolName); - Gauge.builder(CONNECTION_PROVIDER_PREFIX + ACTIVE_STREAMS, metrics, InstrumentedPool.PoolMetrics::acquiredSize) + Gauge.builder(CONNECTION_PROVIDER_PREFIX + ACTIVE_CONNECTIONS, metrics, InstrumentedPool.PoolMetrics::acquiredSize) + .description(ACTIVE_CONNECTIONS_DESCRIPTION) + .tags(tags) + .register(REGISTRY); + + Gauge.builder(CONNECTION_PROVIDER_PREFIX + ACTIVE_STREAMS, metrics, poolMetrics -> ((Http2Pool) poolMetrics).activeStreams()) .description(ACTIVE_STREAMS_DESCRIPTION) .tags(tags) .register(REGISTRY); + Gauge.builder(CONNECTION_PROVIDER_PREFIX + IDLE_CONNECTIONS, metrics, InstrumentedPool.PoolMetrics::idleSize) + .description(IDLE_CONNECTIONS_DESCRIPTION) + .tags(tags) + .register(REGISTRY); + Gauge.builder(CONNECTION_PROVIDER_PREFIX + PENDING_STREAMS, metrics, InstrumentedPool.PoolMetrics::pendingAcquireSize) .description(PENDING_STREAMS_DESCRIPTION) .tags(tags) diff --git a/reactor-netty-http/src/test/java/reactor/netty/http/client/Http2PoolTest.java b/reactor-netty-http/src/test/java/reactor/netty/http/client/Http2PoolTest.java index dc24581f27..ec68ef3669 100644 --- a/reactor-netty-http/src/test/java/reactor/netty/http/client/Http2PoolTest.java +++ b/reactor-netty-http/src/test/java/reactor/netty/http/client/Http2PoolTest.java @@ -24,7 +24,6 @@ import org.junit.jupiter.api.Test; import reactor.core.publisher.Mono; import reactor.netty.Connection; -import reactor.netty.internal.shaded.reactor.pool.InstrumentedPool; import reactor.netty.internal.shaded.reactor.pool.PoolAcquireTimeoutException; import reactor.netty.internal.shaded.reactor.pool.PoolBuilder; import reactor.netty.internal.shaded.reactor.pool.PoolConfig; @@ -38,6 +37,7 @@ import java.util.Random; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicReference; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatExceptionOfType; @@ -53,7 +53,7 @@ void acquireInvalidate() { .idleResourceReuseLruOrder() .maxPendingAcquireUnbounded() .sizeBetween(0, 1); - InstrumentedPool http2Pool = poolBuilder.build(config -> new Http2Pool(config, -1)); + Http2Pool http2Pool = poolBuilder.build(config -> new Http2Pool(config, -1)); try { List> acquired = new ArrayList<>(); @@ -61,21 +61,23 @@ void acquireInvalidate() { http2Pool.acquire().subscribe(acquired::add); http2Pool.acquire().subscribe(acquired::add); + channel.runPendingTasks(); + assertThat(acquired).hasSize(3); - assertThat(http2Pool.metrics().acquiredSize()).isEqualTo(3); + assertThat(http2Pool.activeStreams()).isEqualTo(3); for (PooledRef slot : acquired) { slot.invalidate().block(Duration.ofSeconds(1)); } - assertThat(http2Pool.metrics().acquiredSize()).isEqualTo(0); + assertThat(http2Pool.activeStreams()).isEqualTo(0); for (PooledRef slot : acquired) { // second invalidate() should be ignored and ACQUIRED size should remain the same slot.invalidate().block(Duration.ofSeconds(1)); } - assertThat(http2Pool.metrics().acquiredSize()).isEqualTo(0); + assertThat(http2Pool.activeStreams()).isEqualTo(0); } finally { channel.finishAndReleaseAll(); @@ -92,7 +94,7 @@ void acquireRelease() { .idleResourceReuseLruOrder() .maxPendingAcquireUnbounded() .sizeBetween(0, 1); - InstrumentedPool http2Pool = poolBuilder.build(config -> new Http2Pool(config, -1)); + Http2Pool http2Pool = poolBuilder.build(config -> new Http2Pool(config, -1)); try { List> acquired = new ArrayList<>(); @@ -100,21 +102,23 @@ void acquireRelease() { http2Pool.acquire().subscribe(acquired::add); http2Pool.acquire().subscribe(acquired::add); + channel.runPendingTasks(); + assertThat(acquired).hasSize(3); - assertThat(http2Pool.metrics().acquiredSize()).isEqualTo(3); + assertThat(http2Pool.activeStreams()).isEqualTo(3); for (PooledRef slot : acquired) { slot.release().block(Duration.ofSeconds(1)); } - assertThat(http2Pool.metrics().acquiredSize()).isEqualTo(0); + assertThat(http2Pool.activeStreams()).isEqualTo(0); for (PooledRef slot : acquired) { // second release() should be ignored and ACQUIRED size should remain the same slot.release().block(Duration.ofSeconds(1)); } - assertThat(http2Pool.metrics().acquiredSize()).isEqualTo(0); + assertThat(http2Pool.activeStreams()).isEqualTo(0); } finally { channel.finishAndReleaseAll(); @@ -141,7 +145,7 @@ void evictClosedConnection() throws Exception { PooledRef acquired1 = http2Pool.acquire().block(); assertThat(acquired1).isNotNull(); - assertThat(http2Pool.metrics().acquiredSize()).isEqualTo(1); + assertThat(http2Pool.activeStreams()).isEqualTo(1); assertThat(http2Pool.connections.size()).isEqualTo(1); connection = acquired1.poolable(); @@ -153,18 +157,18 @@ void evictClosedConnection() throws Exception { assertThat(latch.await(1, TimeUnit.SECONDS)).as("latch await").isTrue(); - assertThat(http2Pool.metrics().acquiredSize()).isEqualTo(1); + assertThat(http2Pool.activeStreams()).isEqualTo(1); assertThat(http2Pool.connections.size()).isEqualTo(1); acquired1.invalidate().block(); - assertThat(http2Pool.metrics().acquiredSize()).isEqualTo(0); + assertThat(http2Pool.activeStreams()).isEqualTo(0); assertThat(http2Pool.connections.size()).isEqualTo(0); PooledRef acquired2 = http2Pool.acquire().block(); assertThat(acquired2).isNotNull(); - assertThat(http2Pool.metrics().acquiredSize()).isEqualTo(1); + assertThat(http2Pool.activeStreams()).isEqualTo(1); assertThat(http2Pool.connections.size()).isEqualTo(1); connection = acquired2.poolable(); @@ -174,8 +178,8 @@ void evictClosedConnection() throws Exception { acquired2.invalidate().block(); - assertThat(http2Pool.metrics().acquiredSize()).isEqualTo(0); - assertThat(http2Pool.connections.size()).isEqualTo(0); + assertThat(http2Pool.activeStreams()).isEqualTo(0); + assertThat(http2Pool.connections.size()).isEqualTo(1); } finally { if (connection != null) { @@ -186,12 +190,22 @@ void evictClosedConnection() throws Exception { } @Test - void evictClosedConnectionMaxConnectionsNotReached() throws Exception { + void evictClosedConnectionMaxConnectionsNotReached_1() throws Exception { + evictClosedConnectionMaxConnectionsNotReached(false); + } + + @Test + void evictClosedConnectionMaxConnectionsNotReached_2() throws Exception { + evictClosedConnectionMaxConnectionsNotReached(true); + } + + private void evictClosedConnectionMaxConnectionsNotReached(boolean closeSecond) throws Exception { PoolBuilder> poolBuilder = PoolBuilder.from(Mono.fromSupplier(() -> { Channel channel = new EmbeddedChannel( new TestChannelId(), - Http2FrameCodecBuilder.forClient().build()); + Http2FrameCodecBuilder.forClient().build(), + new Http2MultiplexHandler(new ChannelHandlerAdapter() {})); return Connection.from(channel); })) .idleResourceReuseLruOrder() @@ -204,7 +218,7 @@ void evictClosedConnectionMaxConnectionsNotReached() throws Exception { PooledRef acquired1 = http2Pool.acquire().block(); assertThat(acquired1).isNotNull(); - assertThat(http2Pool.metrics().acquiredSize()).isEqualTo(1); + assertThat(http2Pool.activeStreams()).isEqualTo(1); assertThat(http2Pool.connections.size()).isEqualTo(1); connection = acquired1.poolable(); @@ -216,25 +230,48 @@ void evictClosedConnectionMaxConnectionsNotReached() throws Exception { assertThat(latch.await(1, TimeUnit.SECONDS)).as("latch await").isTrue(); - assertThat(http2Pool.metrics().acquiredSize()).isEqualTo(1); + assertThat(http2Pool.activeStreams()).isEqualTo(1); assertThat(http2Pool.connections.size()).isEqualTo(1); PooledRef acquired2 = http2Pool.acquire().block(); - assertThat(acquired2).isNotNull(); - assertThat(http2Pool.metrics().acquiredSize()).isEqualTo(2); - assertThat(http2Pool.connections.size()).isEqualTo(2); + + AtomicReference> acquired3 = new AtomicReference<>(); + http2Pool.acquire().subscribe(acquired3::set); connection = acquired2.poolable(); - ChannelId id2 = connection.channel().id(); + ((EmbeddedChannel) connection.channel()).runPendingTasks(); + + assertThat(http2Pool.activeStreams()).isEqualTo(3); + assertThat(http2Pool.connections.size()).isEqualTo(2); + + if (closeSecond) { + latch = new CountDownLatch(1); + ((EmbeddedChannel) connection.channel()).finishAndReleaseAll(); + connection.onDispose(latch::countDown); + connection.dispose(); + assertThat(latch.await(1, TimeUnit.SECONDS)).as("latch await").isTrue(); + } + + ChannelId id2 = connection.channel().id(); assertThat(id1).isNotEqualTo(id2); acquired1.invalidate().block(); acquired2.invalidate().block(); - assertThat(http2Pool.metrics().acquiredSize()).isEqualTo(0); - assertThat(http2Pool.connections.size()).isEqualTo(0); + assertThat(http2Pool.activeStreams()).isEqualTo(1); + assertThat(http2Pool.connections.size()).isEqualTo(1); + + acquired3.get().invalidate().block(); + + assertThat(http2Pool.activeStreams()).isEqualTo(0); + if (closeSecond) { + assertThat(http2Pool.connections.size()).isEqualTo(0); + } + else { + assertThat(http2Pool.connections.size()).isEqualTo(1); + } } finally { if (connection != null) { @@ -263,7 +300,7 @@ void evictClosedConnectionMaxConnectionsReached() throws Exception { PooledRef acquired1 = http2Pool.acquire().block(); assertThat(acquired1).isNotNull(); - assertThat(http2Pool.metrics().acquiredSize()).isEqualTo(1); + assertThat(http2Pool.activeStreams()).isEqualTo(1); assertThat(http2Pool.connections.size()).isEqualTo(1); connection = acquired1.poolable(); @@ -274,7 +311,7 @@ void evictClosedConnectionMaxConnectionsReached() throws Exception { assertThat(latch.await(1, TimeUnit.SECONDS)).as("latch await").isTrue(); - assertThat(http2Pool.metrics().acquiredSize()).isEqualTo(1); + assertThat(http2Pool.activeStreams()).isEqualTo(1); assertThat(http2Pool.connections.size()).isEqualTo(1); http2Pool.acquire(Duration.ofMillis(10)) @@ -282,12 +319,12 @@ void evictClosedConnectionMaxConnectionsReached() throws Exception { .expectError(PoolAcquireTimeoutException.class) .verify(Duration.ofSeconds(1)); - assertThat(http2Pool.metrics().acquiredSize()).isEqualTo(1); + assertThat(http2Pool.activeStreams()).isEqualTo(1); assertThat(http2Pool.connections.size()).isEqualTo(1); acquired1.invalidate().block(); - assertThat(http2Pool.metrics().acquiredSize()).isEqualTo(0); + assertThat(http2Pool.activeStreams()).isEqualTo(0); assertThat(http2Pool.connections.size()).isEqualTo(0); } finally { @@ -318,7 +355,7 @@ void maxLifeTime() throws Exception { PooledRef acquired1 = http2Pool.acquire().block(); assertThat(acquired1).isNotNull(); - assertThat(http2Pool.metrics().acquiredSize()).isEqualTo(1); + assertThat(http2Pool.activeStreams()).isEqualTo(1); assertThat(http2Pool.connections.size()).isEqualTo(1); connection1 = acquired1.poolable(); @@ -326,18 +363,18 @@ void maxLifeTime() throws Exception { Thread.sleep(10); - assertThat(http2Pool.metrics().acquiredSize()).isEqualTo(1); + assertThat(http2Pool.activeStreams()).isEqualTo(1); assertThat(http2Pool.connections.size()).isEqualTo(1); acquired1.invalidate().block(); - assertThat(http2Pool.metrics().acquiredSize()).isEqualTo(0); + assertThat(http2Pool.activeStreams()).isEqualTo(0); assertThat(http2Pool.connections.size()).isEqualTo(0); PooledRef acquired2 = http2Pool.acquire().block(); assertThat(acquired2).isNotNull(); - assertThat(http2Pool.metrics().acquiredSize()).isEqualTo(1); + assertThat(http2Pool.activeStreams()).isEqualTo(1); assertThat(http2Pool.connections.size()).isEqualTo(1); connection2 = acquired2.poolable(); @@ -347,8 +384,8 @@ void maxLifeTime() throws Exception { acquired2.invalidate().block(); - assertThat(http2Pool.metrics().acquiredSize()).isEqualTo(0); - assertThat(http2Pool.connections.size()).isEqualTo(0); + assertThat(http2Pool.activeStreams()).isEqualTo(0); + assertThat(http2Pool.connections.size()).isEqualTo(1); } finally { if (connection1 != null) { @@ -374,7 +411,7 @@ void maxLifeTimeMaxConnectionsNotReached() throws Exception { .idleResourceReuseLruOrder() .maxPendingAcquireUnbounded() .sizeBetween(0, 2); - Http2Pool http2Pool = poolBuilder.build(config -> new Http2Pool(config, 10)); + Http2Pool http2Pool = poolBuilder.build(config -> new Http2Pool(config, 50)); Connection connection1 = null; Connection connection2 = null; @@ -382,21 +419,21 @@ void maxLifeTimeMaxConnectionsNotReached() throws Exception { PooledRef acquired1 = http2Pool.acquire().block(); assertThat(acquired1).isNotNull(); - assertThat(http2Pool.metrics().acquiredSize()).isEqualTo(1); + assertThat(http2Pool.activeStreams()).isEqualTo(1); assertThat(http2Pool.connections.size()).isEqualTo(1); connection1 = acquired1.poolable(); ChannelId id1 = connection1.channel().id(); - Thread.sleep(10); + Thread.sleep(50); - assertThat(http2Pool.metrics().acquiredSize()).isEqualTo(1); + assertThat(http2Pool.activeStreams()).isEqualTo(1); assertThat(http2Pool.connections.size()).isEqualTo(1); PooledRef acquired2 = http2Pool.acquire().block(); assertThat(acquired2).isNotNull(); - assertThat(http2Pool.metrics().acquiredSize()).isEqualTo(2); + assertThat(http2Pool.activeStreams()).isEqualTo(2); assertThat(http2Pool.connections.size()).isEqualTo(2); connection2 = acquired2.poolable(); @@ -407,8 +444,8 @@ void maxLifeTimeMaxConnectionsNotReached() throws Exception { acquired1.invalidate().block(); acquired2.invalidate().block(); - assertThat(http2Pool.metrics().acquiredSize()).isEqualTo(0); - assertThat(http2Pool.connections.size()).isEqualTo(0); + assertThat(http2Pool.activeStreams()).isEqualTo(0); + assertThat(http2Pool.connections.size()).isEqualTo(1); } finally { if (connection1 != null) { @@ -441,14 +478,14 @@ void maxLifeTimeMaxConnectionsReached() throws Exception { PooledRef acquired1 = http2Pool.acquire().block(); assertThat(acquired1).isNotNull(); - assertThat(http2Pool.metrics().acquiredSize()).isEqualTo(1); + assertThat(http2Pool.activeStreams()).isEqualTo(1); assertThat(http2Pool.connections.size()).isEqualTo(1); connection = acquired1.poolable(); Thread.sleep(10); - assertThat(http2Pool.metrics().acquiredSize()).isEqualTo(1); + assertThat(http2Pool.activeStreams()).isEqualTo(1); assertThat(http2Pool.connections.size()).isEqualTo(1); http2Pool.acquire(Duration.ofMillis(10)) @@ -456,12 +493,12 @@ void maxLifeTimeMaxConnectionsReached() throws Exception { .expectError(PoolAcquireTimeoutException.class) .verify(Duration.ofSeconds(1)); - assertThat(http2Pool.metrics().acquiredSize()).isEqualTo(1); + assertThat(http2Pool.activeStreams()).isEqualTo(1); assertThat(http2Pool.connections.size()).isEqualTo(1); acquired1.invalidate().block(); - assertThat(http2Pool.metrics().acquiredSize()).isEqualTo(0); + assertThat(http2Pool.activeStreams()).isEqualTo(0); assertThat(http2Pool.connections.size()).isEqualTo(0); } finally { @@ -488,24 +525,25 @@ void nonHttp2ConnectionEmittedOnce() { .idleResourceReuseLruOrder() .maxPendingAcquireUnbounded() .sizeBetween(0, 1); - InstrumentedPool http2Pool = poolBuilder.build(config -> new Http2Pool(config, -1)); + Http2Pool http2Pool = poolBuilder.build(config -> new Http2Pool(config, -1)); try { PooledRef acquired = http2Pool.acquire().block(Duration.ofSeconds(1)); assertThat(acquired).isNotNull(); - assertThat(http2Pool.metrics().acquiredSize()).isEqualTo(1); + assertThat(http2Pool.activeStreams()).isEqualTo(1); http2Pool.acquire(Duration.ofMillis(10)) .as(StepVerifier::create) .expectError(PoolAcquireTimeoutException.class) .verify(Duration.ofSeconds(1)); - assertThat(http2Pool.metrics().acquiredSize()).isEqualTo(1); + assertThat(http2Pool.activeStreams()).isEqualTo(1); acquired.invalidate().block(Duration.ofSeconds(1)); - assertThat(http2Pool.metrics().acquiredSize()).isEqualTo(0); + assertThat(http2Pool.activeStreams()).isEqualTo(0); + assertThat(http2Pool.connections.size()).isEqualTo(0); } finally { channel.finishAndReleaseAll(); diff --git a/reactor-netty-http/src/test/java/reactor/netty/resources/DefaultPooledConnectionProviderTest.java b/reactor-netty-http/src/test/java/reactor/netty/resources/DefaultPooledConnectionProviderTest.java index 6ee8a43053..32453a4c24 100644 --- a/reactor-netty-http/src/test/java/reactor/netty/resources/DefaultPooledConnectionProviderTest.java +++ b/reactor-netty-http/src/test/java/reactor/netty/resources/DefaultPooledConnectionProviderTest.java @@ -15,6 +15,10 @@ */ package reactor.netty.resources; +import io.micrometer.core.instrument.Gauge; +import io.micrometer.core.instrument.MeterRegistry; +import io.micrometer.core.instrument.Metrics; +import io.micrometer.core.instrument.simple.SimpleMeterRegistry; import io.netty.channel.Channel; import io.netty.channel.ChannelHandlerContext; import io.netty.channel.ChannelOutboundHandlerAdapter; @@ -28,7 +32,9 @@ import io.netty.handler.ssl.util.InsecureTrustManagerFactory; import io.netty.handler.ssl.util.SelfSignedCertificate; import org.apache.commons.lang3.StringUtils; +import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; @@ -62,18 +68,41 @@ import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicReference; +import java.util.function.Function; import static org.assertj.core.api.Assertions.assertThat; +import static reactor.netty.Metrics.ACTIVE_CONNECTIONS; +import static reactor.netty.Metrics.CONNECTION_PROVIDER_PREFIX; +import static reactor.netty.Metrics.IDLE_CONNECTIONS; +import static reactor.netty.Metrics.NAME; +import static reactor.netty.Metrics.REMOTE_ADDRESS; +import static reactor.netty.Metrics.TOTAL_CONNECTIONS; +import static reactor.netty.http.client.HttpClientState.STREAM_CONFIGURED; class DefaultPooledConnectionProviderTest extends BaseHttpTest { static SelfSignedCertificate ssc; + private MeterRegistry registry; + @BeforeAll static void createSelfSignedCertificate() throws CertificateException { ssc = new SelfSignedCertificate(); } + @BeforeEach + void setUp() { + registry = new SimpleMeterRegistry(); + Metrics.addRegistry(registry); + } + + @AfterEach + void tearDown() { + Metrics.removeRegistry(registry); + registry.clear(); + registry.close(); + } + @Test void testIssue903() { Http11SslContextSpec serverCtx = Http11SslContextSpec.forServer(ssc.key(), ssc.cert()); @@ -290,7 +319,7 @@ public void connect(ChannelHandlerContext ctx, SocketAddress remoteAddress, } @Test - void testConnectionReturnedToParentPoolWhenNoActiveStreams() throws Exception { + void testConnectionIdleWhenNoActiveStreams() throws Exception { Http2SslContextSpec serverCtx = Http2SslContextSpec.forServer(ssc.certificate(), ssc.privateKey()); Http2SslContextSpec clientCtx = Http2SslContextSpec.forClient() @@ -307,19 +336,31 @@ void testConnectionReturnedToParentPoolWhenNoActiveStreams() throws Exception { int requestsNum = 10; CountDownLatch latch = new CountDownLatch(1); DefaultPooledConnectionProvider provider = - (DefaultPooledConnectionProvider) ConnectionProvider.create("testConnectionReturnedToParentPoolWhenNoActiveStreams", 5); + (DefaultPooledConnectionProvider) ConnectionProvider.create("testConnectionIdleWhenNoActiveStreams", 5); AtomicInteger counter = new AtomicInteger(); + AtomicReference serverAddress = new AtomicReference<>(); HttpClient client = createClient(provider, disposableServer.port()) - .wiretap(false) + .wiretap(false) .protocol(HttpProtocol.H2) .secure(spec -> spec.sslContext(clientCtx)) + .metrics(true, Function.identity()) + .doAfterRequest((req, conn) -> serverAddress.set(conn.channel().remoteAddress())) .observe((conn, state) -> { - if (state == ConnectionObserver.State.CONNECTED) { + if (state == STREAM_CONFIGURED) { counter.incrementAndGet(); - } - if (state == ConnectionObserver.State.RELEASED && counter.decrementAndGet() == 0) { - latch.countDown(); + conn.onTerminate() + .subscribe(null, + t -> conn.channel().eventLoop().execute(() -> { + if (counter.decrementAndGet() == 0) { + latch.countDown(); + } + }), + () -> conn.channel().eventLoop().execute(() -> { + if (counter.decrementAndGet() == 0) { + latch.countDown(); + } + })); } }); @@ -328,7 +369,7 @@ void testConnectionReturnedToParentPoolWhenNoActiveStreams() throws Exception { .flatMap(i -> client.post() .uri("/") - .send(ByteBufMono.fromString(Mono.just("testConnectionReturnedToParentPoolWhenNoActiveStreams"))) + .send(ByteBufMono.fromString(Mono.just("testConnectionIdleWhenNoActiveStreams"))) .responseContent() .aggregate() .asString()) @@ -336,14 +377,16 @@ void testConnectionReturnedToParentPoolWhenNoActiveStreams() throws Exception { assertThat(latch.await(5, TimeUnit.SECONDS)).isTrue(); - assertThat(provider.channelPools).hasSize(1); + InetSocketAddress sa = (InetSocketAddress) serverAddress.get(); + String address = sa.getHostString() + ":" + sa.getPort(); - @SuppressWarnings({"unchecked", "rawtypes"}) - InstrumentedPool channelPool = - provider.channelPools.values().toArray(new InstrumentedPool[0])[0]; - InstrumentedPool.PoolMetrics metrics = channelPool.metrics(); - assertThat(metrics.acquiredSize()).isEqualTo(0); - assertThat(metrics.allocatedSize()).isEqualTo(metrics.idleSize()); + assertThat(getGaugeValue(CONNECTION_PROVIDER_PREFIX + ACTIVE_CONNECTIONS, + REMOTE_ADDRESS, address, NAME, "http2.testConnectionIdleWhenNoActiveStreams")).isEqualTo(0); + double idleConn = getGaugeValue(CONNECTION_PROVIDER_PREFIX + IDLE_CONNECTIONS, + REMOTE_ADDRESS, address, NAME, "http2.testConnectionIdleWhenNoActiveStreams"); + double totalConn = getGaugeValue(CONNECTION_PROVIDER_PREFIX + TOTAL_CONNECTIONS, + REMOTE_ADDRESS, address, NAME, "testConnectionIdleWhenNoActiveStreams"); + assertThat(totalConn).isEqualTo(idleConn); } finally { provider.disposeLater() @@ -442,21 +485,33 @@ private void doTestIssue1982(HttpProtocol[] serverProtocols, HttpProtocol[] clie .bindNow(); DefaultPooledConnectionProvider provider = - (DefaultPooledConnectionProvider) ConnectionProvider.create("", 5); + (DefaultPooledConnectionProvider) ConnectionProvider.create("doTestIssue1982", 5); CountDownLatch latch = new CountDownLatch(1); AtomicInteger counter = new AtomicInteger(); + AtomicReference serverAddress = new AtomicReference<>(); HttpClient mainClient = clientCtx != null ? HttpClient.create(provider).port(disposableServer.port()).secure(sslContextSpec -> sslContextSpec.sslContext(clientCtx)) : HttpClient.create(provider).port(disposableServer.port()); HttpClient client = mainClient.protocol(clientProtocols) + .metrics(true, Function.identity()) + .doAfterRequest((req, conn) -> serverAddress.set(conn.channel().remoteAddress())) .observe((conn, state) -> { - if (state == ConnectionObserver.State.CONNECTED) { + if (state == STREAM_CONFIGURED) { counter.incrementAndGet(); - } - if (state == ConnectionObserver.State.RELEASED && counter.decrementAndGet() == 0) { - latch.countDown(); + conn.onTerminate() + .subscribe(null, + t -> conn.channel().eventLoop().execute(() -> { + if (counter.decrementAndGet() == 0) { + latch.countDown(); + } + }), + () -> conn.channel().eventLoop().execute(() -> { + if (counter.decrementAndGet() == 0) { + latch.countDown(); + } + })); } }); try { @@ -471,12 +526,16 @@ private void doTestIssue1982(HttpProtocol[] serverProtocols, HttpProtocol[] clie assertThat(latch.await(5, TimeUnit.SECONDS)).isTrue(); - @SuppressWarnings({"unchecked", "rawtypes"}) - InstrumentedPool channelPool = - provider.channelPools.values().toArray(new InstrumentedPool[0])[0]; - InstrumentedPool.PoolMetrics metrics = channelPool.metrics(); - assertThat(metrics.acquiredSize()).isEqualTo(0); - assertThat(metrics.allocatedSize()).isEqualTo(metrics.idleSize()); + InetSocketAddress sa = (InetSocketAddress) serverAddress.get(); + String address = sa.getHostString() + ":" + sa.getPort(); + + assertThat(getGaugeValue(CONNECTION_PROVIDER_PREFIX + ACTIVE_CONNECTIONS, + REMOTE_ADDRESS, address, NAME, "http2.doTestIssue1982")).isEqualTo(0); + double idleConn = getGaugeValue(CONNECTION_PROVIDER_PREFIX + IDLE_CONNECTIONS, + REMOTE_ADDRESS, address, NAME, "http2.doTestIssue1982"); + double totalConn = getGaugeValue(CONNECTION_PROVIDER_PREFIX + TOTAL_CONNECTIONS, + REMOTE_ADDRESS, address, NAME, "doTestIssue1982"); + assertThat(totalConn).isEqualTo(idleConn); } finally { provider.disposeLater() @@ -512,4 +571,13 @@ public boolean trySuccess(Void result) { return r; } } + + private double getGaugeValue(String gaugeName, String... tags) { + Gauge gauge = registry.find(gaugeName).tags(tags).gauge(); + double result = -1; + if (gauge != null) { + result = gauge.value(); + } + return result; + } } diff --git a/reactor-netty-http/src/test/java/reactor/netty/resources/PooledConnectionProviderDefaultMetricsTest.java b/reactor-netty-http/src/test/java/reactor/netty/resources/PooledConnectionProviderDefaultMetricsTest.java index ac752e082f..a466ff3982 100644 --- a/reactor-netty-http/src/test/java/reactor/netty/resources/PooledConnectionProviderDefaultMetricsTest.java +++ b/reactor-netty-http/src/test/java/reactor/netty/resources/PooledConnectionProviderDefaultMetricsTest.java @@ -207,15 +207,17 @@ private void doTest(HttpServer server, HttpClient client, String poolName, boole assertThat(metrics.get()).isTrue(); if (isSecured) { assertThat(getGaugeValue(CONNECTION_PROVIDER_PREFIX + TOTAL_CONNECTIONS, poolName)).isEqualTo(1); - assertThat(getGaugeValue(CONNECTION_PROVIDER_PREFIX + IDLE_CONNECTIONS, poolName)).isEqualTo(1); + assertThat(getGaugeValue(CONNECTION_PROVIDER_PREFIX + ACTIVE_CONNECTIONS, poolName)).isEqualTo(1); + assertThat(getGaugeValue(CONNECTION_PROVIDER_PREFIX + ACTIVE_CONNECTIONS, "http2." + poolName)).isEqualTo(0); + assertThat(getGaugeValue(CONNECTION_PROVIDER_PREFIX + IDLE_CONNECTIONS, "http2." + poolName)).isEqualTo(1); assertThat(getGaugeValue(CONNECTION_PROVIDER_PREFIX + ACTIVE_STREAMS, "http2." + poolName)).isEqualTo(0); assertThat(getGaugeValue(CONNECTION_PROVIDER_PREFIX + PENDING_STREAMS, "http2." + poolName)).isEqualTo(0); } else { assertThat(getGaugeValue(CONNECTION_PROVIDER_PREFIX + TOTAL_CONNECTIONS, poolName)).isEqualTo(0); - assertThat(getGaugeValue(CONNECTION_PROVIDER_PREFIX + IDLE_CONNECTIONS, poolName)).isEqualTo(0); + assertThat(getGaugeValue(CONNECTION_PROVIDER_PREFIX + ACTIVE_CONNECTIONS, poolName)).isEqualTo(0); } - assertThat(getGaugeValue(CONNECTION_PROVIDER_PREFIX + ACTIVE_CONNECTIONS, poolName)).isEqualTo(0); + assertThat(getGaugeValue(CONNECTION_PROVIDER_PREFIX + IDLE_CONNECTIONS, poolName)).isEqualTo(0); assertThat(getGaugeValue(CONNECTION_PROVIDER_PREFIX + PENDING_CONNECTIONS, poolName)).isEqualTo(0); assertThat(getGaugeValue(CONNECTION_PROVIDER_PREFIX + MAX_CONNECTIONS, poolName)).isEqualTo(expectedMaxConnection); assertThat(getGaugeValue(CONNECTION_PROVIDER_PREFIX + MAX_PENDING_CONNECTIONS, poolName)).isEqualTo(expectedMaxPendingAcquire); From d9ae62cbb28b3f7f79ad4dd69626fb7efb795d4f Mon Sep 17 00:00:00 2001 From: Violeta Georgieva Date: Mon, 6 Jun 2022 11:18:25 +0300 Subject: [PATCH 38/42] Add maxIdleTime to Http2Pool (#2257) Related to #2151 and #2262 --- .../resources/PooledConnectionProvider.java | 4 + .../http/client/Http2ConnectionProvider.java | 2 +- .../reactor/netty/http/client/Http2Pool.java | 32 ++++- .../netty/http/client/Http2PoolTest.java | 134 ++++++++++++++++-- 4 files changed, 158 insertions(+), 14 deletions(-) diff --git a/reactor-netty-core/src/main/java/reactor/netty/resources/PooledConnectionProvider.java b/reactor-netty-core/src/main/java/reactor/netty/resources/PooledConnectionProvider.java index 25947313e2..0823c7562a 100644 --- a/reactor-netty-core/src/main/java/reactor/netty/resources/PooledConnectionProvider.java +++ b/reactor-netty-core/src/main/java/reactor/netty/resources/PooledConnectionProvider.java @@ -455,6 +455,10 @@ PoolBuilder> newPoolInternal( return poolBuilder; } + public long maxIdleTime() { + return this.maxIdleTime; + } + public long maxLifeTime() { return maxLifeTime; } diff --git a/reactor-netty-http/src/main/java/reactor/netty/http/client/Http2ConnectionProvider.java b/reactor-netty-http/src/main/java/reactor/netty/http/client/Http2ConnectionProvider.java index f8cb3eb3b4..69f3f5ca7e 100644 --- a/reactor-netty-http/src/main/java/reactor/netty/http/client/Http2ConnectionProvider.java +++ b/reactor-netty-http/src/main/java/reactor/netty/http/client/Http2ConnectionProvider.java @@ -466,7 +466,7 @@ static final class PooledConnectionAllocator { this.remoteAddress = remoteAddress; this.resolver = resolver; this.pool = poolFactory.newPool(connectChannel(), null, DEFAULT_DESTROY_HANDLER, DEFAULT_EVICTION_PREDICATE, - poolConFig -> new Http2Pool(poolConFig, poolFactory.maxLifeTime())); + poolConFig -> new Http2Pool(poolConFig, poolFactory.maxIdleTime(), poolFactory.maxLifeTime())); } Publisher connectChannel() { diff --git a/reactor-netty-http/src/main/java/reactor/netty/http/client/Http2Pool.java b/reactor-netty-http/src/main/java/reactor/netty/http/client/Http2Pool.java index 8162e5df18..5261ba7226 100644 --- a/reactor-netty-http/src/main/java/reactor/netty/http/client/Http2Pool.java +++ b/reactor-netty-http/src/main/java/reactor/netty/http/client/Http2Pool.java @@ -64,6 +64,7 @@ *

    *
  • The connection is closed.
    • *
  • The connection has reached its life time and there are no active streams.
    • + *
  • The connection has reached its idle time and there are no active streams.
    • *
  • When the client is in one of the two modes: 1) H2 and HTTP/1.1 or 2) H2C and HTTP/1.1, * and the negotiated protocol is HTTP/1.1.
    • *
@@ -148,18 +149,20 @@ final class Http2Pool implements InstrumentedPool, InstrumentedPool. AtomicIntegerFieldUpdater.newUpdater(Http2Pool.class, "wip"); final Clock clock; + final long maxIdleTime; final long maxLifeTime; final PoolConfig poolConfig; long lastInteractionTimestamp; - Http2Pool(PoolConfig poolConfig, long maxLifeTime) { + Http2Pool(PoolConfig poolConfig, long maxIdleTime, long maxLifeTime) { if (poolConfig.allocationStrategy().getPermits(0) != 0) { throw new IllegalArgumentException("No support for configuring minimum number of connections"); } this.clock = poolConfig.clock(); this.connections = new ConcurrentLinkedQueue<>(); this.lastInteractionTimestamp = clock.millis(); + this.maxIdleTime = maxIdleTime; this.maxLifeTime = maxLifeTime; this.pending = new ConcurrentLinkedDeque<>(); this.poolConfig = poolConfig; @@ -461,7 +464,18 @@ Slot findConnection(ConcurrentLinkedQueue resources) { continue; } - // check that the connection's max lifetime has not been reached + // check whether the connection's idle time has been reached + if (maxIdleTime != -1 && slot.idleTime() >= maxIdleTime) { + if (log.isDebugEnabled()) { + log.debug(format(slot.connection.channel(), "Idle time is reached, remove from pool")); + } + //"FutureReturnValueIgnored" this is deliberate + slot.connection.channel().close(); + slot.invalidate(); + continue; + } + + // check whether the connection's max lifetime has been reached if (maxLifeReached(slot)) { if (slot.concurrency() > 0) { if (log.isDebugEnabled()) { @@ -798,6 +812,8 @@ static final class Slot extends AtomicBoolean { final Http2Pool pool; final String applicationProtocol; + long idleTimestamp; + volatile ChannelHandlerContext http2FrameCodecCtx; volatile ChannelHandlerContext http2MultiplexHandlerCtx; volatile ChannelHandlerContext h2cUpgradeHandlerCtx; @@ -840,7 +856,17 @@ void deactivate() { } int decrementConcurrencyAndGet() { - return CONCURRENCY.decrementAndGet(this); + int concurrency = CONCURRENCY.decrementAndGet(this); + idleTimestamp = pool.clock.millis(); + return concurrency; + } + + long idleTime() { + if (concurrency() > 0) { + return 0L; + } + long idleTime = idleTimestamp != 0 ? idleTimestamp : creationTimestamp; + return pool.clock.millis() - idleTime; } @Nullable diff --git a/reactor-netty-http/src/test/java/reactor/netty/http/client/Http2PoolTest.java b/reactor-netty-http/src/test/java/reactor/netty/http/client/Http2PoolTest.java index ec68ef3669..bc1ec1a561 100644 --- a/reactor-netty-http/src/test/java/reactor/netty/http/client/Http2PoolTest.java +++ b/reactor-netty-http/src/test/java/reactor/netty/http/client/Http2PoolTest.java @@ -53,7 +53,7 @@ void acquireInvalidate() { .idleResourceReuseLruOrder() .maxPendingAcquireUnbounded() .sizeBetween(0, 1); - Http2Pool http2Pool = poolBuilder.build(config -> new Http2Pool(config, -1)); + Http2Pool http2Pool = poolBuilder.build(config -> new Http2Pool(config, -1, -1)); try { List> acquired = new ArrayList<>(); @@ -94,7 +94,7 @@ void acquireRelease() { .idleResourceReuseLruOrder() .maxPendingAcquireUnbounded() .sizeBetween(0, 1); - Http2Pool http2Pool = poolBuilder.build(config -> new Http2Pool(config, -1)); + Http2Pool http2Pool = poolBuilder.build(config -> new Http2Pool(config, -1, -1)); try { List> acquired = new ArrayList<>(); @@ -138,7 +138,7 @@ void evictClosedConnection() throws Exception { .idleResourceReuseLruOrder() .maxPendingAcquireUnbounded() .sizeBetween(0, 1); - Http2Pool http2Pool = poolBuilder.build(config -> new Http2Pool(config, -1)); + Http2Pool http2Pool = poolBuilder.build(config -> new Http2Pool(config, -1, -1)); Connection connection = null; try { @@ -211,7 +211,7 @@ private void evictClosedConnectionMaxConnectionsNotReached(boolean closeSecond) .idleResourceReuseLruOrder() .maxPendingAcquireUnbounded() .sizeBetween(0, 2); - Http2Pool http2Pool = poolBuilder.build(config -> new Http2Pool(config, -1)); + Http2Pool http2Pool = poolBuilder.build(config -> new Http2Pool(config, -1, -1)); Connection connection = null; try { @@ -293,7 +293,7 @@ void evictClosedConnectionMaxConnectionsReached() throws Exception { .idleResourceReuseLruOrder() .maxPendingAcquireUnbounded() .sizeBetween(0, 1); - Http2Pool http2Pool = poolBuilder.build(config -> new Http2Pool(config, -1)); + Http2Pool http2Pool = poolBuilder.build(config -> new Http2Pool(config, -1, -1)); Connection connection = null; try { @@ -335,6 +335,120 @@ void evictClosedConnectionMaxConnectionsReached() throws Exception { } } + @Test + void maxIdleTime() throws Exception { + PoolBuilder> poolBuilder = + PoolBuilder.from(Mono.fromSupplier(() -> { + Channel channel = new EmbeddedChannel( + new TestChannelId(), + Http2FrameCodecBuilder.forClient().build()); + return Connection.from(channel); + })) + .idleResourceReuseLruOrder() + .maxPendingAcquireUnbounded() + .sizeBetween(0, 1); + Http2Pool http2Pool = poolBuilder.build(config -> new Http2Pool(config, 10, -1)); + + Connection connection1 = null; + Connection connection2 = null; + try { + PooledRef acquired1 = http2Pool.acquire().block(); + + assertThat(acquired1).isNotNull(); + assertThat(http2Pool.activeStreams()).isEqualTo(1); + assertThat(http2Pool.connections.size()).isEqualTo(1); + + connection1 = acquired1.poolable(); + ChannelId id1 = connection1.channel().id(); + + acquired1.invalidate().block(); + + Thread.sleep(15); + + PooledRef acquired2 = http2Pool.acquire().block(); + + assertThat(acquired2).isNotNull(); + assertThat(http2Pool.activeStreams()).isEqualTo(1); + assertThat(http2Pool.connections.size()).isEqualTo(1); + + connection2 = acquired2.poolable(); + ChannelId id2 = connection2.channel().id(); + + assertThat(id1).isNotEqualTo(id2); + + acquired2.invalidate().block(); + + assertThat(http2Pool.activeStreams()).isEqualTo(0); + assertThat(http2Pool.connections.size()).isEqualTo(1); + } + finally { + if (connection1 != null) { + ((EmbeddedChannel) connection1.channel()).finishAndReleaseAll(); + connection1.dispose(); + } + if (connection2 != null) { + ((EmbeddedChannel) connection2.channel()).finishAndReleaseAll(); + connection2.dispose(); + } + } + } + + @Test + void maxIdleTimeActiveStreams() throws Exception { + EmbeddedChannel channel = new EmbeddedChannel(new TestChannelId(), + Http2FrameCodecBuilder.forClient().build(), new Http2MultiplexHandler(new ChannelHandlerAdapter() {})); + PoolBuilder> poolBuilder = + PoolBuilder.from(Mono.just(Connection.from(channel))) + .idleResourceReuseLruOrder() + .maxPendingAcquireUnbounded() + .sizeBetween(0, 1); + Http2Pool http2Pool = poolBuilder.build(config -> new Http2Pool(config, 10, -1)); + + Connection connection1 = null; + Connection connection2 = null; + try { + List> acquired = new ArrayList<>(); + http2Pool.acquire().subscribe(acquired::add); + http2Pool.acquire().subscribe(acquired::add); + + channel.runPendingTasks(); + + assertThat(acquired).hasSize(2); + assertThat(http2Pool.activeStreams()).isEqualTo(2); + assertThat(http2Pool.connections.size()).isEqualTo(1); + + connection1 = acquired.get(0).poolable(); + ChannelId id1 = connection1.channel().id(); + + acquired.get(0).invalidate().block(); + + Thread.sleep(15); + + assertThat(http2Pool.activeStreams()).isEqualTo(1); + assertThat(http2Pool.connections.size()).isEqualTo(1); + + connection2 = acquired.get(1).poolable(); + ChannelId id2 = connection2.channel().id(); + + assertThat(id1).isEqualTo(id2); + + acquired.get(1).invalidate().block(); + + assertThat(http2Pool.activeStreams()).isEqualTo(0); + assertThat(http2Pool.connections.size()).isEqualTo(1); + } + finally { + if (connection1 != null) { + ((EmbeddedChannel) connection1.channel()).finishAndReleaseAll(); + connection1.dispose(); + } + if (connection2 != null) { + ((EmbeddedChannel) connection2.channel()).finishAndReleaseAll(); + connection2.dispose(); + } + } + } + @Test void maxLifeTime() throws Exception { PoolBuilder> poolBuilder = @@ -347,7 +461,7 @@ void maxLifeTime() throws Exception { .idleResourceReuseLruOrder() .maxPendingAcquireUnbounded() .sizeBetween(0, 1); - Http2Pool http2Pool = poolBuilder.build(config -> new Http2Pool(config, 10)); + Http2Pool http2Pool = poolBuilder.build(config -> new Http2Pool(config, -1, 10)); Connection connection1 = null; Connection connection2 = null; @@ -411,7 +525,7 @@ void maxLifeTimeMaxConnectionsNotReached() throws Exception { .idleResourceReuseLruOrder() .maxPendingAcquireUnbounded() .sizeBetween(0, 2); - Http2Pool http2Pool = poolBuilder.build(config -> new Http2Pool(config, 50)); + Http2Pool http2Pool = poolBuilder.build(config -> new Http2Pool(config, -1, 50)); Connection connection1 = null; Connection connection2 = null; @@ -471,7 +585,7 @@ void maxLifeTimeMaxConnectionsReached() throws Exception { .idleResourceReuseLruOrder() .maxPendingAcquireUnbounded() .sizeBetween(0, 1); - Http2Pool http2Pool = poolBuilder.build(config -> new Http2Pool(config, 10)); + Http2Pool http2Pool = poolBuilder.build(config -> new Http2Pool(config, -1, 10)); Connection connection = null; try { @@ -514,7 +628,7 @@ void minConnectionsConfigNotSupported() { PoolBuilder> poolBuilder = PoolBuilder.from(Mono.empty()).sizeBetween(1, 2); assertThatExceptionOfType(IllegalArgumentException.class) - .isThrownBy(() -> poolBuilder.build(config -> new Http2Pool(config, -1))); + .isThrownBy(() -> poolBuilder.build(config -> new Http2Pool(config, -1, -1))); } @Test @@ -525,7 +639,7 @@ void nonHttp2ConnectionEmittedOnce() { .idleResourceReuseLruOrder() .maxPendingAcquireUnbounded() .sizeBetween(0, 1); - Http2Pool http2Pool = poolBuilder.build(config -> new Http2Pool(config, -1)); + Http2Pool http2Pool = poolBuilder.build(config -> new Http2Pool(config, -1, -1)); try { PooledRef acquired = http2Pool.acquire().block(Duration.ofSeconds(1)); From dae5de852a9966146471ba291de3670292959d10 Mon Sep 17 00:00:00 2001 From: Violeta Georgieva Date: Mon, 6 Jun 2022 11:22:08 +0300 Subject: [PATCH 39/42] Add evictInBackground to Http2Pool (#2257) Related to #2151 and #2262 --- .../reactor/netty/http/client/Http2Pool.java | 88 +++++++- .../netty/http/client/Http2PoolTest.java | 207 ++++++++++++++++++ 2 files changed, 291 insertions(+), 4 deletions(-) diff --git a/reactor-netty-http/src/main/java/reactor/netty/http/client/Http2Pool.java b/reactor-netty-http/src/main/java/reactor/netty/http/client/Http2Pool.java index 5261ba7226..ccf34fb809 100644 --- a/reactor-netty-http/src/main/java/reactor/netty/http/client/Http2Pool.java +++ b/reactor-netty-http/src/main/java/reactor/netty/http/client/Http2Pool.java @@ -17,6 +17,7 @@ import java.time.Clock; import java.time.Duration; +import java.util.Iterator; import java.util.Objects; import java.util.concurrent.ConcurrentLinkedDeque; import java.util.concurrent.ConcurrentLinkedQueue; @@ -88,9 +89,6 @@ * Configurations that are not applicable * *

+ * If minimum connections is specified, the cached connections with active streams will be kept at that minimum + * (can be the best effort). However, if the cached connections have reached max concurrent streams, + * then new connections will be allocated up to the maximum connections limit. + *

* Configurations that are not applicable *

    *
  • {@link PoolConfig#destroyHandler()} - the destroy handler cannot be used as the destruction is more complex.
    • @@ -97,7 +103,6 @@ *
  • {@link PoolConfig#reuseIdleResourcesInLruOrder()} - FIFO is used when checking the connections.
    • *
  • FIFO is used when obtaining the pending borrowers
    • *
  • Warm up functionality is not supported
    • - *
  • Setting minimum connections configuration is not supported
    • *
*

This class is based on * https://github.com/reactor/reactor-pool/blob/v0.2.7/src/main/java/reactor/pool/SimpleDequePool.java @@ -142,28 +147,35 @@ final class Http2Pool implements InstrumentedPool, InstrumentedPool. @SuppressWarnings("rawtypes") static final ConcurrentLinkedDeque TERMINATED = new ConcurrentLinkedDeque(); + volatile long totalMaxConcurrentStreams; + static final AtomicLongFieldUpdater TOTAL_MAX_CONCURRENT_STREAMS = + AtomicLongFieldUpdater.newUpdater(Http2Pool.class, "totalMaxConcurrentStreams"); + volatile int wip; static final AtomicIntegerFieldUpdater WIP = AtomicIntegerFieldUpdater.newUpdater(Http2Pool.class, "wip"); final Clock clock; + final Long maxConcurrentStreams; final long maxIdleTime; final long maxLifeTime; + final int minConnections; final PoolConfig poolConfig; long lastInteractionTimestamp; Disposable evictionTask; - Http2Pool(PoolConfig poolConfig, long maxIdleTime, long maxLifeTime) { - if (poolConfig.allocationStrategy().getPermits(0) != 0) { - throw new IllegalArgumentException("No support for configuring minimum number of connections"); - } + Http2Pool(PoolConfig poolConfig, @Nullable ConnectionProvider.AllocationStrategy allocationStrategy, + long maxIdleTime, long maxLifeTime) { this.clock = poolConfig.clock(); this.connections = new ConcurrentLinkedQueue<>(); this.lastInteractionTimestamp = clock.millis(); + this.maxConcurrentStreams = allocationStrategy instanceof Http2AllocationStrategy ? + ((Http2AllocationStrategy) allocationStrategy).maxConcurrentStreams() : -1; this.maxIdleTime = maxIdleTime; this.maxLifeTime = maxLifeTime; + this.minConnections = allocationStrategy == null ? 0 : allocationStrategy.permitMinimum(); this.pending = new ConcurrentLinkedDeque<>(); this.poolConfig = poolConfig; @@ -356,7 +368,10 @@ void drainLoop() { if (borrowersCount != 0) { // find a connection that can be used for opening a new stream - Slot slot = findConnection(resources); + // when cached connections are below minimum connections, then allocate a new connection + boolean belowMinConnections = minConnections > 0 && + poolConfig.allocationStrategy().permitGranted() < minConnections; + Slot slot = belowMinConnections ? null : findConnection(resources); if (slot != null) { Borrower borrower = pollPending(borrowers, true); if (borrower == null) { @@ -378,52 +393,64 @@ void drainLoop() { }); } else { - int permits = poolConfig.allocationStrategy().getPermits(1); - if (permits <= 0) { - if (maxPending >= 0) { - borrowersCount = pendingSize; - int toCull = borrowersCount - maxPending; - for (int i = 0; i < toCull; i++) { - Borrower extraneous = pollPending(borrowers, true); - if (extraneous != null) { - pendingAcquireLimitReached(extraneous, maxPending); - } - } - } + int resourcesCount = idleSize; + if (minConnections > 0 && + poolConfig.allocationStrategy().permitGranted() >= minConnections && + resourcesCount == 0) { + // connections allocations were triggered } else { - Borrower borrower = pollPending(borrowers, true); - if (borrower == null) { - continue; + int permits = poolConfig.allocationStrategy().getPermits(1); + if (permits <= 0) { + if (maxPending >= 0) { + borrowersCount = pendingSize; + int toCull = borrowersCount - maxPending; + for (int i = 0; i < toCull; i++) { + Borrower extraneous = pollPending(borrowers, true); + if (extraneous != null) { + pendingAcquireLimitReached(extraneous, maxPending); + } + } + } } - if (isDisposed()) { - borrower.fail(new PoolShutdownException()); - return; + else { + if (permits > 1) { + // warmup is not supported + poolConfig.allocationStrategy().returnPermits(permits - 1); + } + Borrower borrower = pollPending(borrowers, true); + if (borrower == null) { + continue; + } + if (isDisposed()) { + borrower.fail(new PoolShutdownException()); + return; + } + borrower.stopPendingCountdown(); + Mono allocator = poolConfig.allocator(); + Mono primary = + allocator.doOnEach(sig -> { + if (sig.isOnNext()) { + Connection newInstance = sig.get(); + assert newInstance != null; + Slot newSlot = new Slot(this, newInstance); + if (log.isDebugEnabled()) { + log.debug(format(newInstance.channel(), "Channel activated")); + } + ACQUIRED.incrementAndGet(this); + borrower.deliver(new Http2PooledRef(newSlot)); + } + else if (sig.isOnError()) { + Throwable error = sig.getThrowable(); + assert error != null; + poolConfig.allocationStrategy().returnPermits(1); + borrower.fail(error); + } + }) + .contextWrite(borrower.currentContext()); + + primary.subscribe(alreadyPropagated -> {}, alreadyPropagatedOrLogged -> drain(), this::drain); } - borrower.stopPendingCountdown(); - Mono allocator = poolConfig.allocator(); - Mono primary = - allocator.doOnEach(sig -> { - if (sig.isOnNext()) { - Connection newInstance = sig.get(); - assert newInstance != null; - Slot newSlot = new Slot(this, newInstance); - if (log.isDebugEnabled()) { - log.debug(format(newInstance.channel(), "Channel activated")); - } - ACQUIRED.incrementAndGet(this); - borrower.deliver(new Http2PooledRef(newSlot)); - } - else if (sig.isOnError()) { - Throwable error = sig.getThrowable(); - assert error != null; - poolConfig.allocationStrategy().returnPermits(1); - borrower.fail(error); - } - }) - .contextWrite(borrower.currentContext()); - - primary.subscribe(alreadyPropagated -> {}, alreadyPropagatedOrLogged -> drain(), this::drain); } } } @@ -728,10 +755,10 @@ Context currentContext() { @Override public void request(long n) { if (Operators.validate(n)) { - // Cannot rely on idleSize because there might be idle connections but not suitable for use + long estimateStreamsCount = pool.totalMaxConcurrentStreams - pool.acquired; int permits = pool.poolConfig.allocationStrategy().estimatePermitCount(); int pending = pool.pendingSize; - if (!acquireTimeout.isZero() && permits <= pending) { + if (!acquireTimeout.isZero() && permits + estimateStreamsCount <= pending) { timeoutTask = Schedulers.parallel().schedule(this, acquireTimeout.toMillis(), TimeUnit.MILLISECONDS); } pool.doAcquire(this); @@ -893,6 +920,7 @@ static final class Slot extends AtomicBoolean { final String applicationProtocol; long idleTimestamp; + long maxConcurrentStreams; volatile ChannelHandlerContext http2FrameCodecCtx; volatile ChannelHandlerContext http2MultiplexHandlerCtx; @@ -910,12 +938,26 @@ static final class Slot extends AtomicBoolean { else { this.applicationProtocol = null; } + ChannelHandlerContext frameCodec = http2FrameCodecCtx(); + if (frameCodec != null && http2MultiplexHandlerCtx() != null) { + this.maxConcurrentStreams = ((Http2FrameCodec) frameCodec.handler()).connection().local().maxActiveStreams(); + this.maxConcurrentStreams = pool.maxConcurrentStreams == -1 ? maxConcurrentStreams : + Math.min(pool.maxConcurrentStreams, maxConcurrentStreams); + } + TOTAL_MAX_CONCURRENT_STREAMS.addAndGet(this.pool, this.maxConcurrentStreams); } boolean canOpenStream() { ChannelHandlerContext frameCodec = http2FrameCodecCtx(); if (frameCodec != null && http2MultiplexHandlerCtx() != null) { - int maxActiveStreams = ((Http2FrameCodec) frameCodec.handler()).connection().local().maxActiveStreams(); + long maxActiveStreams = ((Http2FrameCodec) frameCodec.handler()).connection().local().maxActiveStreams(); + maxActiveStreams = pool.maxConcurrentStreams == -1 ? maxActiveStreams : + Math.min(pool.maxConcurrentStreams, maxActiveStreams); + long diff = maxActiveStreams - maxConcurrentStreams; + if (diff != 0) { + maxConcurrentStreams = maxActiveStreams; + TOTAL_MAX_CONCURRENT_STREAMS.addAndGet(this.pool, diff); + } int concurrency = this.concurrency; return concurrency < maxActiveStreams; } @@ -992,6 +1034,7 @@ void invalidate() { log.debug(format(connection.channel(), "Channel removed from pool")); } pool.poolConfig.allocationStrategy().returnPermits(1); + TOTAL_MAX_CONCURRENT_STREAMS.addAndGet(this.pool, -maxConcurrentStreams); } } diff --git a/reactor-netty-http/src/test/java/reactor/netty/http/client/Http2AllocationStrategyTest.java b/reactor-netty-http/src/test/java/reactor/netty/http/client/Http2AllocationStrategyTest.java new file mode 100644 index 0000000000..d708754cf2 --- /dev/null +++ b/reactor-netty-http/src/test/java/reactor/netty/http/client/Http2AllocationStrategyTest.java @@ -0,0 +1,108 @@ +/* + * Copyright (c) 2022 VMware, Inc. or its affiliates, All Rights Reserved. + * + * 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 + * + * https://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 reactor.netty.http.client; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatExceptionOfType; +import static reactor.netty.http.client.Http2AllocationStrategy.Build.DEFAULT_MAX_CONCURRENT_STREAMS; +import static reactor.netty.http.client.Http2AllocationStrategy.Build.DEFAULT_MAX_CONNECTIONS; +import static reactor.netty.http.client.Http2AllocationStrategy.Build.DEFAULT_MIN_CONNECTIONS; + +class Http2AllocationStrategyTest { + private Http2AllocationStrategy.Builder builder; + + @BeforeEach + void setUp() { + builder = Http2AllocationStrategy.builder(); + } + + @Test + void build() { + builder.maxConcurrentStreams(2).maxConnections(2).minConnections(1); + Http2AllocationStrategy strategy = builder.build(); + assertThat(strategy.maxConcurrentStreams()).isEqualTo(2); + assertThat(strategy.permitMaximum()).isEqualTo(2); + assertThat(strategy.permitMinimum()).isEqualTo(1); + } + + @Test + void buildBadValues() { + assertThatExceptionOfType(IllegalArgumentException.class) + .isThrownBy(() -> builder.maxConnections(1).minConnections(2).build()) + .withMessage("minConnections (2) must be less than or equal to maxConnections (1)"); + } + + @Test + void copy() { + builder.maxConcurrentStreams(2).maxConnections(2).minConnections(1); + Http2AllocationStrategy strategy = builder.build(); + Http2AllocationStrategy copy = strategy.copy(); + assertThat(copy.maxConcurrentStreams()).isEqualTo(strategy.maxConcurrentStreams()); + assertThat(copy.permitMaximum()).isEqualTo(strategy.permitMaximum()); + assertThat(copy.permitMinimum()).isEqualTo(strategy.permitMinimum()); + } + + @Test + void maxConcurrentStreams() { + builder.maxConcurrentStreams(2); + Http2AllocationStrategy strategy = builder.build(); + assertThat(strategy.maxConcurrentStreams()).isEqualTo(2); + assertThat(strategy.permitMaximum()).isEqualTo(DEFAULT_MAX_CONNECTIONS); + assertThat(strategy.permitMinimum()).isEqualTo(DEFAULT_MIN_CONNECTIONS); + } + + @Test + void maxConcurrentStreamsBadValues() { + assertThatExceptionOfType(IllegalArgumentException.class) + .isThrownBy(() -> builder.maxConcurrentStreams(-2)) + .withMessage("maxConcurrentStreams must be greater than or equal to -1"); + } + + @Test + void permitMaximum() { + builder.maxConnections(2); + Http2AllocationStrategy strategy = builder.build(); + assertThat(strategy.maxConcurrentStreams()).isEqualTo(DEFAULT_MAX_CONCURRENT_STREAMS); + assertThat(strategy.permitMaximum()).isEqualTo(2); + assertThat(strategy.permitMinimum()).isEqualTo(DEFAULT_MIN_CONNECTIONS); + } + + @Test + void permitMaximumBadValues() { + assertThatExceptionOfType(IllegalArgumentException.class) + .isThrownBy(() -> builder.maxConnections(0)) + .withMessage("maxConnections must be strictly positive"); + } + + @Test + void permitMinimum() { + builder.minConnections(2); + Http2AllocationStrategy strategy = builder.build(); + assertThat(strategy.maxConcurrentStreams()).isEqualTo(DEFAULT_MAX_CONCURRENT_STREAMS); + assertThat(strategy.permitMaximum()).isEqualTo(DEFAULT_MAX_CONNECTIONS); + assertThat(strategy.permitMinimum()).isEqualTo(2); + } + + @Test + void permitMinimumBadValues() { + assertThatExceptionOfType(IllegalArgumentException.class) + .isThrownBy(() -> builder.minConnections(-1)) + .withMessage("minConnections must be positive or zero"); + } +} diff --git a/reactor-netty-http/src/test/java/reactor/netty/http/client/Http2PoolTest.java b/reactor-netty-http/src/test/java/reactor/netty/http/client/Http2PoolTest.java index ec317c8828..6780f795c2 100644 --- a/reactor-netty-http/src/test/java/reactor/netty/http/client/Http2PoolTest.java +++ b/reactor-netty-http/src/test/java/reactor/netty/http/client/Http2PoolTest.java @@ -22,6 +22,7 @@ import io.netty.handler.codec.http2.Http2FrameCodecBuilder; import io.netty.handler.codec.http2.Http2MultiplexHandler; import org.junit.jupiter.api.Test; +import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; import reactor.netty.Connection; import reactor.netty.internal.shaded.reactor.pool.PoolAcquireTimeoutException; @@ -40,7 +41,6 @@ import java.util.concurrent.atomic.AtomicReference; import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.assertThatExceptionOfType; class Http2PoolTest { @@ -53,7 +53,7 @@ void acquireInvalidate() { .idleResourceReuseLruOrder() .maxPendingAcquireUnbounded() .sizeBetween(0, 1); - Http2Pool http2Pool = poolBuilder.build(config -> new Http2Pool(config, -1, -1)); + Http2Pool http2Pool = poolBuilder.build(config -> new Http2Pool(config, null, -1, -1)); try { List> acquired = new ArrayList<>(); @@ -65,12 +65,14 @@ void acquireInvalidate() { assertThat(acquired).hasSize(3); assertThat(http2Pool.activeStreams()).isEqualTo(3); + assertThat(http2Pool.totalMaxConcurrentStreams).isEqualTo(Integer.MAX_VALUE); for (PooledRef slot : acquired) { slot.invalidate().block(Duration.ofSeconds(1)); } assertThat(http2Pool.activeStreams()).isEqualTo(0); + assertThat(http2Pool.totalMaxConcurrentStreams).isEqualTo(Integer.MAX_VALUE); for (PooledRef slot : acquired) { // second invalidate() should be ignored and ACQUIRED size should remain the same @@ -78,6 +80,7 @@ void acquireInvalidate() { } assertThat(http2Pool.activeStreams()).isEqualTo(0); + assertThat(http2Pool.totalMaxConcurrentStreams).isEqualTo(Integer.MAX_VALUE); } finally { channel.finishAndReleaseAll(); @@ -94,7 +97,7 @@ void acquireRelease() { .idleResourceReuseLruOrder() .maxPendingAcquireUnbounded() .sizeBetween(0, 1); - Http2Pool http2Pool = poolBuilder.build(config -> new Http2Pool(config, -1, -1)); + Http2Pool http2Pool = poolBuilder.build(config -> new Http2Pool(config, null, -1, -1)); try { List> acquired = new ArrayList<>(); @@ -106,12 +109,14 @@ void acquireRelease() { assertThat(acquired).hasSize(3); assertThat(http2Pool.activeStreams()).isEqualTo(3); + assertThat(http2Pool.totalMaxConcurrentStreams).isEqualTo(Integer.MAX_VALUE); for (PooledRef slot : acquired) { slot.release().block(Duration.ofSeconds(1)); } assertThat(http2Pool.activeStreams()).isEqualTo(0); + assertThat(http2Pool.totalMaxConcurrentStreams).isEqualTo(Integer.MAX_VALUE); for (PooledRef slot : acquired) { // second release() should be ignored and ACQUIRED size should remain the same @@ -119,6 +124,7 @@ void acquireRelease() { } assertThat(http2Pool.activeStreams()).isEqualTo(0); + assertThat(http2Pool.totalMaxConcurrentStreams).isEqualTo(Integer.MAX_VALUE); } finally { channel.finishAndReleaseAll(); @@ -138,7 +144,7 @@ void evictClosedConnection() throws Exception { .idleResourceReuseLruOrder() .maxPendingAcquireUnbounded() .sizeBetween(0, 1); - Http2Pool http2Pool = poolBuilder.build(config -> new Http2Pool(config, -1, -1)); + Http2Pool http2Pool = poolBuilder.build(config -> new Http2Pool(config, null, -1, -1)); Connection connection = null; try { @@ -147,6 +153,7 @@ void evictClosedConnection() throws Exception { assertThat(acquired1).isNotNull(); assertThat(http2Pool.activeStreams()).isEqualTo(1); assertThat(http2Pool.connections.size()).isEqualTo(1); + assertThat(http2Pool.totalMaxConcurrentStreams).isEqualTo(0); connection = acquired1.poolable(); ChannelId id1 = connection.channel().id(); @@ -159,17 +166,20 @@ void evictClosedConnection() throws Exception { assertThat(http2Pool.activeStreams()).isEqualTo(1); assertThat(http2Pool.connections.size()).isEqualTo(1); + assertThat(http2Pool.totalMaxConcurrentStreams).isEqualTo(0); acquired1.invalidate().block(); assertThat(http2Pool.activeStreams()).isEqualTo(0); assertThat(http2Pool.connections.size()).isEqualTo(0); + assertThat(http2Pool.totalMaxConcurrentStreams).isEqualTo(0); PooledRef acquired2 = http2Pool.acquire().block(); assertThat(acquired2).isNotNull(); assertThat(http2Pool.activeStreams()).isEqualTo(1); assertThat(http2Pool.connections.size()).isEqualTo(1); + assertThat(http2Pool.totalMaxConcurrentStreams).isEqualTo(0); connection = acquired2.poolable(); ChannelId id2 = connection.channel().id(); @@ -180,6 +190,7 @@ void evictClosedConnection() throws Exception { assertThat(http2Pool.activeStreams()).isEqualTo(0); assertThat(http2Pool.connections.size()).isEqualTo(1); + assertThat(http2Pool.totalMaxConcurrentStreams).isEqualTo(0); } finally { if (connection != null) { @@ -211,7 +222,7 @@ private void evictClosedConnectionMaxConnectionsNotReached(boolean closeSecond) .idleResourceReuseLruOrder() .maxPendingAcquireUnbounded() .sizeBetween(0, 2); - Http2Pool http2Pool = poolBuilder.build(config -> new Http2Pool(config, -1, -1)); + Http2Pool http2Pool = poolBuilder.build(config -> new Http2Pool(config, null, -1, -1)); Connection connection = null; try { @@ -220,6 +231,7 @@ private void evictClosedConnectionMaxConnectionsNotReached(boolean closeSecond) assertThat(acquired1).isNotNull(); assertThat(http2Pool.activeStreams()).isEqualTo(1); assertThat(http2Pool.connections.size()).isEqualTo(1); + assertThat(http2Pool.totalMaxConcurrentStreams).isEqualTo(Integer.MAX_VALUE); connection = acquired1.poolable(); ChannelId id1 = connection.channel().id(); @@ -232,6 +244,7 @@ private void evictClosedConnectionMaxConnectionsNotReached(boolean closeSecond) assertThat(http2Pool.activeStreams()).isEqualTo(1); assertThat(http2Pool.connections.size()).isEqualTo(1); + assertThat(http2Pool.totalMaxConcurrentStreams).isEqualTo(Integer.MAX_VALUE); PooledRef acquired2 = http2Pool.acquire().block(); assertThat(acquired2).isNotNull(); @@ -244,6 +257,7 @@ private void evictClosedConnectionMaxConnectionsNotReached(boolean closeSecond) assertThat(http2Pool.activeStreams()).isEqualTo(3); assertThat(http2Pool.connections.size()).isEqualTo(2); + assertThat(http2Pool.totalMaxConcurrentStreams).isEqualTo(2L * Integer.MAX_VALUE); if (closeSecond) { latch = new CountDownLatch(1); @@ -262,15 +276,18 @@ private void evictClosedConnectionMaxConnectionsNotReached(boolean closeSecond) assertThat(http2Pool.activeStreams()).isEqualTo(1); assertThat(http2Pool.connections.size()).isEqualTo(1); + assertThat(http2Pool.totalMaxConcurrentStreams).isEqualTo(Integer.MAX_VALUE); acquired3.get().invalidate().block(); assertThat(http2Pool.activeStreams()).isEqualTo(0); if (closeSecond) { assertThat(http2Pool.connections.size()).isEqualTo(0); + assertThat(http2Pool.totalMaxConcurrentStreams).isEqualTo(0); } else { assertThat(http2Pool.connections.size()).isEqualTo(1); + assertThat(http2Pool.totalMaxConcurrentStreams).isEqualTo(Integer.MAX_VALUE); } } finally { @@ -293,7 +310,7 @@ void evictClosedConnectionMaxConnectionsReached() throws Exception { .idleResourceReuseLruOrder() .maxPendingAcquireUnbounded() .sizeBetween(0, 1); - Http2Pool http2Pool = poolBuilder.build(config -> new Http2Pool(config, -1, -1)); + Http2Pool http2Pool = poolBuilder.build(config -> new Http2Pool(config, null, -1, -1)); Connection connection = null; try { @@ -302,6 +319,7 @@ void evictClosedConnectionMaxConnectionsReached() throws Exception { assertThat(acquired1).isNotNull(); assertThat(http2Pool.activeStreams()).isEqualTo(1); assertThat(http2Pool.connections.size()).isEqualTo(1); + assertThat(http2Pool.totalMaxConcurrentStreams).isEqualTo(0); connection = acquired1.poolable(); CountDownLatch latch = new CountDownLatch(1); @@ -313,6 +331,7 @@ void evictClosedConnectionMaxConnectionsReached() throws Exception { assertThat(http2Pool.activeStreams()).isEqualTo(1); assertThat(http2Pool.connections.size()).isEqualTo(1); + assertThat(http2Pool.totalMaxConcurrentStreams).isEqualTo(0); http2Pool.acquire(Duration.ofMillis(10)) .as(StepVerifier::create) @@ -321,11 +340,13 @@ void evictClosedConnectionMaxConnectionsReached() throws Exception { assertThat(http2Pool.activeStreams()).isEqualTo(1); assertThat(http2Pool.connections.size()).isEqualTo(1); + assertThat(http2Pool.totalMaxConcurrentStreams).isEqualTo(0); acquired1.invalidate().block(); assertThat(http2Pool.activeStreams()).isEqualTo(0); assertThat(http2Pool.connections.size()).isEqualTo(0); + assertThat(http2Pool.totalMaxConcurrentStreams).isEqualTo(0); } finally { if (connection != null) { @@ -357,6 +378,7 @@ void evictInBackgroundClosedConnection() throws Exception { assertThat(acquired1).isNotNull(); assertThat(http2Pool.activeStreams()).isEqualTo(1); assertThat(http2Pool.connections.size()).isEqualTo(1); + assertThat(http2Pool.totalMaxConcurrentStreams).isEqualTo(0); connection = acquired1.poolable(); ChannelId id1 = connection.channel().id(); @@ -369,6 +391,7 @@ void evictInBackgroundClosedConnection() throws Exception { assertThat(http2Pool.activeStreams()).isEqualTo(1); assertThat(http2Pool.connections.size()).isEqualTo(1); + assertThat(http2Pool.totalMaxConcurrentStreams).isEqualTo(0); acquired1.invalidate().block(); @@ -376,12 +399,14 @@ void evictInBackgroundClosedConnection() throws Exception { assertThat(http2Pool.activeStreams()).isEqualTo(0); assertThat(http2Pool.connections.size()).isEqualTo(0); + assertThat(http2Pool.totalMaxConcurrentStreams).isEqualTo(0); PooledRef acquired2 = http2Pool.acquire().block(); assertThat(acquired2).isNotNull(); assertThat(http2Pool.activeStreams()).isEqualTo(1); assertThat(http2Pool.connections.size()).isEqualTo(1); + assertThat(http2Pool.totalMaxConcurrentStreams).isEqualTo(0); connection = acquired2.poolable(); ChannelId id2 = connection.channel().id(); @@ -394,6 +419,7 @@ void evictInBackgroundClosedConnection() throws Exception { assertThat(http2Pool.activeStreams()).isEqualTo(0); assertThat(http2Pool.connections.size()).isEqualTo(1); + assertThat(http2Pool.totalMaxConcurrentStreams).isEqualTo(0); } finally { if (connection != null) { @@ -426,6 +452,7 @@ void evictInBackgroundMaxIdleTime() throws Exception { assertThat(acquired1).isNotNull(); assertThat(http2Pool.activeStreams()).isEqualTo(1); assertThat(http2Pool.connections.size()).isEqualTo(1); + assertThat(http2Pool.totalMaxConcurrentStreams).isEqualTo(0); connection1 = acquired1.poolable(); ChannelId id1 = connection1.channel().id(); @@ -438,12 +465,14 @@ void evictInBackgroundMaxIdleTime() throws Exception { assertThat(http2Pool.activeStreams()).isEqualTo(0); assertThat(http2Pool.connections.size()).isEqualTo(0); + assertThat(http2Pool.totalMaxConcurrentStreams).isEqualTo(0); PooledRef acquired2 = http2Pool.acquire().block(); assertThat(acquired2).isNotNull(); assertThat(http2Pool.activeStreams()).isEqualTo(1); assertThat(http2Pool.connections.size()).isEqualTo(1); + assertThat(http2Pool.totalMaxConcurrentStreams).isEqualTo(0); connection2 = acquired2.poolable(); ChannelId id2 = connection2.channel().id(); @@ -458,6 +487,7 @@ void evictInBackgroundMaxIdleTime() throws Exception { assertThat(http2Pool.activeStreams()).isEqualTo(0); assertThat(http2Pool.connections.size()).isEqualTo(0); + assertThat(http2Pool.totalMaxConcurrentStreams).isEqualTo(0); } finally { if (connection1 != null) { @@ -494,6 +524,7 @@ void evictInBackgroundMaxLifeTime() throws Exception { assertThat(acquired1).isNotNull(); assertThat(http2Pool.activeStreams()).isEqualTo(1); assertThat(http2Pool.connections.size()).isEqualTo(1); + assertThat(http2Pool.totalMaxConcurrentStreams).isEqualTo(0); connection1 = acquired1.poolable(); ChannelId id1 = connection1.channel().id(); @@ -502,6 +533,7 @@ void evictInBackgroundMaxLifeTime() throws Exception { assertThat(http2Pool.activeStreams()).isEqualTo(1); assertThat(http2Pool.connections.size()).isEqualTo(1); + assertThat(http2Pool.totalMaxConcurrentStreams).isEqualTo(0); acquired1.invalidate().block(); @@ -509,12 +541,14 @@ void evictInBackgroundMaxLifeTime() throws Exception { assertThat(http2Pool.activeStreams()).isEqualTo(0); assertThat(http2Pool.connections.size()).isEqualTo(0); + assertThat(http2Pool.totalMaxConcurrentStreams).isEqualTo(0); PooledRef acquired2 = http2Pool.acquire().block(); assertThat(acquired2).isNotNull(); assertThat(http2Pool.activeStreams()).isEqualTo(1); assertThat(http2Pool.connections.size()).isEqualTo(1); + assertThat(http2Pool.totalMaxConcurrentStreams).isEqualTo(0); connection2 = acquired2.poolable(); ChannelId id2 = connection2.channel().id(); @@ -529,6 +563,7 @@ void evictInBackgroundMaxLifeTime() throws Exception { assertThat(http2Pool.activeStreams()).isEqualTo(0); assertThat(http2Pool.connections.size()).isEqualTo(0); + assertThat(http2Pool.totalMaxConcurrentStreams).isEqualTo(0); } finally { if (connection1 != null) { @@ -554,7 +589,7 @@ void maxIdleTime() throws Exception { .idleResourceReuseLruOrder() .maxPendingAcquireUnbounded() .sizeBetween(0, 1); - Http2Pool http2Pool = poolBuilder.build(config -> new Http2Pool(config, 10, -1)); + Http2Pool http2Pool = poolBuilder.build(config -> new Http2Pool(config, null, 10, -1)); Connection connection1 = null; Connection connection2 = null; @@ -564,6 +599,7 @@ void maxIdleTime() throws Exception { assertThat(acquired1).isNotNull(); assertThat(http2Pool.activeStreams()).isEqualTo(1); assertThat(http2Pool.connections.size()).isEqualTo(1); + assertThat(http2Pool.totalMaxConcurrentStreams).isEqualTo(0); connection1 = acquired1.poolable(); ChannelId id1 = connection1.channel().id(); @@ -577,6 +613,7 @@ void maxIdleTime() throws Exception { assertThat(acquired2).isNotNull(); assertThat(http2Pool.activeStreams()).isEqualTo(1); assertThat(http2Pool.connections.size()).isEqualTo(1); + assertThat(http2Pool.totalMaxConcurrentStreams).isEqualTo(0); connection2 = acquired2.poolable(); ChannelId id2 = connection2.channel().id(); @@ -587,6 +624,7 @@ void maxIdleTime() throws Exception { assertThat(http2Pool.activeStreams()).isEqualTo(0); assertThat(http2Pool.connections.size()).isEqualTo(1); + assertThat(http2Pool.totalMaxConcurrentStreams).isEqualTo(0); } finally { if (connection1 != null) { @@ -609,7 +647,7 @@ void maxIdleTimeActiveStreams() throws Exception { .idleResourceReuseLruOrder() .maxPendingAcquireUnbounded() .sizeBetween(0, 1); - Http2Pool http2Pool = poolBuilder.build(config -> new Http2Pool(config, 10, -1)); + Http2Pool http2Pool = poolBuilder.build(config -> new Http2Pool(config, null, 10, -1)); Connection connection1 = null; Connection connection2 = null; @@ -623,6 +661,7 @@ void maxIdleTimeActiveStreams() throws Exception { assertThat(acquired).hasSize(2); assertThat(http2Pool.activeStreams()).isEqualTo(2); assertThat(http2Pool.connections.size()).isEqualTo(1); + assertThat(http2Pool.totalMaxConcurrentStreams).isEqualTo(Integer.MAX_VALUE); connection1 = acquired.get(0).poolable(); ChannelId id1 = connection1.channel().id(); @@ -633,6 +672,7 @@ void maxIdleTimeActiveStreams() throws Exception { assertThat(http2Pool.activeStreams()).isEqualTo(1); assertThat(http2Pool.connections.size()).isEqualTo(1); + assertThat(http2Pool.totalMaxConcurrentStreams).isEqualTo(Integer.MAX_VALUE); connection2 = acquired.get(1).poolable(); ChannelId id2 = connection2.channel().id(); @@ -643,6 +683,7 @@ void maxIdleTimeActiveStreams() throws Exception { assertThat(http2Pool.activeStreams()).isEqualTo(0); assertThat(http2Pool.connections.size()).isEqualTo(1); + assertThat(http2Pool.totalMaxConcurrentStreams).isEqualTo(Integer.MAX_VALUE); } finally { if (connection1 != null) { @@ -668,7 +709,7 @@ void maxLifeTime() throws Exception { .idleResourceReuseLruOrder() .maxPendingAcquireUnbounded() .sizeBetween(0, 1); - Http2Pool http2Pool = poolBuilder.build(config -> new Http2Pool(config, -1, 10)); + Http2Pool http2Pool = poolBuilder.build(config -> new Http2Pool(config, null, -1, 10)); Connection connection1 = null; Connection connection2 = null; @@ -678,6 +719,7 @@ void maxLifeTime() throws Exception { assertThat(acquired1).isNotNull(); assertThat(http2Pool.activeStreams()).isEqualTo(1); assertThat(http2Pool.connections.size()).isEqualTo(1); + assertThat(http2Pool.totalMaxConcurrentStreams).isEqualTo(0); connection1 = acquired1.poolable(); ChannelId id1 = connection1.channel().id(); @@ -686,17 +728,20 @@ void maxLifeTime() throws Exception { assertThat(http2Pool.activeStreams()).isEqualTo(1); assertThat(http2Pool.connections.size()).isEqualTo(1); + assertThat(http2Pool.totalMaxConcurrentStreams).isEqualTo(0); acquired1.invalidate().block(); assertThat(http2Pool.activeStreams()).isEqualTo(0); assertThat(http2Pool.connections.size()).isEqualTo(0); + assertThat(http2Pool.totalMaxConcurrentStreams).isEqualTo(0); PooledRef acquired2 = http2Pool.acquire().block(); assertThat(acquired2).isNotNull(); assertThat(http2Pool.activeStreams()).isEqualTo(1); assertThat(http2Pool.connections.size()).isEqualTo(1); + assertThat(http2Pool.totalMaxConcurrentStreams).isEqualTo(0); connection2 = acquired2.poolable(); ChannelId id2 = connection2.channel().id(); @@ -707,6 +752,7 @@ void maxLifeTime() throws Exception { assertThat(http2Pool.activeStreams()).isEqualTo(0); assertThat(http2Pool.connections.size()).isEqualTo(1); + assertThat(http2Pool.totalMaxConcurrentStreams).isEqualTo(0); } finally { if (connection1 != null) { @@ -732,7 +778,7 @@ void maxLifeTimeMaxConnectionsNotReached() throws Exception { .idleResourceReuseLruOrder() .maxPendingAcquireUnbounded() .sizeBetween(0, 2); - Http2Pool http2Pool = poolBuilder.build(config -> new Http2Pool(config, -1, 50)); + Http2Pool http2Pool = poolBuilder.build(config -> new Http2Pool(config, null, -1, 50)); Connection connection1 = null; Connection connection2 = null; @@ -742,6 +788,7 @@ void maxLifeTimeMaxConnectionsNotReached() throws Exception { assertThat(acquired1).isNotNull(); assertThat(http2Pool.activeStreams()).isEqualTo(1); assertThat(http2Pool.connections.size()).isEqualTo(1); + assertThat(http2Pool.totalMaxConcurrentStreams).isEqualTo(0); connection1 = acquired1.poolable(); ChannelId id1 = connection1.channel().id(); @@ -750,12 +797,14 @@ void maxLifeTimeMaxConnectionsNotReached() throws Exception { assertThat(http2Pool.activeStreams()).isEqualTo(1); assertThat(http2Pool.connections.size()).isEqualTo(1); + assertThat(http2Pool.totalMaxConcurrentStreams).isEqualTo(0); PooledRef acquired2 = http2Pool.acquire().block(); assertThat(acquired2).isNotNull(); assertThat(http2Pool.activeStreams()).isEqualTo(2); assertThat(http2Pool.connections.size()).isEqualTo(2); + assertThat(http2Pool.totalMaxConcurrentStreams).isEqualTo(0); connection2 = acquired2.poolable(); ChannelId id2 = connection2.channel().id(); @@ -767,6 +816,7 @@ void maxLifeTimeMaxConnectionsNotReached() throws Exception { assertThat(http2Pool.activeStreams()).isEqualTo(0); assertThat(http2Pool.connections.size()).isEqualTo(1); + assertThat(http2Pool.totalMaxConcurrentStreams).isEqualTo(0); } finally { if (connection1 != null) { @@ -792,7 +842,7 @@ void maxLifeTimeMaxConnectionsReached() throws Exception { .idleResourceReuseLruOrder() .maxPendingAcquireUnbounded() .sizeBetween(0, 1); - Http2Pool http2Pool = poolBuilder.build(config -> new Http2Pool(config, -1, 10)); + Http2Pool http2Pool = poolBuilder.build(config -> new Http2Pool(config, null, -1, 10)); Connection connection = null; try { @@ -801,6 +851,7 @@ void maxLifeTimeMaxConnectionsReached() throws Exception { assertThat(acquired1).isNotNull(); assertThat(http2Pool.activeStreams()).isEqualTo(1); assertThat(http2Pool.connections.size()).isEqualTo(1); + assertThat(http2Pool.totalMaxConcurrentStreams).isEqualTo(0); connection = acquired1.poolable(); @@ -808,6 +859,7 @@ void maxLifeTimeMaxConnectionsReached() throws Exception { assertThat(http2Pool.activeStreams()).isEqualTo(1); assertThat(http2Pool.connections.size()).isEqualTo(1); + assertThat(http2Pool.totalMaxConcurrentStreams).isEqualTo(0); http2Pool.acquire(Duration.ofMillis(10)) .as(StepVerifier::create) @@ -816,11 +868,13 @@ void maxLifeTimeMaxConnectionsReached() throws Exception { assertThat(http2Pool.activeStreams()).isEqualTo(1); assertThat(http2Pool.connections.size()).isEqualTo(1); + assertThat(http2Pool.totalMaxConcurrentStreams).isEqualTo(0); acquired1.invalidate().block(); assertThat(http2Pool.activeStreams()).isEqualTo(0); assertThat(http2Pool.connections.size()).isEqualTo(0); + assertThat(http2Pool.totalMaxConcurrentStreams).isEqualTo(0); } finally { if (connection != null) { @@ -831,11 +885,101 @@ void maxLifeTimeMaxConnectionsReached() throws Exception { } @Test - void minConnectionsConfigNotSupported() { + void minConnections() { + EmbeddedChannel channel = new EmbeddedChannel(new TestChannelId(), + Http2FrameCodecBuilder.forClient().build(), new Http2MultiplexHandler(new ChannelHandlerAdapter() {})); PoolBuilder> poolBuilder = - PoolBuilder.from(Mono.empty()).sizeBetween(1, 2); - assertThatExceptionOfType(IllegalArgumentException.class) - .isThrownBy(() -> poolBuilder.build(config -> new Http2Pool(config, -1, -1))); + PoolBuilder.from(Mono.just(Connection.from(channel))) + .idleResourceReuseLruOrder() + .maxPendingAcquireUnbounded() + .sizeBetween(1, 3); + Http2AllocationStrategy strategy = Http2AllocationStrategy.builder() + .maxConnections(3) + .minConnections(1) + .build(); + Http2Pool http2Pool = poolBuilder.build(config -> new Http2Pool(config, strategy, -1, -1)); + + List> acquired = new ArrayList<>(); + try { + Flux.range(0, 3) + .flatMap(i -> http2Pool.acquire().doOnNext(acquired::add)) + .subscribe(); + + channel.runPendingTasks(); + + assertThat(acquired).hasSize(3); + + assertThat(http2Pool.activeStreams()).isEqualTo(3); + assertThat(acquired.get(0).poolable()).isSameAs(acquired.get(1).poolable()); + assertThat(acquired.get(0).poolable()).isSameAs(acquired.get(2).poolable()); + assertThat(http2Pool.totalMaxConcurrentStreams).isEqualTo(Integer.MAX_VALUE); + + for (PooledRef slot : acquired) { + slot.release().block(Duration.ofSeconds(1)); + } + + assertThat(http2Pool.activeStreams()).isEqualTo(0); + assertThat(http2Pool.totalMaxConcurrentStreams).isEqualTo(Integer.MAX_VALUE); + } + finally { + for (PooledRef slot : acquired) { + Connection conn = slot.poolable(); + ((EmbeddedChannel) conn.channel()).finishAndReleaseAll(); + conn.dispose(); + } + } + } + + @Test + void minConnectionsMaxStreamsReached() { + PoolBuilder> poolBuilder = + PoolBuilder.from(Mono.fromSupplier(() -> { + Channel channel = new EmbeddedChannel( + new TestChannelId(), + Http2FrameCodecBuilder.forClient().build()); + return Connection.from(channel); + })) + .idleResourceReuseLruOrder() + .maxPendingAcquireUnbounded() + .sizeBetween(1, 3); + Http2AllocationStrategy strategy = Http2AllocationStrategy.builder() + .maxConnections(3) + .minConnections(1) + .build(); + Http2Pool http2Pool = poolBuilder.build(config -> new Http2Pool(config, strategy, -1, -1)); + + List> acquired = new ArrayList<>(); + try { + Flux.range(0, 3) + .flatMap(i -> http2Pool.acquire().doOnNext(acquired::add)) + .blockLast(Duration.ofSeconds(1)); + + assertThat(acquired).hasSize(3); + + for (PooledRef pooledRef : acquired) { + ((EmbeddedChannel) pooledRef.poolable().channel()).runPendingTasks(); + } + + assertThat(http2Pool.activeStreams()).isEqualTo(3); + assertThat(acquired.get(0).poolable()).isNotSameAs(acquired.get(1).poolable()); + assertThat(acquired.get(0).poolable()).isNotSameAs(acquired.get(2).poolable()); + assertThat(acquired.get(1).poolable()).isNotSameAs(acquired.get(2).poolable()); + assertThat(http2Pool.totalMaxConcurrentStreams).isEqualTo(0); + + for (PooledRef slot : acquired) { + slot.release().block(Duration.ofSeconds(1)); + } + + assertThat(http2Pool.activeStreams()).isEqualTo(0); + assertThat(http2Pool.totalMaxConcurrentStreams).isEqualTo(0); + } + finally { + for (PooledRef slot : acquired) { + Connection conn = slot.poolable(); + ((EmbeddedChannel) conn.channel()).finishAndReleaseAll(); + conn.dispose(); + } + } } @Test @@ -846,13 +990,14 @@ void nonHttp2ConnectionEmittedOnce() { .idleResourceReuseLruOrder() .maxPendingAcquireUnbounded() .sizeBetween(0, 1); - Http2Pool http2Pool = poolBuilder.build(config -> new Http2Pool(config, -1, -1)); + Http2Pool http2Pool = poolBuilder.build(config -> new Http2Pool(config, null, -1, -1)); try { PooledRef acquired = http2Pool.acquire().block(Duration.ofSeconds(1)); assertThat(acquired).isNotNull(); assertThat(http2Pool.activeStreams()).isEqualTo(1); + assertThat(http2Pool.totalMaxConcurrentStreams).isEqualTo(0); http2Pool.acquire(Duration.ofMillis(10)) .as(StepVerifier::create) @@ -865,6 +1010,7 @@ void nonHttp2ConnectionEmittedOnce() { assertThat(http2Pool.activeStreams()).isEqualTo(0); assertThat(http2Pool.connections.size()).isEqualTo(0); + assertThat(http2Pool.totalMaxConcurrentStreams).isEqualTo(0); } finally { channel.finishAndReleaseAll(); diff --git a/reactor-netty-http/src/test/java/reactor/netty/resources/DefaultPooledConnectionProviderTest.java b/reactor-netty-http/src/test/java/reactor/netty/resources/DefaultPooledConnectionProviderTest.java index 32453a4c24..0cad951be8 100644 --- a/reactor-netty-http/src/test/java/reactor/netty/resources/DefaultPooledConnectionProviderTest.java +++ b/reactor-netty-http/src/test/java/reactor/netty/resources/DefaultPooledConnectionProviderTest.java @@ -49,6 +49,7 @@ import reactor.netty.http.Http11SslContextSpec; import reactor.netty.http.Http2SslContextSpec; import reactor.netty.http.HttpProtocol; +import reactor.netty.http.client.Http2AllocationStrategy; import reactor.netty.http.client.HttpClient; import reactor.netty.http.server.HttpServer; import reactor.netty.internal.shaded.reactor.pool.InstrumentedPool; @@ -543,6 +544,86 @@ private void doTestIssue1982(HttpProtocol[] serverProtocols, HttpProtocol[] clie } } + //https://github.com/reactor/reactor-netty/issues/1808 + @Test + void testMinConnections() throws Exception { + Http2SslContextSpec serverCtx = Http2SslContextSpec.forServer(ssc.certificate(), ssc.privateKey()); + Http2SslContextSpec clientCtx = + Http2SslContextSpec.forClient() + .configure(builder -> builder.trustManager(InsecureTrustManagerFactory.INSTANCE)); + + disposableServer = + createServer() + .wiretap(false) + .protocol(HttpProtocol.H2) + .secure(spec -> spec.sslContext(serverCtx)) + .route(routes -> routes.post("/", (req, res) -> res.send(req.receive().retain()))) + .bindNow(); + + int requestsNum = 100; + CountDownLatch latch = new CountDownLatch(1); + DefaultPooledConnectionProvider provider = + (DefaultPooledConnectionProvider) ConnectionProvider.builder("testMinConnections") + .allocationStrategy(Http2AllocationStrategy.builder().maxConnections(20).minConnections(5).build()) + .build(); + AtomicInteger counter = new AtomicInteger(); + AtomicReference serverAddress = new AtomicReference<>(); + HttpClient client = + createClient(provider, disposableServer.port()) + .wiretap(false) + .protocol(HttpProtocol.H2) + .secure(spec -> spec.sslContext(clientCtx)) + .metrics(true, Function.identity()) + .doAfterRequest((req, conn) -> serverAddress.set(conn.channel().remoteAddress())) + .observe((conn, state) -> { + if (state == STREAM_CONFIGURED) { + counter.incrementAndGet(); + conn.onTerminate() + .subscribe(null, + t -> conn.channel().eventLoop().execute(() -> { + if (counter.decrementAndGet() == 0) { + latch.countDown(); + } + }), + () -> conn.channel().eventLoop().execute(() -> { + if (counter.decrementAndGet() == 0) { + latch.countDown(); + } + })); + } + }); + + try { + Flux.range(0, requestsNum) + .flatMap(i -> + client.post() + .uri("/") + .send(ByteBufMono.fromString(Mono.just("testMinConnections"))) + .responseContent() + .aggregate() + .asString()) + .blockLast(Duration.ofSeconds(5)); + + assertThat(latch.await(5, TimeUnit.SECONDS)).isTrue(); + + InetSocketAddress sa = (InetSocketAddress) serverAddress.get(); + String address = sa.getHostString() + ":" + sa.getPort(); + + assertThat(getGaugeValue(CONNECTION_PROVIDER_PREFIX + ACTIVE_CONNECTIONS, + REMOTE_ADDRESS, address, NAME, "http2.testMinConnections")).isEqualTo(0); + double idleConn = getGaugeValue(CONNECTION_PROVIDER_PREFIX + IDLE_CONNECTIONS, + REMOTE_ADDRESS, address, NAME, "http2.testMinConnections"); + double totalConn = getGaugeValue(CONNECTION_PROVIDER_PREFIX + TOTAL_CONNECTIONS, + REMOTE_ADDRESS, address, NAME, "testMinConnections"); + assertThat(totalConn).isEqualTo(idleConn); + assertThat(totalConn).isLessThan(10); + } + finally { + provider.disposeLater() + .block(Duration.ofSeconds(5)); + } + } + static final class TestPromise extends DefaultChannelPromise { final ChannelPromise parent;