Repository navigation
[Prism] Drop elements for windows whose trigger has finished - #40878
Conversation
Once a window's root trigger has finished, Prism kept accepting new elements and emitted them in an extra pane at end of window. Drop them instead, as Java's ReduceFnRunner and Python's trigger driver do. - Re-enable the TestTrigger* and TestPanes integration tests on Prism. - Move TriggerElementCount and TriggerOrFinally to TestImplemented. - Expect only 6.0 from TriggerAfterProcessingTime: the trigger fires once, then 4.0 arrives in the closed window and is dropped.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #40878 +/- ##
===========================================
+ Coverage 0 56.40% +56.40%
===========================================
Files 0 876 +876
Lines 0 154798 +154798
===========================================
+ Hits 0 87319 +87319
- Misses 0 65617 +65617
- Partials 0 1862 +1862
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
AfterCount(3) isn't repeated, so it finishes after its first firing and closes the window. Prism now drops later elements for that window instead of emitting them in an extra pane.
|
Yeah, this is great to have narrowed down! Especially the documentation update in the next PR. The bots are very good extensions of oneself if you already have plenty of information to start. Glad they're working out for Beam! |
|
Assigning reviewers: R: @lostluck for label go. Note: If you would like to opt out of this review, comment Available commands:
The PR bot will only process comments in the main thread (not review comments). |
Summary
In Beam, when a window's root trigger finishes (it will never fire again), the window is closed. The firing that finished the trigger is the final pane, no more panes are emitted, and any elements that arrive later are dropped.
Prism didn't do this. It kept buffering elements for a closed window and emitted them in an extra pane when the watermark passed the end of the window. This PR makes Prism drop those elements, matching the Java and Python runners, and re-enables the trigger integration tests on Prism.
Related: #31153
Changes
1. Prism: drop elements for closed windows (
runners/prism/internal/engine/elementmanager.go)stageState.isTriggerFinished(window, key)reports whether the root trigger for a window and key has finished.aggregateStageKind.addPendinguses it to drop elements for closed windows. The check runs per element, so in streaming mode an element that finishes the trigger causes later elements in the same batch to be dropped.Observed panes from Prism before and after the fix:
TriggerElementCount:AfterCount(2), fixed 10s windows[0,10): {1,2}, {3,4,5};[50,60): {10} → 3 panesTriggerOrFinally:OrFinally(Always, AfterCount(2))[0,10): {1}, {2}, {3};[10,20): {5}, {8} → 5 panesTriggerAfterProcessingTime:AfterProcessingTime(+5s), not repeated, global window2. Tests
test/integration/integration.go: removed the Prism filters forTestTrigger.*andTestPanes. They now all pass on Prism.runners/prism/internal/unimplemented_test.go: movedTriggerElementCountandTriggerOrFinallyfromTestUnimplementedtoTestImplemented.test/integration/primitives/windowinto.go:TriggerAfterProcessingTimenow expects only6.0. The trigger isn't repeated, so it finishes after its first firing;4.0then arrives in the closed window and is dropped. No other runner runs this test: direct, portable, spark and dataflow filterTestTrigger.*, and flink filters it explicitly.Why this is the expected behavior
See #40879 for details.