SEZ9 commented on PR #11077: URL: https://github.com/apache/seatunnel/pull/11077#issuecomment-6486669330
Thanks for the work on this so far. A short follow-up on head `8b383c04eeed65c41a14ba756100a33f430124eb`: **Source findings (F1–F8) remain open** - **F1 / F2 (HIGH)**: destination-key collisions that can route one table's rows through another sink's writer, and the snapshot fan-out plus restore-time union that duplicates shared-writer state N times on recovery. These are the blockers; please push the fixes (or explain why the current behavior is intended) so they can be examined in code. - **F3, F5, F7 (MEDIUM)**: the shared writer is built from an arbitrary alias's sink/CatalogTable with no guard against schema/config divergence across aliased shards; `proxyContexts` registers only the first alias per destination via `containsValue`; and IOException from createWriter/restoreWriter is wrapped in an unchecked RuntimeException inside `computeIfAbsent`, breaking the declared failure contract. - **F4, F6 (MEDIUM, Docs)**: `getPhysicalDestinationIdentifier()` and the writer-sharing behavior need coverage in docs/, and the changed `restoreWriter` contract (merged state from all aliased identifiers passed to one `SeaTunnelSink#restoreWriter` call) needs to be documented for connector implementers. - **F8 (LOW)**: the `getDestinationKey` Javadoc is missing param/return tags. **Next steps** 1. When you push the next head, please list which of F1–F8 it addresses (and which are deferred, with a reason), so we can review one concrete version. 2. [Build](https://github.com/apache/seatunnel/runs/114987916217) on this head is `ACTION_REQUIRED` because no fork Build run was found for this SHA. Please start the Build workflow from your fork's Actions tab and link the run here; no empty commit is needed for that. 3. GitHub reports merge conflicts with `dev`. Please sync and resolve them; the resulting head will need checks and re-review. The [previous source conclusion](https://github.com/apache/seatunnel/pull/11077#issuecomment-6338374312) stands until those fixes land. Happy to take a look as soon as there's a new head to review. <!-- streview-comment:2113 --> -- 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]
