Skip to content

Commit c2c4b47

Browse files
committed
Revert the change to offload metrics reporting and remove false comment
1 parent e0d8e10 commit c2c4b47

File tree

2 files changed

+5
-10
lines changed

2 files changed

+5
-10
lines changed

core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/http/pipeline/stages/MakeAsyncHttpRequestStage.java

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -174,11 +174,10 @@ private CompletableFuture<Void> doExecuteHttpRequest(RequestExecutionContext con
174174
long callStart = System.nanoTime();
175175
CompletableFuture<Void> httpClientFuture = sdkAsyncHttpClient.execute(executeRequest);
176176

177-
// Offload the metrics reporting from this stage onto the future completion executor
178-
CompletableFuture<Void> result = httpClientFuture.whenCompleteAsync((r, t) -> {
177+
CompletableFuture<Void> result = httpClientFuture.whenComplete((r, t) -> {
179178
long duration = System.nanoTime() - callStart;
180179
metricCollector.reportMetric(CoreMetric.SERVICE_CALL_DURATION, Duration.ofNanos(duration));
181-
}, futureCompletionExecutor);
180+
});
182181

183182
// Make sure failures on the result future are forwarded to the http client future.
184183
CompletableFutureUtils.forwardExceptionTo(result, httpClientFuture);

test/protocol-tests/src/test/java/software/amazon/awssdk/protocol/tests/AsyncResponseThreadingTest.java

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@
2222
import static org.assertj.core.api.AssertionsForClassTypes.assertThat;
2323
import static org.assertj.core.api.AssertionsForClassTypes.assertThatThrownBy;
2424
import static org.mockito.Matchers.any;
25-
import static org.mockito.Mockito.times;
2625
import static org.mockito.Mockito.verify;
2726
import static software.amazon.awssdk.core.client.config.SdkAdvancedAsyncClientOption.FUTURE_COMPLETION_EXECUTOR;
2827

@@ -68,8 +67,7 @@ public void completionWithNioThreadWorksCorrectly() {
6867
client.streamingOutputOperation(StreamingOutputOperationRequest.builder().build(),
6968
AsyncResponseTransformer.toBytes()).join();
7069

71-
// #1 reporting metrics, #2 completing response
72-
verify(mockExecutor, times(2)).execute(any());
70+
verify(mockExecutor).execute(any());
7371

7472
byte[] arrayCopy = response.asByteArray();
7573
assertThat(arrayCopy).containsExactly('t', 'e', 's', 't');
@@ -95,8 +93,7 @@ public void connectionError_completionWithNioThreadWorksCorrectly() {
9593
AsyncResponseTransformer.toBytes()).join())
9694
.hasCauseInstanceOf(SdkClientException.class);
9795

98-
// #1 reporting metrics, #2 completing response
99-
verify(mockExecutor, times(2)).execute(any());
96+
verify(mockExecutor).execute(any());
10097
}
10198

10299
@Test
@@ -117,8 +114,7 @@ public void serverError_completionWithNioThreadWorksCorrectly() {
117114
assertThatThrownBy(() ->
118115
client.streamingOutputOperation(StreamingOutputOperationRequest.builder().build(),
119116
AsyncResponseTransformer.toBytes()).join()).hasCauseInstanceOf(ProtocolRestJsonException.class);
120-
// #1 reporting metrics, #2 completing response
121-
verify(mockExecutor, times(2)).execute(any());
117+
verify(mockExecutor).execute(any());
122118
}
123119

124120
private static class SpyableExecutor implements Executor {

0 commit comments

Comments
 (0)