[spark] Support time-range incremental batch reads - #3883
Conversation
80165a8 to
be893fe
Compare
|
@YannByron @fresh-borzoni @luoyuxia @beryllw PTAL, this is an actual customer requirement. |
YannByron
left a comment
There was a problem hiding this comment.
Thanks for the PR — the feature direction makes sense and the docs are unusually thorough. I traced the PK changelog-fold semantics (snapshotId = -1 -> reader skips the snapshot -> SortMergeReader drops delete rows) down through FlussUpsertPartitionReader and SortMergeReader, and it does behave as described.
I have three must-fix findings before this lands, all on the incremental upsert path plus the option parsing. Details are inline; summary:
read.optimized=truecombined with an incremental PK read silently returns zero rows, with no error. This is the one I would call a genuine bug.failOnTimestampOutOfRangeuses.getandEnumeration.withName, and is evaluated eagerly for every batch read — so a bad value of an incremental-only option breaks plain full-table reads with a bareNoSuchElementException.createIncrementalUpsertPartitionsis missing the empty-range guard that the append path gained in this same PR, so empty buckets each still spin up a Spark task and a Fluss connection.
I also collected some non-blocking notes (short-circuiting the lake-snapshot probe in incremental mode, end-side handling versus the server's strict ts > now rejection in Replica#getOffsetByTimestamp, the scan.incremental.* naming versus Fluss's existing scan.startup.* vocabulary, and the Thread.sleep-based test timing). I left those out to keep this review focused — happy to post them separately if useful.
There was a problem hiding this comment.
Pull request overview
Adds time-range incremental batch reads to the Fluss Spark connector, exposing the feature both as per-query scan options and as a SQL table-valued function (TVF), with documented semantics for log / primary-key / lake-enabled tables.
Changes:
- Introduces
scan.incremental.{start,end}.timestamp+scan.incremental.timestamp.out-of-rangeand wires them through split planning for append/upsert tables (including retention guard behavior). - Adds SQL TVF
fluss_incremental_between_timestamp(table, start[, end])viaFlussSparkSessionExtensions, translating TVF arguments into per-relation scan options. - Adds unit/integration tests and updates Spark connector docs to describe time-range batch read semantics and option scoping.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| website/docs/engine-spark/structured-streaming.md | Clarifies streaming startup-mode limitation and points users to incremental batch reads for bounded windows. |
| website/docs/engine-spark/reads.md | Documents time-range batch reads, TVF usage, DataFrame API options, and retention/window validation semantics. |
| website/docs/engine-spark/options.md | Splits session-level options vs per-query read options and documents new incremental scan options. |
| fluss-spark/fluss-spark-ut/src/test/scala/org/apache/fluss/spark/SparkTimeRangeTvfTest.scala | End-to-end TVF coverage for log/pk/partitioned tables, option scoping, and timestamp literal forms. |
| fluss-spark/fluss-spark-ut/src/test/scala/org/apache/fluss/spark/read/FlussOffsetInitializersTest.scala | Unit tests for incremental option parsing, window validation, and retention-guard decisions. |
| fluss-spark/fluss-spark-ut/src/test/scala/org/apache/fluss/spark/lake/SparkLakeTimeRangeReadTest.scala | Integration coverage ensuring incremental reads on lake-enabled tables stay Fluss-only (no lake/kv snapshot union). |
| fluss-spark/fluss-spark-common/src/main/scala/org/apache/fluss/spark/SparkFlussConf.scala | Adds config options/constants/enums for incremental batch reads. |
| fluss-spark/fluss-spark-common/src/main/scala/org/apache/fluss/spark/read/SplitPlanner.scala | Implements incremental planning branches, retention guard hooks, and empty-range handling. |
| fluss-spark/fluss-spark-common/src/main/scala/org/apache/fluss/spark/read/FlussOffsetInitializers.scala | Adds incremental option parsing, timestamp parsing, window validation, and retention-guard helpers. |
| fluss-spark/fluss-spark-common/src/main/scala/org/apache/fluss/spark/read/FlussMicroBatchStream.scala | Updates stopping-offset initializer wiring for streaming. |
| fluss-spark/fluss-spark-common/src/main/scala/org/apache/fluss/spark/FlussSparkSessionExtensions.scala | Registers the TVF resolver and injects supported TVFs into Spark extensions. |
| fluss-spark/fluss-spark-common/src/main/scala/org/apache/fluss/spark/catalyst/plans/logical/FlussTableValuedFunctions.scala | Implements TVF logical plan + argument normalization into scan options and resolution to a V2 relation. |
| fluss-spark/fluss-spark-common/src/main/scala/org/apache/fluss/spark/catalyst/analysis/FlussTableValuedFunctionResolver.scala | Analyzer rule to rewrite unresolved TVF nodes into DataSourceV2Relation. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
fresh-borzoni
left a comment
There was a problem hiding this comment.
@Yohahaha Thank you for the PR, left some commments, PTAL
|
@luoyuxia @beryllw @fresh-borzoni PTAL! |
fresh-borzoni
left a comment
There was a problem hiding this comment.
@Yohahaha Thank you, left one comment, PTAL
| } | ||
| val startOffset = Long2long(startBucketOffsets.get(Integer.valueOf(bucketId))) | ||
| val stopOffset = Long2long(stoppingBucketOffsets.get(Integer.valueOf(bucketId))) | ||
| if (startOffset >= stopOffset) { |
There was a problem hiding this comment.
This can't fire anymore stopOffset is always latest, so an empty window still gets a partition.
If there's data after the window the start offset lands past the end, the reader keeps nothing, and LogChangesIterator throws NoSuchElementException on .head.
There was a problem hiding this comment.
thank you for catching this real bug! fixed with return empty iter when log records is empty.
Add timestamp-bounded batch reads to the Spark connector so downstream pipelines can incrementally read rows written within a [t1, t2) window: - scan.startup.mode=timestamp + scan.startup.timestamp (inclusive start) - scan.bounded.mode=timestamp + scan.bounded.timestamp (exclusive end, defaults to latest committed data at planning time) - Log tables return raw records in the window; primary key tables return keys inserted/updated in the window folded to their latest value - Out-of-range start fails fast by default; scan.startup.timestamp.out-of-range=adjust clamps to earliest retained data - Default behavior unchanged (scan.startup.mode=full)
…-range tests Blank scan.incremental.* values now count as unset, so a whitespace-only start timestamp no longer enables an incremental read. Test cleanups: merge the redundant datetime-expression TVF case into the timestamp arguments case, drop the future-end case (server-side validation), slim the retention-guard message test, and replace the weak option-scoping case with a session-configuration negative test. Co-Authored-By: Qoder <noreply@qoder.com> AI-Model: Qoder Auto AI-Contributed/Feature: 7/7 AI-Contributed/UT: 81/81
…ables Cover -U/+U, +I/-D, -D/+I and pure -D folding within the time-range window for primary key tables, including partitioned PK tables. Each test first asserts the raw changelog really contains the claimed change types, so the folded-output assertions cannot pass vacuously.
Address review feedback on the time-range incremental read: - failOnTimestampOutOfRange falls back to the option default for blank values and rejects unknown modes with an IllegalArgumentException listing the supported values, instead of a bare NoSuchElementException. The planner call sites become lazy vals so the incremental-only option never breaks plain batch reads. - The incremental upsert planner skips buckets whose resolved [start, stop) range is empty, mirroring the append planner guard, so empty windows cost no Spark task, Fluss connection or RPC. - The incremental upsert planner fails fast when read.optimized is enabled: the combination has no snapshot to read and would silently return zero rows. Co-Authored-By: Qoder <noreply@qoder.com> AI-Model: Qoder Auto AI-Contributed/Feature: 51/51 AI-Contributed/UT: 54/54
… windows Interpret TIMESTAMP_NTZ TVF arguments in the Spark session time zone (matching the string form and the Flink connector convention) instead of pinning them to UTC, and reject invalid window specifications instead of silently changing semantics: a blank start timestamp, an end timestamp set without a start timestamp, and a window whose start is not strictly before its end all fail fast at planning time.
…ading A time-range batch read now returns whatever Fluss still retains inside the requested window instead of failing when the start predates retention: the result is always a genuine subset of the window, so the extra earliest-offset lookup and the scan.incremental.timestamp.out-of-range option paid for nothing. Both bounds are resolved while the statement is analyzed, filling an omitted end with the current timestamp, so the window no longer shifts between analysis and planning. The scan positions itself with the start offset but stops at the latest offset, and the reader cuts the window on each record's commit timestamp, which keeps [start, end) exact on segments whose time index is sparse and avoids sending a driver-computed "now" to the server. Co-Authored-By: Qoder <noreply@qoder.com> AI-Model: Qoder Auto AI-Contributed/Feature: 225/233 AI-Contributed/UT: 250/250
An incremental read scans up to the latest offset and applies its end bound on the record commit timestamp while reading, so a non-empty offset range can still leave zero records behind: this happens whenever the window itself is empty but data was committed after it, because the start timestamp then resolves to a record past the end bound. The upsert reader handed that empty batch to LogChangesIterator, which seeds its cursor from the first record and threw NoSuchElementException. Return an empty iterator instead, and correct the planner comment that claimed its start >= stop guard already ruled this out. Cover it in the TVF suite, which now asserts several adjacent windows per table, empty leading, gap and trailing windows, a delete-only window that folds to nothing, a primary key table spread over three buckets, and an explicitly triggered kv snapshot that a window must never serve. Co-Authored-By: Qoder <noreply@qoder.com> AI-Model: Qoder Auto AI-Contributed/Feature: 8/16 AI-Contributed/UT: 195/195
ba8f59e to
2883e7d
Compare
The incremental branch marked its bucket splits as log-only by passing a bare -1 as the snapshot id, and the lake time-range test compared against the same magic number. Reuse TableBucketSnapshot.NO_SNAPSHOT_ID on both sides so the check reads the same way as every other snapshot guard in the codebase, and update the block comment along with it. Co-Authored-By: Qoder <noreply@qoder.com> AI-Model: Qoder Auto AI-Contributed/Feature: 7/7 AI-Contributed/UT: 5/5
Summary
Support time-range incremental batch reads in the Spark connector, so pipelines can read only the data written within a
[start, end)window (start inclusive, end exclusive) — the building block for an hourly/daily incremental job.New TVF
fluss_incremental_between_timestamp(table, start[, end]), registered viaFlussSparkSessionExtensions:The table argument accepts
table,database.tableorcatalog.database.table. The bound arguments accept a string (epoch millis oryyyy-MM-dd HH:mm:ss), an integral epoch-millis value, aDATE(start of that day) or aTIMESTAMP/TIMESTAMP_NTZ, all in the Spark session time zone, and may come from any constant expression (column references are rejected). Both bounds are resolved during analysis — omittingendfills in Spark'scurrent_timestamp()— so the window is pinned by the statement: rows committed while the query is planned stay out of it, and re-executing the same relation reads the same window.New options backing the TVF, also usable from the DataFrame API. They are per-query read options and deliberately not read from session configuration, so a window can never leak into later reads; they are batch-only and ignored by streaming:
scan.incremental.start.timestampscan.incremental.end.timestampRead semantics per table type:
Exactness of the window. The start bound positions the scan through the server's timestamp-to-offset lookup, but the scan stops at the latest offset and both bounds are then applied to each record's commit timestamp by the reader (
FlussTimeRange, carried on the input partition), terminating the partition at the first record at or afterend. That keeps[start, end)exact even for data already tiered to remote storage, where a timestamp lookup is only as accurate as the sparse server-side time index — and it avoids sending a driver-computed "now" to the server, which would be rejected as a future timestamp under clock skew.Retention. A window reaching further back than what Fluss still retains (
table.log.ttl) is not an error: the dropped part simply yields fewer rows, or none. The result is always a genuine subset of the requested window, so a caller never sees foreign data — there is no separate out-of-range mode to configure, and no extralistOffsets(EARLIEST)round trip at planning time.Invalid windows fail fast instead of silently changing semantics: a blank or unparseable timestamp, an end without a start, and (in the TVF, at analysis time) a start that is not strictly before the end.
Also:
scan.startup.modeis documented as affecting streaming reads only; plain batch reads remain full-table, so default behavior is unchanged.SET spark.sql.fluss.*values were merged into the catalog-wide configuration shared by every table, instead of a per-scan copy.FlussScan: [fluss.t], Type: [Append] [TimeRange: [1767225600000, 1767312000000)].website/docs/engine-spark/reads.md(new "Time-Range Batch Read" section) andoptions.md.Fixes #3842
Test Plan
FlussOffsetInitializersTest: option gating, blank/malformed values, end-without-start, and the time range handed to the reader.SparkTimeRangeTvfTest(partitioned tables by default): log-table window incl. the pinned upper bound, primary key changelog folding, non-partitioned log/pk tables, equivalence of all timestamp argument forms, invalid usage, and that bounds are never taken from session configuration.SparkLakeTimeRangeReadTest: log and pk tables skip the lake/kv snapshot.mvn spotless:checkpasses on the affected modules.🤖 AI-assisted changes - reviewed by human developer