SEZ9 commented on PR #12943: URL: https://github.com/apache/seatunnel/pull/12943#issuecomment-6552711791
Thanks for the continued work on this one — it's a large change and the iteration has been steady. A few notes on the latest state: - The `d69dad9` → `ef55131` range is test-only, switching the takeover test to a shared `localfile` checkpoint store in a temporary directory. That wiring looks correct to me, and dropping the Hadoop native-utility requirement for this test is a good call while still exercising a real file store shared by both members. - At `73d340488777` I re-read the full 78-file diff. The earlier conflict request and the terminal-pipeline test gap are resolved, and the production-side design (exactly-two-masters validation, UID/resourceVersion-conditional owner annotation claim, successor attaching instead of resubmitting, distributed I/O outside the global monitor, monotonic checkpoint counter advancement, execution-ID-tagged task reports) still reads as intended. Remaining asks before moving this forward: 1. The remaining comments on core ownership/protocol state that I missed in an earlier round are still open. Could you either address them or reply inline explaining why the current behaviour is correct, so they are closed out explicitly? 2. Since my review is static, please point me to a green CI run on the current head (especially the takeover test) once it is available. Once those are in, I'll do a final pass. <!-- streview-comment:2188 --> -- 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]
