SEZ9 commented on issue #12353: URL: https://github.com/apache/seatunnel/issues/12353#issuecomment-5738891481
Thanks for digging into #12311, @tomatotomata — that is useful context, and I agree we should not duplicate the implementation. My preference is to add the extra coverage in #12311 itself rather than in a separate tests-only PR. The change that maps a lost `CANCELING` vertex to `CANCELED` is exactly what the new cases exercise, so landing the fix and its deterministic coverage together keeps the terminal-state contract reviewable in one place and avoids a window where the behavior change merges without the ordering cases being pinned down. If the #12311 author would rather keep that PR narrow, a linked follow-up is acceptable, but it should be opened before #12311 merges and referenced from this issue so nothing falls through. Concretely, what I would like to see covered (whichever PR they end up in), each driven by latches rather than sleeps: 1. Worker failure before any cancellation is requested -> job ends `FAILED`. 2. Cancellation accepted before the worker failure -> job ends `CANCELED`. 3. The crash-before-cancel-ack path: cancellation requested, worker dies before the ack, and the result is `CANCELED`, with an assertion that checkpoint cancellation still happens and the failure is still reported (not silently dropped). Two asks to move this forward: - Could you check whether #12311's existing state-mapping unit test already asserts anything about checkpoint cancellation or error reporting in case (3)? If it does not, please note that on #12311 so the author knows the gap. - Please confirm on #12311 that the existing crash-before-ack E2E assertion stays as-is and that #11602 does not disable or skip it to go green. If you want to take the tests-only work, go ahead and coordinate with the #12311 author on where it lands, and drop a link here once there is a PR. <!-- streview-comment:1170 --> -- 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]
