SEZ9 commented on PR #12152: URL: https://github.com/apache/seatunnel/pull/12152#issuecomment-5852092838
Thanks for the follow-up on `0a4b61553` — I re-read the current head against the two points I raised earlier, and both look resolved: - **F1 (rejected `execute(...)` silently dropping the report):** the offload is now guarded — a `RejectedExecutionException` from the primary pool falls through to the static daemon `ERROR_REPORT_FALLBACK_EXECUTOR`, and the future is completed after dispatch, so a `.join()`/`.get()` waiter can no longer hang when the shared pool has been `shutdownNow()`-ed on a master switch. Dispatch-before-complete ordering keeps the `isDone()` guard meaningful. - **F2 (test only proved the caller returned):** `testCheckpointErrorReportDoesNotRunOnCallerThread` now positively asserts delivery via `Mockito.verify(checkpointManager, Mockito.timeout(5000)).handleCheckpointError(eq(1), eq(false))` once the busy executor is released, and `testCheckpointErrorReportCompletesFutureWhenExecutorRejects` / `testCheckpointErrorReportRejectionStillReachesCancellationOffCallerThread` cover the shutdown/rejection path end to end. That is exactly the coverage I was after. One small, non-blocking note on the same topic: the double-rejection last-resort branch (`CheckpointCoordinator.java:585-604`) isn't exercised by any test and is unreachable today since nothing shuts down the fallback executor; if it is ever reached, the exception message recorded at line 597 looks wrong. Happy for that to land as a fast follow-up rather than in this PR — just confirm you're fine tracking it separately. Remaining ask before I approve: the `Build` check on this head is still the earlier completed run (`35681392273`), whose 3 failures don't touch `CheckpointCoordinator.java` or its test. Since the branch has drifted behind `dev`, could you rebase and push so we get a fresh run? A green (or confirmed-unrelated) result on the rebased head and I'll approve. <!-- streview-comment:1352 --> -- 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]
