li3zhi4 commented on PR #11677: URL: https://github.com/apache/seatunnel/pull/11677#issuecomment-6035500141
Mapping all three, with the honest caveat on what the single run did and didn't exercise.\n\n**1. The three robustness changes, mapped to your earlier points:**\n\n- **(a) `getServerLogs().substring(logOffset)` guard \u2014 yes, in this head.** `AbstractMysqlCDCITBase.java:275` now reads `.substring(Math.min(logOffset, serverLogs.length()))`, so a shrunk/rotated log yields \"no match yet\" and the awaitility check retries instead of `StringIndexOutOfBoundsException` aborting the wait. Being precise about your caveat: the green run did **not** exercise this path \u2014 the log didn't shrink mid-test \u2014 it's defensive hardening for a failure mode we observed as a risk, and the clamp is a one-liner to audit.\n- **(b) replay window before `DROP TRIGGER` lands \u2014 yes, handled, by the tolerant-retry option.** `mysqlcdc_to_mysql_with_sink_failure_recovery.conf` now sets `job.retry.times = 3` (lines 24-27, restored from `1` to the engine default per `EnvCommonOptions.JOB_RETRY_TIM ES`, `job.retry.interval.seconds = 15`), with a comment in the conf explaining the replay-burns-one-retry reasoning. So if the restarted job replays the failing insert before the `finally` block's `DROP TRIGGER` executes, that cycle fails, the job retries, and the trigger-drop (executed immediately after the checkpoint-count wait, before the next submit) makes the next cycle succeed \u2014 the test no longer fails spuriously on a capped single retry. The `DROP TRIGGER IF EXISTS` itself runs right after the 2-minute checkpoint wait (`AbstractMysqlCDCITBase.java:~234`).\n- Plus the third change: the injected-failure wait aligned from 30s to 2 minutes (`await().atMost(2, TimeUnit.MINUTES)` at `:227`).\n\n**2. `if (running)` ordering comments \u2014 already in `677216dd4f`, both sites:**\n- `run()`'s trailing `assignSplits()` (`IncrementalSourceEnumerator.java:77-83`): \"Dispatch the splits that addSplitsBack() returned before run() executed: the engine hands the checkpoint-restored spl its of a restarting reader to the enumerator and only invokes run() once every reader has registered, so in a restart the restored splits are normally queued in the assigner while running is still false and addSplitsBack() skipped its assignment pass. They are not lost: any split request that arrived in the meantime is already queued in readersAwaitingSplit by handleSplitRequest(), so this first assignment pass hands out the returned splits to those waiting readers.\"\n- the `if (running)` gate in `addSplitsBack()` (`:105-111`): covers both branches \u2014 `running == false` (engine delivers restored splits before `run()`; they stay queued and are dispatched by `run()`'s first pass) and `running == true` (restored split arrives after the reader's request is parked in `readersAwaitingSplit`; re-run the loop so the waiting reader gets it immediately).\n- The unit test you earlier asked for (`shouldAssignSplitsAddedBackBeforeRunExactlyOnce`) also landed before you walked it back \u2014 it stays, it's cheap and directly pins the pre-`run()` ordering.\n\n**3. `SourceSplitEnumerator` javadoc \u2014 already narrowed in `677216dd4f` (your preferred option (a)):** the `@implNote` obligation on every implementation is gone; the contract now reads (`SourceSplitEnumerator.java:52-56`): \"The engine only guarantees that `run()` is invoked after `open()` and after every reader has been registered. It does not guarantee the order listed above: restored splits may be returned via `addSplitsBack(List, int)` at any point of the source lifecycle, for example after a checkpoint restore, before or after `run()`. How such late returned splits are dispatched to readers is left to the implementation.\" The CDC-specific stricter handling moved to `IncrementalSourceEnumerator`'s class javadoc. No PR-description statement about other connectors is needed since the API no longer promises anything beyond the engine guarantee.\n\nCI on `677216dd4f`: failures confined to the `dev`-inherited families (`engine-v2-it`, `all-connectors-it-2/6/7`, `paimon`, `transform-v2-it-part-1`, `unit-test windows`, plus the upstream `Dead links` badge) \u2014 `mysql-cdc-connector-it` is **not** among them. Failed-job rerun in flight; I'll post the outcome and that should be everything for your final pass.\n -- 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]
