Rangsh commented on PR #12081: URL: https://github.com/apache/seatunnel/pull/12081#issuecomment-5987155188
@SEZ9 Thanks for the detailed review and for confirming the `notifyCheckpointMonitor` direction — I will push the fix (narrowing the catch to `IMapStorageException` so node-shutdown exceptions propagate exactly as on `dev`) together with the remaining review points, then re-run `engine-v2-it` on both JDKs. One small follow-up while I work on that: in my previous comment I also asked about `SplitClusterFaultToleranceIT#testStreamJobRestoreInAllNodeDown` (JDK 11 only, `UNKNOWABLE` vs `CANCELED`), where the 60s store write timeout during cancellation is pre-existing, but the fail-loud propagation added in this PR changes the observable outcome of the test. Would you prefer (a) keeping the fail-loud behaviour as intended and tracking the slow write during cancellation as a separate issue, or (b) adjusting how a store timeout is handled during cancellation in this PR? Happy to go either way — I just want to make sure I handle it the way you prefer. -- 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]
