Vivek1106-04 commented on PR #12165: URL: https://github.com/apache/seatunnel/pull/12165#issuecomment-6393409404
Thanks @DanielLeens, good catch on the semantic conflict with #12448. Both items are addressed at head `29ab7519b`: **Issue 1 (blocker):** merged the latest `dev` (`feedd2506`, brings in #12448 and #12387). Before the fix, `CheckpointCoordinatorScheduledTriggerTest` failed on the merged tree exactly as you described: the plain mock returned a `null` lease, and the NPE from `cancelCheckpoint()` in `finally` hid the one from `scheduleTriggerPendingCheckpoint`. `f6224fc3b` makes the mock hand out a lease from a test-scoped `SharedCheckpointScheduler`, which is closed in `finally`, the same way `CheckpointCoordinatorTest.mockCheckpointManager()` does. All of the #12448 assertions are unchanged, and the test now passes. So #12448's wrapped trigger body still fails the coordinator (`FAILED` plus `handleCheckpointError(1, false)`) when it runs through the shared dispatcher. No other test on `dev` builds a coordinator on a bare manager mock. **Issue 4:** `29ab7519b` completes the future before logging. Local, JDK 17: `CheckpointCoordinatorScheduledTriggerTest` 1/1, `CheckpointCoordinatorTest` 18/18, `SharedCheckpointSchedulerTest` 11/11, `StateTransitionCleanupTest` 13/13; `spotless:apply` and module `verify` are clean. Fork Build on the new head (still running, I will follow up with the result): https://github.com/Vivek1106-04/seatunnel/actions/runs/40161631838 Issues 2 and 3 stay as tracked notes (#12441 / #12511). -- 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]
