[FLINK-15914][checkpointing][metrics] Miss the barrier alignment metric for the case of two inputs#11019
Conversation
|
Thanks a lot for your contribution to the Apache Flink project. I'm the @flinkbot. I help the community Automated ChecksLast check on commit 012203b (Wed Feb 05 09:14:40 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. DetailsThe 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:
|
pnowojski
left a comment
There was a problem hiding this comment.
I would be fine with merging it as it is to release 1.10.
For master I would prefer the comments to be addressed and maybe add a test that metric is present? But it would be nice for at leat the production code be the same for 1.10 and master.
...k-streaming-java/src/main/java/org/apache/flink/streaming/runtime/io/InputProcessorUtil.java
Outdated
Show resolved
Hide resolved
...k-streaming-java/src/main/java/org/apache/flink/streaming/runtime/io/InputProcessorUtil.java
Show resolved
Hide resolved
|
@zhijiangW if you think it's better to have this minimal change for release-1.10 and do the more proper fixes for master +1 from my side. |
…ic for the case of two inputs When the StreamTwoInputSelectableProcessor was introduced before, it was missing to add the barrier alignment metric in the constructor. But it does not cause problems then, because only StreamTwoInputProcessor works at that time. After StreamTwoInputProcessor is replaced by StreamTwoInputSelectableProcessor as now, this bug is exposed and we will not see the barrier alignment metric for the case of two inputs. The solution is to add this metric while constructing the CheckpointBarrierHandler.
012203b to
ac23472
Compare
|
Thanks for the review @pnowojski . I already submitted the updates. |
pnowojski
left a comment
There was a problem hiding this comment.
LGTM, let's think about deduplication and test in the master version.
Let's also wait for green travis before merging.
|
The failing python build should be unrelated. See FLINK-15921. +1 for merging. |
What is the purpose of the change
When the
StreamTwoInputSelectableProcessorwas introduced before, it was missing to add the barrier alignment metric in the constructor. But it does not cause problems then, because onlyStreamTwoInputProcessorworks at that time. AfterStreamTwoInputProcessoris replaced byStreamTwoInputSelectableProcessoras now, this bug is exposed and we will not see the barrier alignment metric for the case of two inputs.The solution is to add this metric while constructing the
CheckpointBarrierHandler.Brief change log
CheckpointBarrierHandlerVerifying this change
Via the job testing.
Does this pull request potentially affect one of the following parts:
@Public(Evolving): (yes / no)Documentation