DanielLeens commented on PR #11721: URL: https://github.com/apache/seatunnel/pull/11721#issuecomment-5419376419
Thanks for the detailed status table — I re-verified both independently against the current head (`aea9854a1bb1`) in a fresh worktree rather than trusting the counts. **F1 — confirmed resolved.** `extractPrimaryKeyIfPresent` occurs 0 times in both `docs/en/architecture/features/multi-table.md` and `docs/zh/architecture/features/multi-table.md`. The routing lines at `docs/en/...multi-table.md:311` and `:405` are `(object.hashCode() & Integer.MAX_VALUE) % blockingQueues.size()` over `element.getField(primaryKey.get())` — token-for-token identical to `MultiTableSinkWriter.java:616` and `:622`. The old `int replica = (primaryKey.hashCode() & Integer.MAX_VALUE) % replicaNum;` line is gone, and the "simplified... not a copy of the source" disclaimer is present right above the listing (around `docs/en:249-253`), so both remedy paths were satisfied, not just one. I also traced the zh `bmod replicaNum` wording: `MultiTableSink.java` passes `replicaNum` as the writer's `queueSize` constructor argument, and `MultiTableSinkWriter`'s constructor builds exactly one `blockingQueues` entry per `queueSize` in its init loop — so `blockingQueues.size() == r eplicaNum` by construction, and the zh phrasing isn't a real divergence. No change needed there. **F2 — deferral is reasonable, agreed.** Low severity, and expanding scope to the 12 sibling call sites across 8+ modules you found is correctly out of bounds for a one-line bug fix. Please do file the follow-up as promised, with the `Integer.MIN_VALUE` unit test. **CI — verified independently too.** `unit-test (8, ubuntu-latest)` in fork run `32746106023` shows `conclusion: success` on `run_attempt: 2`. The only non-passing job left is `kudu-connector-it (11, ubuntu-latest)`, `cancelled`, with a wall clock of 12:31:05→14:01:35 UTC — exactly the 90-minute `timeout-minutes` budget — while its JDK-8 twin finished the same suite on the same commit in ~28 minutes. That reads as a stuck runner, not a regression, and it's a connector module untouched by this diff either way. With F1 confirmed closed, F2 correctly deferred as its own follow-up, and F3 already resolved from the prior round, I don't see any open blocker left on my side. My `Ready to merge` conclusion from the 08-24 review stands at this head — thanks for the very thorough follow-through across all these rounds. -- 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]
