SEZ9 commented on PR #11677:
URL: https://github.com/apache/seatunnel/pull/11677#issuecomment-6009495354

   Thanks @li3zhi4 for the real Docker/Zeta run on `677216dd4f` — a green 
`testMysqlCdcContinuesAfterSinkFailure` at ~120.9s is useful, and noted that 
the old 30s wait has been aligned to the 2-minute budget in this commit.
   
   A few things I'd still like to pin down before marking the earlier points 
resolved:
   
   1. **Which three robustness changes?** Could you map them to the earlier 
points? In particular, does this head include (a) a guard around 
`getServerLogs().substring(logOffset)` so a shrunk/rotated log retries inside 
the awaitility loop instead of throwing `StringIndexOutOfBoundsException`, and 
(b) anything for the window where the restarted job can replay the failing 
insert before the `finally` block's `DROP TRIGGER` lands and burns extra 
restart attempts? A single passing run doesn't by itself show whether those 
paths were exercised.
   
   2. **`if (running)` gate in `addSplitsBack()`.** Splits returned before 
`run()` executes are assigned only via `run()`'s trailing `assignSplits()`. The 
E2E restart test already exercises that path, so no extra unit test is needed — 
but could you add a short comment in the enumerator documenting that ordering 
so it isn't broken accidentally later?
   
   3. **`SourceSplitEnumerator` javadoc.** The rewritten contract applies to 
every connector's `addSplitsBack()`, but only the CDC base enumerator was 
hardened. Either narrow the wording back to what the engine actually guarantees 
for all implementations, or state in the PR description that other enumerators 
are expected to follow in separate PRs.
   
   Please post the CI result for the new head here when it lands, and I'll do a 
final pass then.
   
   <!-- streview-comment:1559 -->


-- 
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]

Reply via email to