[Spark][#36841] Add the DataSourceV2 unbounded source for the Spark 4 streaming runner - #39971
[Spark][#36841] Add the DataSourceV2 unbounded source for the Spark 4 streaming runner#39971tkaymak wants to merge 3 commits into
Conversation
Exposes any Beam UnboundedSource as a Spark 4 DataSourceV2 streaming table with a fixed two column schema, encoded payload plus event timestamp. Offsets are opaque, strictly increasing epoch counters, so Spark keeps scheduling micro-batches and termination stays with the lifecycle owner. Recovery is durable under the query's checkpoint location: the source id derives deterministically from the read transform's full name, the first run pins its split list (Beam sources do not guarantee deterministic splitting), and every split persists its CheckpointMark per epoch with a retention of two, written atomically via temp file and rename. Executors cache live readers between micro-batches and fall back to the newest durable mark at or before the replayed epoch after a restart. Semantics are at least once, a crash between finishing a read and Spark's commit replays the last micro-batch. The batch cutoff honors maxRecordsPerBatch, values below 1, including the default, mean no limit and the batch ends on the duration deadline.
|
Assigning reviewers: R: @tvalentyn added as fallback since no labels match configuration 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). |
|
Thank you for the review @Abacn! |
Marks are no longer finalized when a partition reader closes. A reader finalizes the mark taken at its start offset when the next micro-batch for that split is scheduled, because Spark only starts a batch at the initial offset or at the end offset of a batch already in its commit log. A reader whose position does not match the start offset, or that moved without completing its batch (task retry, killed attempt, executor change, restart), is dropped without finalizing and recreated from the durable mark at that offset. Per source state lives under the checkpoint location Spark hands to toMicroBatchStream, written through CheckpointFileManager with the session Hadoop configuration broadcast to executors. Marks are coded with the source's checkpoint mark coder. commit(end) purges marks below end on a background thread, nothing is retained by a fixed count. The dataset is built like BoundedDatasetFactory, a Table holding the real objects wrapped in StreamingRelationV2, no string options, no Base64. Splits travel as objects in the InputPartition, options and Hadoop configuration as broadcasts. maxRecordsPerBatch is divided across splits like the legacy MicrobatchSource, defaultParallelism decides the split count, idle readers back off with FluentBackoff, offsets serialize as the bare epoch like LongOffset with the base class equality. A new option readerIdleTimeoutMillis bounds how long an executor keeps an idle reader. Tests drive the reader cache protocol directly and prove restart recovery, finalization only after commit, and mark purging against Spark's real offsets and commits logs. The JUnit per test timeout is removed from the streaming test, its throwaway thread group poisoned Spark's static pools for later batch tests in the same JVM.
|
@Abacn thanks for the thorough review, the failing tests seem to be unrelated (the setup environment step fails, because GitHub's org policy rejects the pinned SHA), will have a look at that later if I can.
|
…atch source Brings the final file states of the spark4-streaming-poc branch onto the head of the slice 3 rework (apache#39971) as one commit: the streaming pipeline translator and evaluation context, the Read, Impulse, GroupByKey and stateful ParDo translators, the transformWithState state and timer bridge, and the end to end streaming tests. The io/streaming package of the rework is kept as is, the POC's own version of it is dropped. The end of stream sentinel the POC had added to the old source is re-applied on the reworked BeamPartitionReader and BeamReaderCache: a batch that holds data ends at the first empty poll so its watermark is declared first, and an exhausted reader whose watermark reached the end of the global window emits one empty payload row at the maximum timestamp once per cached reader. The translators filter that row. Callers of the removed int maxRecordsPerMicroBatch option now use the long maxRecordsPerBatch option of master, whose per batch quota is split across the splits with at least one record per split, which is what the tests relied on. StreamingCheckpointRestartTest asserts the reworked checkpoint layout, splits and marks under the per source location Spark hands the stream, instead of the old beam-source-<id> directory found by a recursive search. The JUnit method timeouts are removed from every streaming test. Its timeout thread group leaks into Spark's static pools and breaks later tests in the same JVM. StreamingTestUtils gains run and waitUntilFinish helpers with a five minute deadline that cancel the pipeline and fail the test instead. SparkSessionFactory, build.gradle, the pipeline options, result, runner, evaluation context and pipeline translator of the shared base need no change, the merged slices already carry the POC's deltas including the RocksDB state store default and the Kryo registrations.
Abacn
left a comment
There was a problem hiding this comment.
Thanks for addressing the Round 1 comments and migrating to CheckpointFileManager and StreamingRelationV2.
However, the PR diff has expanded from ~1,000 to over 3,300 lines (with tests now comprising over 55% of the code). Much of this inflation comes from duplicate test fixtures, over-engineered scheduling heuristics, and white-box protocol testing:
-
Rendezvous Hashing & Private Spark API: sortedExecutors() calls SparkEnv.get().blockManager().master().getPeers(). This is a private Spark internal API that is brittle across cluster managers (K8s, YARN, standalone) and dynamic allocation. Spark's scheduler already handles task placement, and fallback recovery from durable marks is already required and implemented. We can simply remove the rendezvous hashing and murmur3 logic and return empty preferredLocations().
-
Mock Source Duplication: We currently have 3 separate synthetic unbounded sources (ListSource, ShardedListSource, and IntListSource) spanning ~500 lines across test files. ShardedListSource and IntListSource are practically identical (custom non-serializable marks, static maps for tracking finalizations, custom coders). Using existing Beam test sources is preferred, or consolidate them into a single reusable test source.
-
Prune White-Box Unit Tests: BeamReaderCacheProtocolTest (478 lines) and BeamSourceCheckpointTest (209 lines) test internal bookkeeping without Spark. Meanwhile, BeamMicroBatchSourceTest already tests recovery, finalization, and purging against a live Spark query. We can significantly cut down maintenance overhead by pruning these unit tests and relying on integration tests.
-
Consolidate POJO Classes: Compare with BoundedDatasetFactory.java, which contains the entire bounded source implementation, having 10 separate files for streaming (BeamSourceSpec, BeamPartitionReaderFactory, BeamStreamingTable, etc.) likely introduced unnecessary boilerplate.
Other comments --- need to verify if it's factually correct
-
Deferred Finalization Lifecycle Edge Cases: In BeamReaderCache, deferring mark finalization until the next batch's acquire() means:
- The final micro-batch on graceful query shutdown is never finalized.
- An idle reader eviction (closeIdle()) drops the reader without finalizing the pending mark. We should ensure marks are finalized on clean query termination and idle close.
-
splitQuotas Distribution Bug: In BeamMicroBatchStream.splitQuotas, using Math.max(1L, ...) causes queries where maxRecordsPerBatch < numSplits to emit more records than the configured limit (e.g. 20 records for limit of 10). Please adopt the quota splitting logic from MicrobatchSource.splitNumRecords where partitions with 0 quota emit 0 records (using < 0 for unlimited).
| purgeFloors.put(splitId, epoch); | ||
| return; | ||
| } | ||
| for (long e = floor; e < epoch; e++) { |
There was a problem hiding this comment.
It issues individual synchronous delete() RPCs for every epoch. On cloud object stores like GCS/S3, this can result in hundreds of sequential HTTP calls on every commit. Use directory listing or batch deletions instead.
| fastForwardEpoch(endEpoch); | ||
| List<UnboundedSource<T, ?>> pinned = splits(); | ||
| long[] quotas = splitQuotas(spec.maxRecordsPerBatch(), pinned.size()); | ||
| List<String> executors = sortedExecutors(); |
There was a problem hiding this comment.
(from AI reivew) sortedExecutors uses .master().getPeers(...) and assumes naming format of Spark internals, both are unsupported and fragile in managed environments. We already support restoring from durable marks when a partition runs on another executor, so we should let Spark handle partition locality.
Third slice of the Spark 4 Structured Streaming work split out of #39576, following the dispatch seam (#39906) and the Kryo registrations (#39939). Addresses #36841.
This adds the DataSourceV2 micro-batch source that exposes any Beam UnboundedSource as a Spark 4 streaming table. The only change outside the new package is one option on
SparkStructuredStreamingPipelineOptions.Design notes:
BoundedDatasetFactory, aTableholding the source, coder and broadcasts wrapped inStreamingRelationV2. No string options, splits travel as objects inside theInputPartition, pipeline options and the session Hadoop configuration as broadcasts.LongOffset.latestOffsetalways advances so Spark keeps scheduling micro-batches, termination belongs to the lifecycle owner.toMicroBatchStream, written throughCheckpointFileManager. The first run pins its split list because Beam sources do not guarantee deterministic splitting. Each split writes its CheckpointMark, coded withgetCheckpointMarkCoder(), at the end of every micro-batch under its end epoch.commit(end)purges marks belowend.PubsubCheckpointthrows on a restored checkpoint andKafkaCheckpointMarkis a no-op without its reader, and DSv2 has no executor side commit callback.spark.speculationis not supported for sources with non deterministic reads.maxRecordsPerBatchis a per batch total divided across splits like the legacyMicrobatchSource, values below 1 mean no limit and the batch ends on themaxBatchDurationMillisdeadline.defaultParallelismdecides the desired split count. Idle readers back off withFluentBackoff. The new optionreaderIdleTimeoutMillisbounds how long an executor keeps an idle reader, its last mark is not finalized when it is closed.Tests cover element delivery, watermark tracking through typed maps, the offset round trip, the quota division, the reader cache protocol driven directly (continuation, retry, executor change, missing mark, never started reader, failed mark write), the checkpoint layout, and, against Spark's real offsets and commits logs, restart recovery with at most one replayed batch, finalization only after commit, and mark purging.
Remaining slices: the state and timer bridge on transformWithState, then the translators with the end to end tests. End to end evidence remains in draft #39576.
R: @Abacn