XComp commented on code in PR #20829:
URL: https://github.com/apache/flink/pull/20829#discussion_r972680499
##########
flink-rpc/flink-rpc-akka/src/test/java/org/apache/flink/runtime/rpc/akka/AkkaRpcServiceTest.java:
##########
@@ -371,7 +352,29 @@ private Collection<CompletableFuture<Void>>
startStopNCountingAsynchronousOnStop
countDownLatch.await();
Review Comment:
I cannot mark the two `assert` calls above, but I'm wondering whether we
should move them underneath the `countDownLatch.await` call. :thinking:
Don't we want to test that the RpcService is not terminated, yet, even after
stop is called on all Endpoints? That is the case when the `countDownLatch` is
triggered and its `await` leaves the blocking state.
##########
flink-rpc/flink-rpc-akka/src/test/java/org/apache/flink/runtime/rpc/akka/AkkaRpcServiceTest.java:
##########
@@ -305,24 +291,19 @@ void testAkkaRpcServiceShutDownWithFailingRpcEndpoints()
throws Exception {
final int numberActors = 5;
- CompletableFuture<Void> terminationFuture =
akkaRpcService.getTerminationFuture();
-
- final Collection<CompletableFuture<Void>> onStopFutures =
+ RpcServiceShutdownTestHelper rpcServiceShutdownTestHelper =
startStopNCountingAsynchronousOnStopEndpoints(akkaRpcService,
numberActors);
- Iterator<CompletableFuture<Void>> iterator = onStopFutures.iterator();
+ Iterator<CompletableFuture<Void>> iterator =
Review Comment:
```suggestion
final Iterator<CompletableFuture<Void>> iterator =
```
nit
##########
flink-rpc/flink-rpc-akka/src/test/java/org/apache/flink/runtime/rpc/akka/AkkaRpcServiceTest.java:
##########
@@ -305,24 +291,19 @@ void testAkkaRpcServiceShutDownWithFailingRpcEndpoints()
throws Exception {
final int numberActors = 5;
- CompletableFuture<Void> terminationFuture =
akkaRpcService.getTerminationFuture();
-
- final Collection<CompletableFuture<Void>> onStopFutures =
+ RpcServiceShutdownTestHelper rpcServiceShutdownTestHelper =
Review Comment:
```suggestion
final RpcServiceShutdownTestHelper rpcServiceShutdownTestHelper =
```
nit
##########
flink-rpc/flink-rpc-akka/src/test/java/org/apache/flink/runtime/rpc/akka/AkkaRpcServiceTest.java:
##########
@@ -279,16 +266,15 @@ void testAkkaRpcServiceShutDownWithRpcEndpoints() throws
Exception {
try {
final int numberActors = 5;
- CompletableFuture<Void> terminationFuture =
akkaRpcService.getTerminationFuture();
-
- final Collection<CompletableFuture<Void>> onStopFutures =
+ RpcServiceShutdownTestHelper rpcServiceShutdownTestHelper =
Review Comment:
```suggestion
final RpcServiceShutdownTestHelper rpcServiceShutdownTestHelper =
```
nit
--
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]
