davidzollo commented on PR #11945:
URL: https://github.com/apache/seatunnel/pull/11945#issuecomment-5579304479
Sorry for the two-week silence on this — catching it up now.
**Rebase**: this branch was 139 commits behind `dev` with a stale CI result
from 2026-08-23. Merged current `dev` (`8bea8c681ca`) and pushed `9b33dcbefb1`;
clean merge, zero conflicts, and `dev` never touched
`IncrementalSourceEnumerator.java` or `AbstractMysqlCDCITBase.java` in the
interim, so the diff itself is unchanged.
**Issue 1 (`testMultiTableWithRestore:727` failure) — root cause confirmed
independently, and I agree with your read.** I pulled the raw container-log
dump printed just before the failing assertion in the fork CI run
(`32612293744`) and found the specific `ERROR`-level line that trips
`containerLogs.contains("ERROR")`:
```
ERROR org.apache.seatunnel.e2e.common.container.AbstractTestContainer -
Container[...] command .../seatunnel.sh ... --restore-with-checkpoint ...
--set-job-id 902 ... STDERR:
```
— followed by ordinary Hazelcast client startup log lines (config loading,
then a normal "trying ports 5806/5805/5811/5815 until one connects" sequence,
all `INFO`/`WARNING`, nothing exceptional). `testMultiTableWithRestore` itself
never uses job id 902, so this line belongs to a *different* test method's job
— confirming your diagnosis that `container.getServerLogs()` returns the
cumulative log across the whole shared-container test class, and the new
`testMysqlCdcParallelSnapshotRestoreAcrossMultipleRounds`'s restore/cancel
cycle(s) are the plausible source of `ERROR`-level noise (yours found a
Debezium "sleep interrupted" instance of the same mechanism from the same run;
either is sufficient to trip the blanket check). The production diff itself
(`addSplitsBack` → `synchronized` + conditional `assignSplits()`) adds no new
logging and has no plausible path to this failure.
**Issue 2 (duplicate of #11677) — you're right, and I don't think I should
just push forward here without addressing it.** I compared file scope: this PR
touches `IncrementalSourceEnumerator.java` + its unit test +
`AbstractMysqlCDCITBase.java`. apache/seatunnel#11677 touches the same
`IncrementalSourceEnumerator.java` method plus `SourceSplitEnumerator.java`
(Javadoc correction) and `SourceSplitEnumeratorTask.java` (the cross-class
ordering invariant you mention), and adds its own E2E conf. #11677 is still
open and shows `MERGEABLE`. This is a real overlap on the same root cause, not
just a similar idea.
Given that, this needs a maintainer/user decision rather than me picking one
unilaterally by closing either PR:
- **Option A** — close this PR in favor of #11677, optionally porting over
this PR's two deterministic unit tests
(`shouldAssignRestoredSplitsToWaitingReaderWhenEnumeratorIsRunning` /
`shouldQueueRestoredSplitsUntilEnumeratorStarts`) to #11677 if they add
coverage it doesn't already have.
- **Option B** — keep this PR and fold in the two pieces #11677 has that
this one doesn't (the `SourceSplitEnumerator` Javadoc fix and the
`SourceSplitEnumeratorTask` ordering-invariant comment), and additionally fix
Issue 1 by scoping the log check in `testMultiTableWithRestore` (offset-based,
the way this file's own `awaitSourceAndMirrorSinkConsistent` scopes query
results) instead of asserting on the whole shared-container history.
I'm not closing this PR myself; flagging the choice here for whoever makes
that call. Issue 3 (un-joined `CompletableFuture`s) and Issue 4 (unrelated
`EngineStateStoreLogicalMetricExportsTest` flake) I agree are non-blocking /
not caused by this diff.
`./mvnw spotless:apply` run on `connector-cdc-base` after the merge, no
formatting changes produced.
--
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]
