fatmanverse commented on PR #11487: URL: https://github.com/apache/seatunnel/pull/11487#issuecomment-5032419535
Thanks @SEZ9 for the thorough follow-up review. I've addressed all three non-blocking issues in commit `628fadb4c`. **Issue 1 — cross-restore `startup.mode` switch (fail-fast)** I traced the runtime path and found the bounded-contract violation has two distinct entry points, so I added fail-fast at both, plus a defense-in-depth check: - `IncrementalSource.restoreEnumerator()` now rejects restoring an `IncrementalPhaseState` when `startup.mode = snapshot`. This covers a job originally started with `earliest`/`latest`/`specific`/`timestamp` (which checkpoints an `IncrementalPhaseState`) being restored as snapshot. - `IncrementalSourceReader.addSplits()` now rejects an incremental (binlog) split under snapshot mode. This is the actual "streams forever" path: a job that ran `initial`, completed the snapshot, entered the binlog phase and checkpointed, then restarts with `startup.mode = snapshot` — the enumerator state is clean/bounded, but the **reader** restores an `IncrementalSplit` and would stream binlog indefinitely. Since `IncrementalPhaseState` is an empty marker and the incremental split state actually lives in the reader, the reader-side guard is essential rather than optional. - `HybridSplitAssigner.addSplits()` also rejects an incremental split under snapshot mode as defense-in-depth. New tests: `IncrementalSourceTest#testSnapshotOnlyRestoreRejectsIncrementalCheckpoint` (enumerator) and `HybridSplitAssignerTest#testSnapshotOnlyRejectsRestoredIncrementalSplit` (assigner). Docs now state that changing `startup.mode` across a restore is not supported. **Issue 2 — `exactly_once` doc contract** You're right that the wording wrongly narrowed the contract. I confirmed via `ConfigValidator` that the `.conditional(STARTUP_MODE, [INITIAL, SNAPSHOT], EXACTLY_ONCE)` rule never rejects `exactly_once` at runtime (it has a default of `false`, so it is never treated as absent). The description now keeps the general statement and adds the snapshot-phase note, in both en and zh: > Enable exactly once semantics. When `startup.mode` is `initial` or `snapshot`, it additionally enables bounded low-to-high-watermark binlog backfill during the snapshot phase. **Issue 3 — snapshot-only example `server-id`** Changed the example to a range (`"5656-5657"`) matching the E2E, and added a note that `exactly_once` backfill opens a replication connection per reader, so a `server-id` range covering the job parallelism is required. Fixed in both en and zh. **Validation** - `./mvnw spotless:apply` — clean - `connector-cdc-base` compiles clean (main + test) - Targeted tests pass: `IncrementalSourceTest` (3), `HybridSplitAssignerTest` (4) -- 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]
