SEZ9 commented on PR #11382: URL: https://github.com/apache/seatunnel/pull/11382#issuecomment-5659086707
Thanks for pulling the stored body of comment 5645087530 and for the consolidated F1–F8 status against `1ee22aced026`. One note first: on my end your latest comment appears cut off in the F8 bullet (it ends at "for a name it d"), so I don't see anything after that, including any F4/F5 status. Could you re-post the remainder, or just the F4/F5 lines? On the items I can see: - **F1 / F3 (write side):** The synchronized guard with a single up-front deadline and the fail-fast `IOException` on a null `checkpointType` match what I asked for. I'll verify both against the diff before marking them resolved. - **F3 (wire format):** The original finding was also about the `CheckpointFinishedOperation` format change lacking a version guard. I don't see that addressed yet, so I'm keeping that part open — happy to hear if you think it's covered. - **F2:** Agreed this stays a HIGH blocker. I'll check the re-delivery path and the test you cite against the diff; independently of that, the `checkpointCompleted` javadoc stating the coordinator is "always recreated" on task restart needs correcting to describe the actual mechanism. - **F8:** Agreed, still open. Please handle an unrecognised type name on the read side explicitly (e.g. rethrow as an `IOException` naming the offending value) so a mixed-version peer gets a clear error rather than an unchecked exception escaping deserialization. - **F6 / F7:** Noted as open and unchanged. Happy to re-review as soon as a commit lands. <!-- streview-comment:1040 --> -- 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]
