SEZ9 commented on PR #12391: URL: https://github.com/apache/seatunnel/pull/12391#issuecomment-5771308241
@SeaSand1024 thanks for the CI breakdown on `0fc63dc5` and for keeping this PR scoped to the lifecycle-close contract. Agreed on scope: please do not fold the #12353 / #12377 cancel-race fix into #12391. That touches `SubPlan` terminal-state selection, which is a separate contract from the close-all-flow-lifecycles change here, and mixing them would make both harder to review and revert. On the code side, the three points from the earlier review (the `Throwable` catch in the `BlockingWorker` fallback close, the added `SeaTunnelTaskStateTest` cases, and the `t != closeException[0]` guard around `addSuppressed`) are resolved between `0663920` and `0fc63dc`, so I don't see a remaining code blocker. Remaining asks, all CI-related: - Use "Re-run failed jobs" only for `engine-v2-it` and `all-connectors-it-2`; no additional commits to this branch unless something in the re-run points back at the changed code. - If `SplitClusterFaultToleranceIT.testStreamJobCancelResolvesWhenWorkerCrashesBeforeCancelAck` (expected `CANCELED`, got `FAILED`) and `OpengaussCDCIT` stay red after the re-run, please paste the new failing job links here so the attribution can be checked against that exact head rather than job names alone. - Note that neither #12311 nor #12377 is in `dev` yet, so a `dev` sync alone would not clear the cancel-race failure; no need to rebase just for that. Once the re-run result is posted, we can decide whether to merge on the green unit-test matrix plus the known-flake attribution, or wait for #12311 / #12377 to land first. <!-- streview-comment:1238 --> -- 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]
