XComp commented on code in PR #22987:
URL: https://github.com/apache/flink/pull/22987#discussion_r1269401834
##########
flink-runtime/src/test/java/org/apache/flink/runtime/rest/RestClientTest.java:
##########
@@ -207,6 +209,42 @@ public void testRestClientClosedHandling() throws
Exception {
}
}
+ /**
+ * Tests that the futures returned by {@link RestClient} fail immediately
if the client is
+ * already closed.
+ *
+ * <p>See FLINK-32583
+ */
+ @Test
+ public void testCloseClientBeforeRequest() throws Exception {
+ // Note that the executor passed to the RestClient constructor is not
the same as the
+ // executor used by Netty
+ try (final RestClient restClient =
+ new RestClient(new Configuration(),
Executors.directExecutor())) {
+ // Intentionally close the client (and thus also the executor used
by Netty)
+ restClient.close();
+
+ CompletableFuture<?> future =
+ restClient.sendRequest(
+ unroutableIp,
+ 80,
+ new TestMessageHeaders(),
+ EmptyMessageParameters.getInstance(),
+ EmptyRequestBody.getInstance());
+
+ // Call get() on the future with a timeout of 0 so we can test
that the exception thrown
+ // is not a TimeoutException, which is what would be thrown if
restClient were not
+ // already closed
+ final ThrowingRunnable getFuture = () -> future.get(0,
TimeUnit.SECONDS);
+
+ final ExecutionException executionException =
+ assertThrows(ExecutionException.class, getFuture);
+ final Throwable throwable =
ExceptionUtils.stripExecutionException(executionException);
+ assertThat(throwable, instanceOf(IOException.class));
+ assertThat(throwable.getMessage(), containsString("RestClient is
closed"));
Review Comment:
You could change the other test(s) in a hotfix commit...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
