tillrohrmann commented on a change in pull request #14268: URL: https://github.com/apache/flink/pull/14268#discussion_r533296691
########## File path: flink-runtime/src/test/java/org/apache/flink/runtime/leaderelection/ZooKeeperLeaderElectionConnectionHandlingTest.java ########## @@ -281,6 +282,10 @@ public void notifyLeaderAddress(LeaderInformation leaderInformation) { } public CompletableFuture<String> next() { + return next(timeout); Review comment: Can we clean up the `timeout` field of the `QueueLeaderElectionListener`? I think it does not make sense that the leader election listener has such a field. ########## File path: flink-runtime/src/test/java/org/apache/flink/runtime/leaderelection/ZooKeeperLeaderElectionConnectionHandlingTest.java ########## @@ -102,9 +102,10 @@ public void testConnectionSuspendedHandlingDuringInitialization() throws Excepti closeTestServer(); - CompletableFuture<String> secondAddress = queueLeaderElectionListener.next(); - assertThat("No result is expected since there was no leader elected before stopping the server, yet.", - secondAddress, is(nullValue())); + // QueueLeaderElectionListener will be notified with an empty leader when ZK connection is suspended + final CompletableFuture<String> secondAddress = queueLeaderElectionListener.next(Duration.ofSeconds(3)); Review comment: If we are expecting an empty leader information, then a timeout is not required here. ########## File path: flink-runtime/src/test/java/org/apache/flink/runtime/leaderelection/ZooKeeperLeaderElectionConnectionHandlingTest.java ########## @@ -281,6 +282,10 @@ public void notifyLeaderAddress(LeaderInformation leaderInformation) { } public CompletableFuture<String> next() { + return next(timeout); + } + + public CompletableFuture<String> next(Duration timeout) { Review comment: ```suggestion public CompletableFuture<String> next(@Nullable Duration timeout) { ``` ########## File path: flink-runtime/src/test/java/org/apache/flink/runtime/leaderelection/ZooKeeperLeaderElectionConnectionHandlingTest.java ########## @@ -281,6 +282,10 @@ public void notifyLeaderAddress(LeaderInformation leaderInformation) { } public CompletableFuture<String> next() { + return next(timeout); Review comment: Also, `notifyLeaderAddress` should probably wait until it can add the `leaderInformation` to the `queue`. ---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org