From c128d71c7abaabfc315da72a361591c122f00fc9 Mon Sep 17 00:00:00 2001 From: Zoe Wang <33073555+zoewangg@users.noreply.github.com> Date: Wed, 18 Sep 2024 10:23:48 -0700 Subject: [PATCH 1/3] Reuse connections for 5xx errors for HTTP/1 requests --- .../bugfix-AWSCRTHTTPClient-158f65a.json | 6 +++ .../bugfix-NettyNIOHTTPClient-edf8570.json | 6 +++ .../feature-ApacheHTTPClient-f0116e2.json | 6 +++ .../awssdk/http/apache/ApacheHttpClient.java | 2 - .../internal/SdkConnectionReuseStrategy.java | 49 ------------------- .../internal/response/CrtResponseAdapter.java | 2 +- ...reamAdaptingHttpStreamResponseHandler.java | 2 +- .../response/ResponseHandlerHelper.java | 9 ---- .../nio/netty/internal/ResponseHandler.java | 10 +--- .../UrlConnectionHttpClientWireMockTest.java | 5 -- ...ttpClientWithCustomCreateWireMockTest.java | 5 -- .../http/SdkAsyncHttpClientH1TestSuite.java | 4 +- .../awssdk/http/SdkHttpClientTestSuite.java | 4 +- 13 files changed, 25 insertions(+), 85 deletions(-) create mode 100644 .changes/next-release/bugfix-AWSCRTHTTPClient-158f65a.json create mode 100644 .changes/next-release/bugfix-NettyNIOHTTPClient-edf8570.json create mode 100644 .changes/next-release/feature-ApacheHTTPClient-f0116e2.json delete mode 100644 http-clients/apache-client/src/main/java/software/amazon/awssdk/http/apache/internal/SdkConnectionReuseStrategy.java diff --git a/.changes/next-release/bugfix-AWSCRTHTTPClient-158f65a.json b/.changes/next-release/bugfix-AWSCRTHTTPClient-158f65a.json new file mode 100644 index 000000000000..4515730bee4c --- /dev/null +++ b/.changes/next-release/bugfix-AWSCRTHTTPClient-158f65a.json @@ -0,0 +1,6 @@ +{ + "type": "bugfix", + "category": "AWS CRT HTTP Client", + "contributor": "", + "description": "Reuse connections that receive a 5xx service response." +} diff --git a/.changes/next-release/bugfix-NettyNIOHTTPClient-edf8570.json b/.changes/next-release/bugfix-NettyNIOHTTPClient-edf8570.json new file mode 100644 index 000000000000..cac8be6293ec --- /dev/null +++ b/.changes/next-release/bugfix-NettyNIOHTTPClient-edf8570.json @@ -0,0 +1,6 @@ +{ + "type": "bugfix", + "category": "Netty NIO HTTP Client", + "contributor": "", + "description": "Reuse connections that receive a 5xx service response." +} diff --git a/.changes/next-release/feature-ApacheHTTPClient-f0116e2.json b/.changes/next-release/feature-ApacheHTTPClient-f0116e2.json new file mode 100644 index 000000000000..2aa3e45d466d --- /dev/null +++ b/.changes/next-release/feature-ApacheHTTPClient-f0116e2.json @@ -0,0 +1,6 @@ +{ + "type": "bugfix", + "category": "Apache HTTP Client", + "contributor": "", + "description": "Reuse connections that receive a 5xx service response." +} diff --git a/http-clients/apache-client/src/main/java/software/amazon/awssdk/http/apache/ApacheHttpClient.java b/http-clients/apache-client/src/main/java/software/amazon/awssdk/http/apache/ApacheHttpClient.java index 5e0bf99478d2..b6ad70908496 100644 --- a/http-clients/apache-client/src/main/java/software/amazon/awssdk/http/apache/ApacheHttpClient.java +++ b/http-clients/apache-client/src/main/java/software/amazon/awssdk/http/apache/ApacheHttpClient.java @@ -75,7 +75,6 @@ import software.amazon.awssdk.http.TlsTrustManagersProvider; import software.amazon.awssdk.http.apache.internal.ApacheHttpRequestConfig; import software.amazon.awssdk.http.apache.internal.DefaultConfiguration; -import software.amazon.awssdk.http.apache.internal.SdkConnectionReuseStrategy; import software.amazon.awssdk.http.apache.internal.SdkProxyRoutePlanner; import software.amazon.awssdk.http.apache.internal.conn.ClientConnectionManagerFactory; import software.amazon.awssdk.http.apache.internal.conn.IdleConnectionReaper; @@ -157,7 +156,6 @@ private ConnectionManagerAwareHttpClient createClient(ApacheHttpClient.DefaultBu .disableRedirectHandling() .disableAutomaticRetries() .setUserAgent("") // SDK will set the user agent header in the pipeline. Don't let Apache waste time - .setConnectionReuseStrategy(new SdkConnectionReuseStrategy()) .setConnectionManager(ClientConnectionManagerFactory.wrap(cm)); addProxyConfig(builder, configuration); diff --git a/http-clients/apache-client/src/main/java/software/amazon/awssdk/http/apache/internal/SdkConnectionReuseStrategy.java b/http-clients/apache-client/src/main/java/software/amazon/awssdk/http/apache/internal/SdkConnectionReuseStrategy.java deleted file mode 100644 index ce9698b728c8..000000000000 --- a/http-clients/apache-client/src/main/java/software/amazon/awssdk/http/apache/internal/SdkConnectionReuseStrategy.java +++ /dev/null @@ -1,49 +0,0 @@ -/* - * Copyright Amazon.com, 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. - * A copy of the License is located at - * - * http://aws.amazon.com/apache2.0 - * - * or in the "license" file accompanying this file. This file 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 software.amazon.awssdk.http.apache.internal; - -import org.apache.http.HttpResponse; -import org.apache.http.impl.client.DefaultClientConnectionReuseStrategy; -import org.apache.http.protocol.HttpContext; -import software.amazon.awssdk.annotations.SdkInternalApi; - -/** - * Do not reuse connections that returned a 5xx error. - * - *

This is not strictly the behavior we would want in an AWS client, because sometimes we might want to keep a connection open - * (e.g. an undocumented service's 503 'SlowDown') and sometimes we might want to close the connection (e.g. S3's 400 - * RequestTimeout or Glacier's 408 RequestTimeoutException), but this is good enough for the majority of services, and the ones - * for which it is not should not be impacted too harshly. - */ -@SdkInternalApi -public class SdkConnectionReuseStrategy extends DefaultClientConnectionReuseStrategy { - @Override - public boolean keepAlive(HttpResponse response, HttpContext context) { - if (!super.keepAlive(response, context)) { - return false; - } - - if (response == null || response.getStatusLine() == null) { - return false; - } - - return !is500(response); - } - - private boolean is500(HttpResponse httpResponse) { - return httpResponse.getStatusLine().getStatusCode() / 100 == 5; - } -} diff --git a/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/response/CrtResponseAdapter.java b/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/response/CrtResponseAdapter.java index 89bb50ea99a4..d32c9e2ac228 100644 --- a/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/response/CrtResponseAdapter.java +++ b/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/response/CrtResponseAdapter.java @@ -129,7 +129,7 @@ private void onSuccessfulResponseComplete(HttpStream stream) { completionFuture.complete(null); }); - responseHandlerHelper.cleanUpConnectionBasedOnStatusCode(stream); + responseHandlerHelper.releaseConnection(stream); } private void handlePublisherError(HttpStream stream, Throwable failure) { diff --git a/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/response/InputStreamAdaptingHttpStreamResponseHandler.java b/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/response/InputStreamAdaptingHttpStreamResponseHandler.java index 28f859c2a7fc..ba3a91dcdeca 100644 --- a/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/response/InputStreamAdaptingHttpStreamResponseHandler.java +++ b/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/response/InputStreamAdaptingHttpStreamResponseHandler.java @@ -147,6 +147,6 @@ private void onSuccessfulResponseComplete(HttpStream stream) { // requestCompletionFuture has been completed at this point, no need to notify the future simplePublisher.complete(); - responseHandlerHelper.cleanUpConnectionBasedOnStatusCode(stream); + responseHandlerHelper.releaseConnection(stream); } } diff --git a/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/response/ResponseHandlerHelper.java b/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/response/ResponseHandlerHelper.java index 77adf7d1ceeb..f455469be937 100644 --- a/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/response/ResponseHandlerHelper.java +++ b/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/response/ResponseHandlerHelper.java @@ -88,13 +88,4 @@ public void closeConnection(HttpStream stream) { } } } - - public void cleanUpConnectionBasedOnStatusCode(HttpStream stream) { - // always close the connection on a 5XX response code. - if (HttpStatusFamily.of(responseBuilder.statusCode()) == HttpStatusFamily.SERVER_ERROR) { - closeConnection(stream); - } else { - releaseConnection(stream); - } - } } diff --git a/http-clients/netty-nio-client/src/main/java/software/amazon/awssdk/http/nio/netty/internal/ResponseHandler.java b/http-clients/netty-nio-client/src/main/java/software/amazon/awssdk/http/nio/netty/internal/ResponseHandler.java index eb3ecd09eb3c..a7e9a42a8cc4 100644 --- a/http-clients/netty-nio-client/src/main/java/software/amazon/awssdk/http/nio/netty/internal/ResponseHandler.java +++ b/http-clients/netty-nio-client/src/main/java/software/amazon/awssdk/http/nio/netty/internal/ResponseHandler.java @@ -56,7 +56,6 @@ import org.reactivestreams.Subscriber; import org.reactivestreams.Subscription; import software.amazon.awssdk.annotations.SdkInternalApi; -import software.amazon.awssdk.http.HttpStatusFamily; import software.amazon.awssdk.http.Protocol; import software.amazon.awssdk.http.SdkCancellationException; import software.amazon.awssdk.http.SdkHttpFullResponse; @@ -96,7 +95,7 @@ protected void channelRead0(ChannelHandlerContext channelContext, HttpObject msg .build(); channelContext.channel().attr(RESPONSE_STATUS_CODE).set(response.status().code()); channelContext.channel().attr(RESPONSE_CONTENT_LENGTH).set(responseContentLength(response)); - channelContext.channel().attr(KEEP_ALIVE).set(shouldKeepAlive(response)); + channelContext.channel().attr(KEEP_ALIVE).set(HttpUtil.isKeepAlive(response)); ChannelUtils.getAttribute(channelContext.channel(), CHANNEL_DIAGNOSTICS) .ifPresent(ChannelDiagnostics::incrementResponseCount); requestContext.handler().onHeaders(sdkResponse); @@ -203,13 +202,6 @@ private static void finalizeResponse(RequestContext requestContext, ChannelHandl } } - private boolean shouldKeepAlive(HttpResponse response) { - if (HttpStatusFamily.of(response.status().code()) == HttpStatusFamily.SERVER_ERROR) { - return false; - } - return HttpUtil.isKeepAlive(response); - } - @Override public void exceptionCaught(ChannelHandlerContext ctx, Throwable cause) throws Exception { RequestContext requestContext = ctx.channel().attr(REQUEST_CONTEXT_KEY).get(); diff --git a/http-clients/url-connection-client/src/test/java/software/amazon/awssdk/http/urlconnection/UrlConnectionHttpClientWireMockTest.java b/http-clients/url-connection-client/src/test/java/software/amazon/awssdk/http/urlconnection/UrlConnectionHttpClientWireMockTest.java index 05fdd709b28d..bf56e835829b 100644 --- a/http-clients/url-connection-client/src/test/java/software/amazon/awssdk/http/urlconnection/UrlConnectionHttpClientWireMockTest.java +++ b/http-clients/url-connection-client/src/test/java/software/amazon/awssdk/http/urlconnection/UrlConnectionHttpClientWireMockTest.java @@ -53,11 +53,6 @@ protected SdkHttpClient createSdkHttpClient(SdkHttpClientOptions options) { return builder.buildWithDefaults(attributeMap.build()); } - @Override - public void connectionsAreNotReusedOn5xxErrors() { - // We cannot support this because the URL connection client doesn't allow us to disable connection reuse - } - // https://bugs.openjdk.org/browse/JDK-8163921 @Test public void noAcceptHeader_shouldSet() throws IOException { diff --git a/http-clients/url-connection-client/src/test/java/software/amazon/awssdk/http/urlconnection/UrlConnectionHttpClientWithCustomCreateWireMockTest.java b/http-clients/url-connection-client/src/test/java/software/amazon/awssdk/http/urlconnection/UrlConnectionHttpClientWithCustomCreateWireMockTest.java index 02c0bd252b5d..e6ac8afe5546 100644 --- a/http-clients/url-connection-client/src/test/java/software/amazon/awssdk/http/urlconnection/UrlConnectionHttpClientWithCustomCreateWireMockTest.java +++ b/http-clients/url-connection-client/src/test/java/software/amazon/awssdk/http/urlconnection/UrlConnectionHttpClientWithCustomCreateWireMockTest.java @@ -60,11 +60,6 @@ public void testTrustAllWorks() { public void testCustomTlsTrustManagerAndTrustAllFails() { } - // Empty test; behavior not supported because the URL connection client does not allow disabling connection reuse - @Override - public void connectionsAreNotReusedOn5xxErrors() throws Exception { - } - @Test public void testGetResponseCodeNpeIsWrappedAsIo() throws Exception { connectionInterceptor = safeFunction(connection -> new DelegateHttpURLConnection(connection) { diff --git a/test/http-client-tests/src/main/java/software/amazon/awssdk/http/SdkAsyncHttpClientH1TestSuite.java b/test/http-client-tests/src/main/java/software/amazon/awssdk/http/SdkAsyncHttpClientH1TestSuite.java index d266426d05c0..497a2e02c1ff 100644 --- a/test/http-client-tests/src/main/java/software/amazon/awssdk/http/SdkAsyncHttpClientH1TestSuite.java +++ b/test/http-client-tests/src/main/java/software/amazon/awssdk/http/SdkAsyncHttpClientH1TestSuite.java @@ -88,13 +88,13 @@ public void teardown() throws InterruptedException { } @Test - public void connectionReceiveServerErrorStatusShouldNotReuseConnection() { + public void connectionReceiveServerErrorStatusShouldReuseConnection() { server.return500OnFirstRequest = true; server.closeConnection = false; HttpTestUtils.sendGetRequest(server.port(), client).join(); HttpTestUtils.sendGetRequest(server.port(), client).join(); - assertThat(server.channels.size()).isEqualTo(2); + assertThat(server.channels.size()).isEqualTo(1); } @Test diff --git a/test/http-client-tests/src/main/java/software/amazon/awssdk/http/SdkHttpClientTestSuite.java b/test/http-client-tests/src/main/java/software/amazon/awssdk/http/SdkHttpClientTestSuite.java index 6f26121a3794..71a8f5cc18ab 100644 --- a/test/http-client-tests/src/main/java/software/amazon/awssdk/http/SdkHttpClientTestSuite.java +++ b/test/http-client-tests/src/main/java/software/amazon/awssdk/http/SdkHttpClientTestSuite.java @@ -146,7 +146,7 @@ public void connectionPoolingWorks() throws Exception { } @Test - public void connectionsAreNotReusedOn5xxErrors() throws Exception { + public void connectionsAreReusedOn5xxErrors() throws Exception { int initialOpenedConnections = CONNECTION_COUNTER.openedConnections(); SdkHttpClientOptions httpClientOptions = new SdkHttpClientOptions(); @@ -169,7 +169,7 @@ public void connectionsAreNotReusedOn5xxErrors() throws Exception { // connection count increased by at least as many connections as we got 5xx errors back on. But the connection // manager also predictively creates connections and we need to take those into account in a way that lets it // remain a dynamic behavior. - assertThat(CONNECTION_COUNTER.openedConnections()).isGreaterThanOrEqualTo(initialOpenedConnections + 5); + assertThat(CONNECTION_COUNTER.openedConnections()).isGreaterThanOrEqualTo(initialOpenedConnections); } } From 0cd93224b54648ff346aafef3d89d17b2ede870d Mon Sep 17 00:00:00 2001 From: Zoe Wang <33073555+zoewangg@users.noreply.github.com> Date: Wed, 18 Sep 2024 10:41:10 -0700 Subject: [PATCH 2/3] Reuse HTTP/2 connections that receive a 5xx error --- .../internal/response/ResponseHandlerHelper.java | 1 - .../BaseHttpStreamResponseHandlerTest.java | 2 -- .../http2/Http2ToHttpInboundAdapter.java | 16 ---------------- .../http/nio/netty/fault/H2ServerErrorTest.java | 4 ++-- .../http/SdkAsyncHttpClientH1TestSuite.java | 5 ++++- 5 files changed, 6 insertions(+), 22 deletions(-) diff --git a/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/response/ResponseHandlerHelper.java b/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/response/ResponseHandlerHelper.java index f455469be937..6da44090aa5f 100644 --- a/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/response/ResponseHandlerHelper.java +++ b/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/response/ResponseHandlerHelper.java @@ -20,7 +20,6 @@ import software.amazon.awssdk.crt.http.HttpHeader; import software.amazon.awssdk.crt.http.HttpHeaderBlock; import software.amazon.awssdk.crt.http.HttpStream; -import software.amazon.awssdk.http.HttpStatusFamily; import software.amazon.awssdk.http.SdkHttpResponse; /** diff --git a/http-clients/aws-crt-client/src/test/java/software/amazon/awssdk/http/crt/internal/BaseHttpStreamResponseHandlerTest.java b/http-clients/aws-crt-client/src/test/java/software/amazon/awssdk/http/crt/internal/BaseHttpStreamResponseHandlerTest.java index d59e8e2cc2f3..8e2426cc5371 100644 --- a/http-clients/aws-crt-client/src/test/java/software/amazon/awssdk/http/crt/internal/BaseHttpStreamResponseHandlerTest.java +++ b/http-clients/aws-crt-client/src/test/java/software/amazon/awssdk/http/crt/internal/BaseHttpStreamResponseHandlerTest.java @@ -77,7 +77,6 @@ void serverError_shouldShutdownConnection() { responseHandler.onResponseHeadersDone(httpStream, 0); responseHandler.onResponseComplete(httpStream, 0); requestFuture.join(); - verify(crtConn).shutdown(); verify(crtConn).close(); verify(httpStream).close(); } @@ -121,7 +120,6 @@ void streamClosed_shouldNotIncreaseStreamWindow() throws InterruptedException { responseHandler.onResponseComplete(httpStream, 0); requestFuture.join(); - verify(crtConn).shutdown(); verify(crtConn).close(); verify(httpStream).close(); verify(httpStream, never()).incrementWindow(anyInt()); diff --git a/http-clients/netty-nio-client/src/main/java/software/amazon/awssdk/http/nio/netty/internal/http2/Http2ToHttpInboundAdapter.java b/http-clients/netty-nio-client/src/main/java/software/amazon/awssdk/http/nio/netty/internal/http2/Http2ToHttpInboundAdapter.java index a1a7d70509b2..03e7f9aec020 100644 --- a/http-clients/netty-nio-client/src/main/java/software/amazon/awssdk/http/nio/netty/internal/http2/Http2ToHttpInboundAdapter.java +++ b/http-clients/netty-nio-client/src/main/java/software/amazon/awssdk/http/nio/netty/internal/http2/Http2ToHttpInboundAdapter.java @@ -16,7 +16,6 @@ package software.amazon.awssdk.http.nio.netty.internal.http2; import io.netty.buffer.ByteBuf; -import io.netty.channel.Channel; import io.netty.channel.ChannelHandlerContext; import io.netty.channel.SimpleChannelInboundHandler; import io.netty.handler.codec.http.DefaultHttpContent; @@ -32,7 +31,6 @@ import io.netty.handler.codec.http2.HttpConversionUtil; import java.io.IOException; import software.amazon.awssdk.annotations.SdkInternalApi; -import software.amazon.awssdk.http.HttpStatusFamily; import software.amazon.awssdk.http.nio.netty.internal.utils.NettyClientLogger; /** @@ -62,20 +60,6 @@ private void onHeadersRead(Http2HeadersFrame headersFrame, ChannelHandlerContext HttpResponse httpResponse = HttpConversionUtil.toHttpResponse(headersFrame.stream().id(), headersFrame.headers(), true); ctx.fireChannelRead(httpResponse); - - if (HttpStatusFamily.of(httpResponse.status().code()) == HttpStatusFamily.SERVER_ERROR) { - fireConnectionExceptionForServerError(ctx); - } - } - - private void fireConnectionExceptionForServerError(ChannelHandlerContext ctx) { - if (ctx.channel().parent() != null) { - Channel parent = ctx.channel().parent(); - log.debug(ctx.channel(), - () -> "A 5xx server error occurred on an Http2 stream, notifying the connection channel " + ctx.channel()); - parent.pipeline().fireExceptionCaught(new Http2ConnectionTerminatingException("A 5xx server error occurred on an " - + "Http2 stream " + ctx.channel())); - } } private void onDataRead(Http2DataFrame dataFrame, ChannelHandlerContext ctx) throws Http2Exception { diff --git a/http-clients/netty-nio-client/src/test/java/software/amazon/awssdk/http/nio/netty/fault/H2ServerErrorTest.java b/http-clients/netty-nio-client/src/test/java/software/amazon/awssdk/http/nio/netty/fault/H2ServerErrorTest.java index 27b1ae52313c..9d1b57d42033 100644 --- a/http-clients/netty-nio-client/src/test/java/software/amazon/awssdk/http/nio/netty/fault/H2ServerErrorTest.java +++ b/http-clients/netty-nio-client/src/test/java/software/amazon/awssdk/http/nio/netty/fault/H2ServerErrorTest.java @@ -88,13 +88,13 @@ public void teardown() throws InterruptedException { } @Test - public void serviceReturn500_newRequestShouldUseNewConnection() { + public void serviceReturn500_newRequestShouldReuseConnection() { server.return500OnFirstRequest = true; CompletableFuture firstRequest = sendGetRequest(server.port(), netty); firstRequest.join(); sendGetRequest(server.port(), netty).join(); - assertThat(server.h2ConnectionCount.get()).isEqualTo(2); + assertThat(server.h2ConnectionCount.get()).isEqualTo(1); } @Test diff --git a/test/http-client-tests/src/main/java/software/amazon/awssdk/http/SdkAsyncHttpClientH1TestSuite.java b/test/http-client-tests/src/main/java/software/amazon/awssdk/http/SdkAsyncHttpClientH1TestSuite.java index 497a2e02c1ff..fc76f370ccf4 100644 --- a/test/http-client-tests/src/main/java/software/amazon/awssdk/http/SdkAsyncHttpClientH1TestSuite.java +++ b/test/http-client-tests/src/main/java/software/amazon/awssdk/http/SdkAsyncHttpClientH1TestSuite.java @@ -88,11 +88,14 @@ public void teardown() throws InterruptedException { } @Test - public void connectionReceiveServerErrorStatusShouldReuseConnection() { + public void connectionReceiveServerErrorStatusShouldReuseConnection() throws InterruptedException { server.return500OnFirstRequest = true; server.closeConnection = false; HttpTestUtils.sendGetRequest(server.port(), client).join(); + // The request-complete-future does not await the channel-release-future + // Wait a small amount to allow the channel release to complete + Thread.sleep(100); HttpTestUtils.sendGetRequest(server.port(), client).join(); assertThat(server.channels.size()).isEqualTo(1); } From 8a2492a49de9b731c5bd061d788dddcb87e711fd Mon Sep 17 00:00:00 2001 From: Zoe Wang <33073555+zoewangg@users.noreply.github.com> Date: Tue, 21 Jan 2025 19:47:24 -0800 Subject: [PATCH 3/3] Fix flaky tests --- .../awssdk/http/nio/netty/fault/H2ServerErrorTest.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/http-clients/netty-nio-client/src/test/java/software/amazon/awssdk/http/nio/netty/fault/H2ServerErrorTest.java b/http-clients/netty-nio-client/src/test/java/software/amazon/awssdk/http/nio/netty/fault/H2ServerErrorTest.java index 9d1b57d42033..8b640fb4226a 100644 --- a/http-clients/netty-nio-client/src/test/java/software/amazon/awssdk/http/nio/netty/fault/H2ServerErrorTest.java +++ b/http-clients/netty-nio-client/src/test/java/software/amazon/awssdk/http/nio/netty/fault/H2ServerErrorTest.java @@ -88,11 +88,14 @@ public void teardown() throws InterruptedException { } @Test - public void serviceReturn500_newRequestShouldReuseConnection() { + public void serviceReturn500_newRequestShouldReuseConnection() throws InterruptedException { server.return500OnFirstRequest = true; CompletableFuture firstRequest = sendGetRequest(server.port(), netty); firstRequest.join(); + // The request-complete-future does not await the channel-release-future + // Wait a small amount to allow the channel release to complete + Thread.sleep(100); sendGetRequest(server.port(), netty).join(); assertThat(server.h2ConnectionCount.get()).isEqualTo(1); }