DanielLeens commented on PR #11988: URL: https://github.com/apache/seatunnel/pull/11988#issuecomment-6538227403
Synced `apache/dev` again and fixed the unit-test failures of the previous head. New head: `925e7d3963a36ba8465d8bb042ba72dacd07faf3` (merge commit `b967427dbaa` + follow-up `925e7d3963a`). GitHub now reports the PR as mergeable. **Merge with `dev` (`3ac2d1cec6a`)** Seven files conflicted; all resolved by keeping both sides: - `IncrementalSplit.java`, `IncrementalSplitState.java`, `IncrementalSourceStreamFetcher.java`: `dev` added its own per-table offset map (`tableStartupOffsets`, the positions restored tables had already been read up to, dropped once the split reads past them) in the same places this branch added the mixed-startup lower bounds (`tableStartOffsets`, fixed per job). Both maps are kept, with a canonical 9-argument `IncrementalSplit` constructor (`IncrementalSplit.java:226`) that the two 8-argument constructors, the copy constructor, `pruneTables()` and `IncrementalSplitState#toSourceSplit()` delegate to. In `configureFilter()` the mixed bounds are seeded first (`IncrementalSourceStreamFetcher.java:328`), so `dev`'s loop keeps them for tables without snapshot splits and a restored offset can only raise them; `dev` also removed the old "latest-offset mode" overwrite, which closes Issue 3 of my 09-19 review as a side effect. - `IncrementalSplitAssignerTest.java`: one `TestOffset` helper (dev's `long` variant), both sides' tests kept. `IncrementalSplitStateTest.java`: all three new tests kept. - `docs/en|zh/connectors/source/MySQL-CDC.md`: the `mixed` rows plus `dev`'s new wording/default for `startup.specific-offset.skip-events|skip-rows`. **Root cause of the unit-test failures on `ec87c8fcff3`** (`MySqlIncrementalSourceFactoryTest` 6F/4E, `MySqlIncrementalSourceStartupConfigTest` 3E, all with `OptionsOptionWithConstraint 'startup.specific-offset.file' duplicate in 'ConditionalRequiredOptions'`): the previous conflict resolution kept `startup.specific-offset.file|pos` inside the `mixed` conditional block while `dev` (#12864-style guards) now declares the same options once as guarded optional options; `OptionRule.Builder#optional(option, condition)` rejects an option that already sits in a conditional block (`OptionRule.java:548-571`), so every `optionRule()` call threw and every E2E lane failed at factory validation. Fix (no test assertion touched): - `MySqlIncrementalSourceFactory.java:110-118`: the `mixed` conditional only requires `exactly_once` and `startup.snapshot-table-names`; the offset anchors stay declared once at `:131-146`. - `MySqlSpecificOffsetGuards.java:86-118, 150`: the guards accept `startup.mode = mixed` next to `specific` (`usesSpecificOffsets`), so a mixed job still must configure `file` + `pos` together and other modes are still rejected with the same "can only be used when" messages that `MySqlIncrementalSourceFactoryTest` asserts. This matches the runtime message already used in `MySqlIncrementalSource#createStartupConfig`. **Status of the open items from the 09-19 review** - Issue 1 (E2E for `mixed`): present since the previous head, `MysqlCDCSpecificStartingOffsetIT#testMysqlCdcMixedStartupWithRestore` (`MysqlCDCSpecificStartingOffsetIT.java:495`) with `mysqlcdc_mixed_startup.conf`; it covers fresh start semantics for a snapshot and a non-snapshot table and a savepoint/restore. The restore-drift rejection (c) is still only unit-level. - Issue 4 (stale `MixedPendingSplitsState` when `startup.mode` is no longer `mixed`): added in `925e7d3963a`, `IncrementalSource.java:423-430` now fails with an explicit message. - Issue 5 (INFO log of the resolved mixed policy): added in `925e7d3963a`, `IncrementalSource.java:346`, `:442`, `:553` (snapshot tables, non-snapshot tables and the specific offset; no credentials). Spotless apply/check passed for `connector-cdc-base` and `connector-cdc-mysql`. Compile, unit tests and E2E are left to the fork CI per the validation policy: https://github.com/DanielLeens/seatunnel/actions/runs/41163080925 One design note for reviewers: after this merge `IncrementalSplit` carries two per-table offset maps with similar names and the same gating effect in the reader (`tableStartOffsets` from this branch, `tableStartupOffsets` from `dev`). Folding the mixed bounds into `dev`'s map would be a cleaner end state, but it changes checkpoint lifecycle semantics (`dev` drops its entries once the split reads past them and does not carry them through the legacy `checkpointDataType` copy constructor), so I kept both as the smallest correct resolution rather than redesign it inside a merge commit. -- 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]
