Rangsh commented on PR #12218: URL: https://github.com/apache/seatunnel/pull/12218#issuecomment-6098914285
Thanks @DanielLeens for the detailed review. I synced the latest `dev` and addressed all three issues in `29d664924`. **Issue 1:** `submitBlockingTask` now builds the accepted futures in a local list. The success path still waits for `startedLatch` before publishing them, so #12454's "publish after startup" behavior is preserved. Only the submit-failure path publishes the already-accepted futures before rethrowing, because that path never awaits the latch; rollback can then cancel them, including workers that have not started. Covered by `testBlockingInitFailureDuringSubmitCompletesAsFailed`. **Issue 2:** the new early return is now limited to the rollback signal (`taskGroupContext == null` or a null classloader map). An ordinary `isCancel` still runs `init()`, `call()`, and `close()`. `testBlockingWorkerStartedAfterCancelStillCloses` pins that contract. **Issue 3:** `abortAfterFailedPublish` records the failure and cancels workers without setting `isCancel`, so `taskDone` reports `FAILED` instead of `CANCELED`. The deploy thread also completes the same future normally with `FAILED` if no worker completed it first, rather than completing it exceptionally and hiding that state. Covered by `testAllBlockingDeployFailureAfterWorkersStartedCompletesAsFailed`. I also replaced the `Thread.sleep(200)` quiescence check with a flag set by the probe task when it observes interruption, and the same-package test now assigns `blockingTaskExecutorOverride` directly instead of using reflection. `TaskExecutionServiceTest` is green locally (27 tests). -- 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]
