kezhuw commented on a change in pull request #15241:
URL: https://github.com/apache/flink/pull/15241#discussion_r600750211
##########
File path:
flink-streaming-java/src/main/java/org/apache/flink/streaming/runtime/operators/windowing/WindowOperator.java
##########
@@ -466,9 +466,12 @@ public void onEventTime(InternalTimer<K, W> timer) throws
Exception {
mergingWindows = getMergingWindowSet();
W stateWindow =
mergingWindows.getStateWindow(triggerContext.window);
if (stateWindow == null) {
- // Timer firing for non-existent window, this can only happen
if a
- // trigger did not clean up timers. We have already cleared
the merging
- // window and therefore the Trigger state, however, so nothing
to do.
+ // Timer firing for non-existent window, this can happen:
Review comment:
I guess there are different. The sentence you quoted describes more
about window and trigger state clearing. Timers could be treated as part of
trigger state depends on perspective. However, in merging case, these timers
could be left over due to bookkeeping state merged out in `onMerge`, eg. there
are not cleared. This is why we need the short-circuit return statement: the
window firing timer belongs to is merged and cleared but the timer remains
until firing.
It is my personal understanding. Anyway, I will revert it first. We could
add a note back if required in later reviewing.
--
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]
