[Prism] Honor the resume delay of self-checkpointing SDF residuals - #39849
[Prism] Honor the resume delay of self-checkpointing SDF residuals#39849Eliaaazzz wants to merge 1 commit into
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #39849 +/- ##
=========================================
Coverage 59.69% 59.69%
+ Complexity 16818 16813 -5
=========================================
Files 2863 2863
Lines 297877 297972 +95
Branches 14679 14679
=========================================
+ Hits 177805 177875 +70
- Misses 112468 112486 +18
- Partials 7604 7611 +7
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:
|
|
Checks are failing. Will not request review until checks are succeeding. If you'd like to override that behavior, comment |
|
Run GoPrism PreCommit |
|
Run Java_PVR_Prism_Loopback PreCommit |
|
Assigning reviewers: R: @shunping 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). |
Fixes #39848.
The bug
PersistBundledecoded the SDK's requested resume delay intoengine.Residual.Delayand then returned every residual to the pending queue immediately, at the TODO "actually reschedule based on the residuals delay". A polling SDF therefore busy-spins: on a master build of Prism, a PythonWatchpoll loop requesting 3 second pacing viadefer_remainderexecuted 7,034 poll rounds in 9 seconds of wall time.The change
PersistBundlepartitions residuals by delay. A residual with no delay returns to pending as before. A delayed residual is parked on its stage instageState.delayedResiduals, keyed by the processing time it becomes schedulable, and that time is scheduled through the machinery processing-time timers already use:ptRefreshes→em.processTimeEvents.Schedule→em.wakeUpAt. The watermark evaluation loop releases due parked residuals back to pending before checking bundle readiness.Parked elements pin the stage's input watermark by being included in
minPendingTimestampLocked, the same way pending elements do. An earlier revision usedwatermarkHoldsinstead; those clamp only the output watermark, and since the input watermark is monotonic, a bounded pipeline's global-window aggregation could fire early with partial data once the input watermark ran ahead during a park. TheTestSeparationProcessContinuations subtests catch exactly this and pass with the final design.Under the fast-forward clock (
EnableRTC=false),processingTimeNowpeeks the event queue, so delayed residuals still fire immediately in synthetic time and test pipelines stay fast. Under the default real-time clock the delay is honored;--experiments=prism_disable_rtcremains the opt-out. The ProcessContinuations subtests ofTestSeparationnow take real time (~15s instead of ~4s locally) because the 1 second resume delays they request are honored.Verification
TestPersistBundle_ResidualResumeDelayengine tests: a delayed residual is parked rather than pending, pinsminPendingTimestampLocked, and schedules a processing time event; under a real-time clock it fires only after the delay; under the fast-forward clock it fires without real waiting. Reverting the parking fails the first two subtests.runners/prism/...Go tests pass.MatchContinuouslypipeline that previously missed files added mid-run on Prism now passes;Watchpipelines in both deduplication modes still pass.Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
addresses #123), if applicable. This will automatically add a link to the pull request in the issue. If you would like the issue to automatically close on merging the pull request, commentfixes #<ISSUE NUMBER>instead.CHANGES.mdwith noteworthy changes.See the Contributor Guide for more tips on how to make review process smoother.