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:
[email protected]
