DanielLeens commented on PR #12152: URL: https://github.com/apache/seatunnel/pull/12152#issuecomment-5678484237
Thanks for the heads-up, @CryoThrust, and for the rebase note as well. On the `Build` failure: your diagnosis matches what I see independently. The apache-side `Build` check is a pointer to your fork's workflow run, and the failing lanes you listed (connector-v2/transform-v2/rocketmq/engine-v2 integration suites) don't touch `CheckpointCoordinator.java` or its test, which is the only production/test diff in this PR. I also checked a couple of unrelated open PRs to sanity-check the "infra, not diff" claim — #12173 and #12149 are both red on the same `Build` check right now despite touching completely different code, which is consistent with a shared fork-side/CI infra issue rather than something introduced by this branch. So I agree this looks environmental, not caused by this change, and I won't hold it against the review. That said, to be clear about where this PR actually stands: CI status isn't the thing blocking merge right now. My last full re-review (2026-09-14) is still current — Issue 1 (unguarded `RejectedExecutionException` on `executorService.execute(...)` at `CheckpointCoordinator.java:522`) and Issue 2 (test coverage for the rejection/shutdown path) are the two blockers, and both are still open in the current head (`551442e`), which is a rebase-only commit. Once the agreed fallback shape (fallback dispatch onto an executor independent of the one `clearCoordinatorService()` can shut down, or the documented+tested alternate-ownership note) and the three test invariants land, I'll do a full re-review — at that point I'll also take a fresh look at `Build` to see if it's cleared up by then. -- 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]
