pnowojski commented on code in PR #27638:
URL: https://github.com/apache/flink/pull/27638#discussion_r2839639867
##########
flink-runtime/src/main/java/org/apache/flink/streaming/api/operators/SourceOperator.java:
##########
@@ -781,6 +782,13 @@ public void updateCurrentEffectiveWatermark(long
watermark) {
public void updateCurrentSplitWatermark(String splitId, long watermark) {
WatermarkSampler splitWatermarkSampler =
checkNotNull(sampledSplitWatermarks.get(splitId));
splitWatermarkSampler.addLatest(watermark);
+ if (!currentlyIdleSplits.contains(splitId)) {
+ maybePauseSplit(splitId);
+ }
+ }
+
+ private void maybePauseSplit(String splitId) {
Review Comment:
I think the `maybePauseSplit` decsribes pretty well what's happening. Also
there are plenty of other cases with similar naming convention (method named
`maybeACTION`).
Re `updateCurrentSplitPausedWatermark`:
- I presume you ment `updateCurrentPausedSplitWatermark`, but in this case
"Paused watermark" is still confusing as pointed by @Efrat19 . Even more
correct would have been `updateCurrentlyPausedSplit`...
- but `updateCurrentlyPausedSplit` doesn't capture that currently paused
split split might not be updated
- and `updateCurrentlyPausedSplit` is basically as synonym of `pauseSplits`
So:
- `maybePauseSplit` and `maybeUpdateCurrentlyPausedSplit` are both
technically correct `maybePauseSplits` imo sounds better, so I would keep it as
is.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
