New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[FLINK-19936] Make SinkITCase stable #13898
Conversation
The order of receiving the checkpoint complete is uncertain.(`Committer` receive first or `GlobalCommitter` receive first) This patch changes how to verifying the output of `GlobalCommitter` to resolve `SinkITCase`'s unstable owning to this uncertainty.
Thanks a lot for your contribution to the Apache Flink project. I'm the @flinkbot. I help the community Automated ChecksLast check on commit e9e477b (Tue Nov 03 06:38:13 UTC 2020) Warnings:
Mention the bot in a comment to re-run the automated checks. Review Progress
Please see the Pull Request Review Guide for a full explanation of the review process. 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 Bot commandsThe @flinkbot bot supports the following commands:
|
@guoweiM The build fails on AZP. Also could you explain a bit what the problem is so that I can understand what is the goal of the solution? |
From the solution, it seems that you think that the source of the instability is that the notification for the completion of the checkpoint does not come before the test exits, right? |
Yes. In the streaming execution mode the Another thing that makes the test unstable is the uncertain order in which the Following is an example that |
…/GlobalCommitter receive all the elements. This patch does two things 1. FiniteTestSource exits only after user provide condition is satisfied or time out. 2. All streaming mode ITCases exit only when the Committer/GlobalCommitter receive all the data or time out. fix compile error
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the work @guoweiM . LGTM, I will also let it run on my AZP and then merge when I have the green light.
What is the purpose of the change
In the streaming execution mode there are two uncertain things:
Committer
receive first orGlobalCommitter
receive first)This patch does following two things to resolve
SinkITCase
's unstable owning to these two uncertaintyThis pr does two things
GlobalCommitter
for the case 1FiniteTestSource
exit only when the Committer/GlobalCommitter received all the elements.Brief change log
GlobalCommitter
to resolveSinkITCase
's unstable owning to this uncertainty.Verifying this change
Does this pull request potentially affect one of the following parts:
@Public(Evolving)
: (no)Documentation