SEZ9 commented on issue #10203: URL: https://github.com/apache/seatunnel/issues/10203#issuecomment-6593561092
@ryanmeowy thanks — both points read right, and nothing is blocked on my side. **1. Backlink.** Having both §2 and the table comment point at [6357660652](https://github.com/apache/seatunnel/issues/10203#issuecomment-6357660652), with the table comment authoritative on divergence and §2 explicitly a readability restatement, works. No further change needed there. **2. The two contract-1 traps.** Agreed that both belong in the contract text rather than in a test comment, and calling them out in §0 instead of editing them in silently was the right call. - The `SnapshotSplitAssigner` one is the nastier of the two, since, as you say, each half is survivable alone and the combination only shows up under a specific ordering. Stating "admission is never routed through `SnapshotSplitAssigner`" as a hard rule is the right fix at the design level. - The `assignedSplits` one is the classic silent-data-loss shape: a table set derived from a map that is written once and never cleared, so the rebuilt split carries only the new table and `noMoreSplits()` returns true. Good catch. One ask: please make sure §7 items 4 and 5 assert the *specific* failure modes — item 4 fails if a second `CompletedSnapshotPhaseEvent` is observed after admission, and item 5 fails if any pre-existing table is absent from the rebuilt split's table set — rather than only asserting that the new table is read. Otherwise they would pass on the happy path for the same reason the traps were missed the first time. **3. Stale statements.** Updating against head `d4faf1ea1` rather than leaving stale text is the right choice, and marking which line references are against `dev` at `128994ca0` versus the PR branch keeps it reviewable. I'll check the §5 condition for the `schema-changes.include` guard coming back against that head. With the §7 assertions tightened, I'm fine agreeing the design and keeping implementation queued as §8.3 lays out. <!-- streview-comment:2227 --> -- 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]
