nsivabalan commented on PR #20516:
URL: https://github.com/apache/hudi/pull/20516#issuecomment-6644868416
@danny0405 a suggestion on the shape of the guard. Can we model it on how
`HoodieIncrSource` (HoodieStreamer's Hudi-to-Hudi source) already handles a
missing or expired checkpoint, so both Hudi-on-Hudi incremental paths behave
and are configured the same way?
**What HoodieIncrSource does today**
- **No checkpoint.** It fails by default.
`hoodie.streamer.source.hoodieincr.missing.checkpoint.strategy` opts in to a
starting point:
- `READ_UPTO_LATEST_COMMIT`: snapshot-read everything up to the latest
commit, then continue incrementally;
- `READ_LATEST`: start incrementally from the latest commit and skip
history.
- **Checkpoint before the active timeline (archived).** With
`READ_UPTO_LATEST_COMMIT`, it switches to a snapshot query filtered to the
pending instants (`HoodieIncrSource`, `shouldFullScan`).
- **Files removed by the cleaner.** The incremental relation either falls
back to a range-filtered full table scan
(`hoodie.datasource.read.incr.fallback.fulltablescan.enable`) or fails with
`HoodieIncrementalPathNotFoundException`.
- **Large snapshot loads.** These can be chunked with
`SnapshotLoadQuerySplitter`
(`hoodie.deltastreamer.snapshotload.query.splitter.class.name`).
**What the streaming source does today:**
`hoodie.datasource.streaming.startOffset` (earliest/latest/instant) covers only
the first start. For a saved offset that has been archived or cleaned,
`MergeOnReadIncrementalRelationV2` falls back silently to a full table scan,
because `fallback.fulltablescan.enable` defaults to `true`. The user never
learns that history expired.
**Suggestion**
1. **Fail by default** when the saved offset is no longer readable
incrementally (archived, or required files cleaned) and when the upstream was
reset (overwrite, truncate or recreation). This is the HoodieIncrSource
no-strategy behaviour. The error names the config to set.
2. **One recovery strategy config instead of a boolean**, with values
mirroring `MissingCheckpointStrategy`:
- `FAIL` (default);
- `READ_UPTO_LATEST_COMMIT`: full snapshot read of the upstream as the
next batch, then continue incrementally from the snapshot's end instant;
- `READ_LATEST`: jump to the latest commit and continue incrementally,
accepting the gap.
3. **Reset, truncate and recreation stay fail-by-default.** The downstream
table already holds rows derived from the old upstream, so no automatic choice
is safe. The user has to intervene: either full-refresh the downstream, or
deliberately set `READ_UPTO_LATEST_COMMIT` to re-snapshot and continue. With an
upsert downstream that leaves keys that no longer exist upstream, and with an
insert/append downstream it duplicates rows. The docs should say so next to the
config.
4. **Keep the snapshot fallback basic for now:** a single full snapshot
batch. Scaling it with a batched or mini-batch bootstrap (along the lines of
`SnapshotLoadQuerySplitter`, bounded by AvailableNow admission control) can be
a follow-up.
This also settles the default question for the existing
`fallback.fulltablescan.enable=true` behaviour on the streaming path. When the
new strategy config is set it takes precedence, and the default moves from
"silent full scan" to "fail with an actionable error". That's a behaviour
change for existing structured-streaming readers, so it needs a release note.
Alternatively, we could gate the new default to SDP if we'd rather not change
plain readStream users yet. Thoughts?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
