Repository navigation
[#36841] Spark 4 streaming: windowed GroupByKey and Combine via transformWithState - #40703
Conversation
… transformWithState
|
R: @Abacn |
There was a problem hiding this comment.
📋 Review Summary
This PR cleanly implements windowed GroupByKey, Combine.perKey, and Combine.globally().withoutDefaults() for the Spark 4 Structured Streaming runner using transformWithState. The refactoring of BeamStatefulProcessor into a shared abstract base class for ParDoStatefulProcessor and GroupByKeyStatefulProcessor is well-structured, preserves state/timer/watermark cleanup guarantees, and is backed by thorough end-to-end streaming and translation tests.
🔍 General Feedback
- The separation of
BeamStatefulProcessor,ParDoStatefulProcessor, andGroupByKeyStatefulProcessoreffectively shares state, timer, and per-key watermark management without duplicating code. - Firing due event-time timers at the end of
handleInputRowsafter advancing the watermark is a sound improvement, withreconcileWakeupTimerproperly cleaning up Spark timers to prevent duplicate firing. - Unsupported windowing and trigger configurations (merging windows, custom triggers, accumulating panes, and
Combine.globally()with defaults) are guarded with clear error messages and tested inPipelineTranslatorStreamingTest.
💡 Tip: Comment /gemini review on this pull request to trigger a new Gemini code review.
|
Stopping reviewer notifications for this pull request: review requested by someone other than the bot, ceding control. If you'd like to restart, comment |
| protected void processElements( | ||
| K key, Iterator<WindowedValue<KV<K, V>>> rows, SparkTimerInternals timerInternals) { | ||
| DoFnRunner<KeyedWorkItem<K, V>, KV<K, Iterable<V>>> r = runner(); | ||
| checkStateNotNull(stepContext).set(stateInternals(key), timerInternals); |
There was a problem hiding this comment.
Both processElement and processTimers invoke stepContext.stateInternals(key), in addition to GroupAlsoByWindowViaWindowSetNewDoFn.create() in GroupByKeyStatefulProcessor.java:L107. Is this duplicated SparkStateInternals creation per key?
There was a problem hiding this comment.
Yes, it built several views over the same MapState per visit. Done in 4a7beef: each handler call builds one SparkStateInternals and passes it to the hooks, and the GroupAlsoByWindow factory reads it from the step context (k -> ctx.stateInternals()).
…eInternals per visit
Part of #36841.
This adds windowed GroupByKey to the Spark 4 Structured Streaming runner and lets
Combine.perKeyandCombine.globally().withoutDefaults()expand onto it.Combine.globally()with a default value, custom triggers, accumulating fired panes and merging windows are rejected at translation with a message naming the issue. Accumulating panes are deferred scope: with the default trigger they only matter for late panes inside allowed lateness, they follow with trigger support.BeamStatefulProcessorbecomes an abstract base that keeps the state, timer and per key watermark lag code from #40447 unchanged, decodes the encoded key once and calls two hooks.ParDoStatefulProcessoris the stateful ParDo code moved out of it.GroupByKeyStatefulProcessorhosts aGroupAlsoByWindowViaWindowSetNewDoFnwithSystemReduceFn.bufferingbehindDoFnRunners.lateDataDroppingRunner, fed oneKeyedWorkItemper visit for elements and one for due timers. Due timers fire in one pass, with the default trigger and non merging windowsReduceFnRunner.onTimerssets no timer that is already due.GroupByKeyStreamingTranslatorgroups by encoded key bytes withEncoders.BINARY()like the stateful ParDo translator since #40516, so the null key of theCombine.globallyexpansion works.One behavior change for stateful ParDo: due event time timers now also fire at the end of
handleInputRowsinstead of only inhandleExpiredTimer. The wake up reconciliation removes the Spark timer, so nothing fires twice.WindowedGroupByKeyStreamingTestruns six live cases (fixed windows, sliding windows withCount.perKey,Combine.globallywithout defaults, late data dropped with pane timing asserted, a stateful ParDo timer output chained into GroupByKey, and an expired key that leaves no rows inbeamState,beamTimersandbeamWatermark). The late data and chained cases fail when the per key watermark lag is disabled, the expired key case fails without the cleanup from #40550, which the base class now applies to GroupByKey keys too. ThestateRowshelper from #40550 moves toStreamingTestUtils.PipelineTranslatorStreamingTestgains a table row per new guard. Full:runners:spark:4:testand:runners:spark:3:testpass locally, and the SparkStructuredStreaming ValidatesRunner trigger file is touched.The CHANGES.md entry for Spark 4 streaming follows with the Impulse and PAssert slice that enables the streaming ValidatesRunner suite.