SEZ9 commented on PR #12134: URL: https://github.com/apache/seatunnel/pull/12134#issuecomment-6459636357
Thanks for the follow-up. I re-read the thread against head `6011de825d88de525196d8666fc2311fb419971e` and want to close the loop on the earlier review points. **Production side (F1).** The latest review in this thread describes the `CoordinatorService` / `JobMaster` change (`markRedriveSavepointAfterRestore()` on a `DOING_SAVEPOINT` restore, `consumeRedriveSavepointAfterRestore()` inside the scheduling lambda, then `redriveSavepointAfterMasterFailover(jobMaster, mdcExecutorService)` polling for `RUNNING` and retrying `savePoint().get()`), and notes the production diff is unchanged since `872701ec8d8`. That matches the intent, but I still can't review it from the diff body in the PR view, and the test's `SAVEPOINT_DONE` assertion rests entirely on it. Could you paste or point me at the body of the redrive task, specifically the retry/termination conditions and what happens if the job never reaches `RUNNING` or `savePoint()` keeps failing? I want to be sure the background task cannot spin forever or leave the job in a half-state. **E2E hardening in `419000d09ee` (F2–F8).** To mark these resolved I'd appreciate a short mapping, in the PR description or a comment: - F2 / F5 (race between detecting the in-flight savepoint and `masterNode1.shutdown()`): how does the test now guarantee the savepoint was still pending when the old master went down, so a pass can't come from the savepoint finishing on the old master? - F3 (reflection polling throwing mid-transition): is the Awaitility wait now using `ignoreExceptions()` or an equivalent guard? - F4 / F7 / F8 (`savepointCaller` executor and `savepointCall` future): is the executor shut down on every exit path, and is the future's outcome inspected so a pre-failover `savePointJob` failure surfaces directly rather than as an unrelated 30s timeout? - F6 (reflection into `pendingCheckpoints`): still accessing the private field, or replaced with a public/observable signal? If it stays, a brief comment in the test explaining the coupling is fine with me. Once those answers are in, I'm happy to go through the remaining items and move toward approval. <!-- streview-comment:2090 --> -- 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]
