SEZ9 commented on PR #11382: URL: https://github.com/apache/seatunnel/pull/11382#issuecomment-5643196027
Thanks for the per-finding status at `1ee22aced026` — building on that rather than restarting: - **F1** — Agreed. `checkpointCompleted` being `synchronized` with `notifyAll()` on the qualifying branch, paired with the `synchronized` wait loop in `checkSchemaChangeCanApply()`, matches the intended fix shape. Treating F1 as resolved at this head. - **F2** — Keeping open, as noted. The runtime mechanism (`restoreCoordinator(false)` resetting `isAllTaskReady`) is correct and pinned by the coordinator test, but the `checkpointCompleted` javadoc still carries the sentence claiming the coordinator is always recreated on task restart. Please push a javadoc update describing the actual re-arming mechanism before merge. - **F3 / F8** — The write-side fail-fast with `IOException` on a null `checkpointType` is noted. Still open: read-side handling of an unknown type name in `CheckpointFinishedOperation`, and whether a test covers round-trip serialization including those cases. - **F4** — Is drain-ready state still process-memory only, with notification re-delivery as the sole recovery path? If so, please state that explicitly in the guard's javadoc. - **F5** — Is there now a coordinator-side call site that sends a typed abort, or should the guard's typed-abort cleanup branch be removed or documented as unreachable? - **F6** — How are overlapping schema changes handled with the single-slot tracking, and is the type-based abort reset still broader than the checkpoint it targets? - **F7** — Beyond the guard unit tests and the coordinator test, is there coverage of the `SinkFlowLifeCycle` / `SeaTunnelTask` wiring or an end-to-end schema-evolution-with-failover scenario? Happy to re-review once the F2 javadoc fix and the remaining items land. Thanks! <!-- streview-comment:992 --> -- 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]
