You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by GitBox <gi...@apache.org> on 2020/12/01 06:07:28 UTC
[GitHub] [flink] wangyang0918 opened a new pull request #14268: [FLINK-20428][test] Fix the unstable test ZooKeeperLeaderElectionConnectionHandlingTest#testConnectionSuspendedHandlingDuringInitialization
wangyang0918 opened a new pull request #14268:
URL: https://github.com/apache/flink/pull/14268
When the ZooKeeper connection is suspended, the `LeaderRetrievalEventHandler` will be notified with an empty leader information. In the failed test `ZooKeeperLeaderElectionConnectionHandlingTest.testConnectionSuspendedHandlingDuringInitialization`, we expect no leader will be notified. This is wrong. The reason why it could work is the timeout is set to very small(50ms) and it is too fast to get the leader information.
This PR will fix the unstable test via correcting the expectation.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #14268: [FLINK-20428][test] Fix the unstable test ZooKeeperLeaderElectionConnectionHandlingTest#testConnectionSuspendedHandlingDuringInitialization
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #14268:
URL: https://github.com/apache/flink/pull/14268#issuecomment-736254887
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "00f90b8b5cffb48983182b4d530a6158061037a4",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=10362",
"triggerID" : "00f90b8b5cffb48983182b4d530a6158061037a4",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 00f90b8b5cffb48983182b4d530a6158061037a4 Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=10362)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [flink] wangyang0918 commented on pull request #14268: [FLINK-20428][test] Fix the unstable test ZooKeeperLeaderElectionConnectionHandlingTest#testConnectionSuspendedHandlingDuringInitialization
Posted by GitBox <gi...@apache.org>.
wangyang0918 commented on pull request #14268:
URL: https://github.com/apache/flink/pull/14268#issuecomment-736246075
cc @XComp @tillrohrmann Could you please have a look?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [flink] wangyang0918 commented on pull request #14268: [FLINK-20428][test] Fix the unstable test ZooKeeperLeaderElectionConnectionHandlingTest#testConnectionSuspendedHandlingDuringInitialization
Posted by GitBox <gi...@apache.org>.
wangyang0918 commented on pull request #14268:
URL: https://github.com/apache/flink/pull/14268#issuecomment-736479379
@tillrohrmann Thanks for the review. I have addressed the comments.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [flink] flinkbot commented on pull request #14268: [FLINK-20428][test] Fix the unstable test ZooKeeperLeaderElectionConnectionHandlingTest#testConnectionSuspendedHandlingDuringInitialization
Posted by GitBox <gi...@apache.org>.
flinkbot commented on pull request #14268:
URL: https://github.com/apache/flink/pull/14268#issuecomment-736245974
Thanks a lot for your contribution to the Apache Flink project. I'm the @flinkbot. I help the community
to review your pull request. We will use this comment to track the progress of the review.
## Automated Checks
Last check on commit 00f90b8b5cffb48983182b4d530a6158061037a4 (Tue Dec 01 06:10:09 UTC 2020)
**Warnings:**
* No documentation files were touched! Remember to keep the Flink docs up to date!
* **This pull request references an unassigned [Jira ticket](https://issues.apache.org/jira/browse/FLINK-20428).** According to the [code contribution guide](https://flink.apache.org/contributing/contribute-code.html), tickets need to be assigned before starting with the implementation work.
<sub>Mention the bot in a comment to re-run the automated checks.</sub>
## Review Progress
* ❓ 1. The [description] looks good.
* ❓ 2. There is [consensus] that the contribution should go into to Flink.
* ❓ 3. Needs [attention] from.
* ❓ 4. The change fits into the overall [architecture].
* ❓ 5. Overall code [quality] is good.
Please see the [Pull Request Review Guide](https://flink.apache.org/contributing/reviewing-prs.html) for a full explanation of the review process.<details>
The Bot is tracking the review progress through labels. Labels are applied according to the order of the review items. For consensus, approval by a Flink committer of PMC member is required <summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot approve description` to approve one or more aspects (aspects: `description`, `consensus`, `architecture` and `quality`)
- `@flinkbot approve all` to approve all aspects
- `@flinkbot approve-until architecture` to approve everything until `architecture`
- `@flinkbot attention @username1 [@username2 ..]` to require somebody's attention
- `@flinkbot disapprove architecture` to remove an approval you gave earlier
</details>
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #14268: [FLINK-20428][test] Fix the unstable test ZooKeeperLeaderElectionConnectionHandlingTest#testConnectionSuspendedHandlingDuringInitialization
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #14268:
URL: https://github.com/apache/flink/pull/14268#issuecomment-736254887
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "00f90b8b5cffb48983182b4d530a6158061037a4",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=10362",
"triggerID" : "00f90b8b5cffb48983182b4d530a6158061037a4",
"triggerType" : "PUSH"
}, {
"hash" : "6afa53e362117ed9caddb4432f95fe23e22b4f7b",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=10384",
"triggerID" : "6afa53e362117ed9caddb4432f95fe23e22b4f7b",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 6afa53e362117ed9caddb4432f95fe23e22b4f7b Azure: [SUCCESS](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=10384)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [flink] flinkbot commented on pull request #14268: [FLINK-20428][test] Fix the unstable test ZooKeeperLeaderElectionConnectionHandlingTest#testConnectionSuspendedHandlingDuringInitialization
Posted by GitBox <gi...@apache.org>.
flinkbot commented on pull request #14268:
URL: https://github.com/apache/flink/pull/14268#issuecomment-736254887
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "00f90b8b5cffb48983182b4d530a6158061037a4",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "00f90b8b5cffb48983182b4d530a6158061037a4",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 00f90b8b5cffb48983182b4d530a6158061037a4 UNKNOWN
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [flink] XComp commented on pull request #14268: [FLINK-20428][test] Fix the unstable test ZooKeeperLeaderElectionConnectionHandlingTest#testConnectionSuspendedHandlingDuringInitialization
Posted by GitBox <gi...@apache.org>.
XComp commented on pull request #14268:
URL: https://github.com/apache/flink/pull/14268#issuecomment-736544042
I went through the code and have nothing to add. Thanks @wangyang0918
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #14268: [FLINK-20428][test] Fix the unstable test ZooKeeperLeaderElectionConnectionHandlingTest#testConnectionSuspendedHandlingDuringInitialization
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #14268:
URL: https://github.com/apache/flink/pull/14268#issuecomment-736254887
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "00f90b8b5cffb48983182b4d530a6158061037a4",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=10362",
"triggerID" : "00f90b8b5cffb48983182b4d530a6158061037a4",
"triggerType" : "PUSH"
}, {
"hash" : "6afa53e362117ed9caddb4432f95fe23e22b4f7b",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=10384",
"triggerID" : "6afa53e362117ed9caddb4432f95fe23e22b4f7b",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 00f90b8b5cffb48983182b4d530a6158061037a4 Azure: [SUCCESS](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=10362)
* 6afa53e362117ed9caddb4432f95fe23e22b4f7b Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=10384)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #14268: [FLINK-20428][test] Fix the unstable test ZooKeeperLeaderElectionConnectionHandlingTest#testConnectionSuspendedHandlingDuringInitialization
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #14268:
URL: https://github.com/apache/flink/pull/14268#issuecomment-736254887
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "00f90b8b5cffb48983182b4d530a6158061037a4",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=10362",
"triggerID" : "00f90b8b5cffb48983182b4d530a6158061037a4",
"triggerType" : "PUSH"
}, {
"hash" : "6afa53e362117ed9caddb4432f95fe23e22b4f7b",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "6afa53e362117ed9caddb4432f95fe23e22b4f7b",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 00f90b8b5cffb48983182b4d530a6158061037a4 Azure: [SUCCESS](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=10362)
* 6afa53e362117ed9caddb4432f95fe23e22b4f7b UNKNOWN
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [flink] tillrohrmann commented on a change in pull request #14268: [FLINK-20428][test] Fix the unstable test ZooKeeperLeaderElectionConnectionHandlingTest#testConnectionSuspendedHandlingDuringInitialization
Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #14268: [FLINK-20428][test] Fix the unstable test ZooKeeperLeaderElectionConnectionHandlingTest#testConnectionSuspendedHandlingDuringInitialization
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #14268:
URL: https://github.com/apache/flink/pull/14268#issuecomment-736254887
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "00f90b8b5cffb48983182b4d530a6158061037a4",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=10362",
"triggerID" : "00f90b8b5cffb48983182b4d530a6158061037a4",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 00f90b8b5cffb48983182b4d530a6158061037a4 Azure: [SUCCESS](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=10362)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [flink] tillrohrmann closed pull request #14268: [FLINK-20428][test] Fix the unstable test ZooKeeperLeaderElectionConnectionHandlingTest#testConnectionSuspendedHandlingDuringInitialization
Posted by GitBox <gi...@apache.org>.
tillrohrmann closed pull request #14268:
URL: https://github.com/apache/flink/pull/14268
----------------------------------------------------------------
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:
users@infra.apache.org