hesam-oxe commented on PR #11077: URL: https://github.com/apache/seatunnel/pull/11077#issuecomment-5725176505
Thanks @SEZ9 — all items addressed in `2036ad77` or already in place on `ef2bb095`; item by item: **F1 (both asks)** 1. The `getPhysicalDestinationIdentifier()` javadoc now explicitly calls out divergent credentials: two instances of the same connector class that point at the same endpoint/table but authenticate with different users, tokens, or connection-level settings must not return the same identifier, otherwise rows are silently written under whichever alias created the shared writer first. The user-facing docs (en/zh sink-connector-development) carry the same warning. 2. `MultiTableSink` now logs an INFO line whenever a further alias joins an already-created shared writer — on both `createWriter` and `restoreWriter` paths — recording the connector class, the physical destination identifier, the joining table, and the first table. `MultiTableSink` previously had no logger; it is `@Slf4j` now, same as the other classes in the package. **F2 — option (a) is the implemented behaviour.** `MultiTableSinkWriter.snapshotState()` groups aliases by writer identity and persists each shared writer state **exactly once**, under `primaryIdentifier` (`snapshotStates.put(primaryIdentifier, states)`); there is no fan-out in new checkpoints. `getRestoredState` unions entries only so that *legacy per-alias* checkpoints still restore fully. `testSharedWriterRoundTripRestoresOneCanonicalState` pins the round trip. F6 below covers the duplicate-entries tolerance for those legacy checkpoints. **F3** — `validateSharedDestinationSchemas()` compares **full `TableSchema` objects** with `Objects.equals` (column names, types, and order — not just names/count), and throws `IllegalStateException` naming both tables, the identifier, and the connector class. It runs in the `MultiTableSink` constructor, so it covers **both** the fresh-create and restore paths (the constructor executes before either `createWriter` or `restoreWriter`). `testSharedDestinationWithDivergentSchemasFailsFast` covers the mismatch case. **F4** — `docs/en|zh/developer/sink-connector-development.md` gained a "Multi-Table Writer Sharing" section on `ef2bb095` (identifier contract, schema/commit equivalence, canonical checkpoint state, merged restore list, `Optional.empty()` fallback). `2036ad77` extends it with the credentials guidance and the new sharing log line. **F5** — the current head registers the shared `SinkContextProxy` for **every** alias via `proxyContexts.put(sinkIdentifier, proxy)` (no `containsValue`-style skip); the in-code comment documents that the proxy is intentionally reachable through every alias while `MultiTableSinkWriter` invokes its action once per writer. `registerAggregatedFlushIfNeeded` iterates `proxyContexts.values()`, which is why every alias must be present. **F6** — `SeaTunnelSink#restoreWriter` javadoc now documents that the merged list may contain equivalent entries more than once when restoring legacy per-alias checkpoints, and that implementations must be idempotent when applying them. **F7** — there is no I/O inside any `computeIfAbsent` on the current head: `destinationProxyContexts.computeIfAbsent` only constructs a `SinkContextProxy`, and writer creation uses `destinationWriters.get(...)` + explicit `put(...)` wrapped in the `try/catch (IOException)` that closes already-created writers before rethrowing (`closeCreatedWriters`). So the checked-exception-wrapping hazard is gone. **F8** — confirmed: `getDestinationKey` carries `@param tablePath`, `@param replicaIndex`, and `@return` on `ef2bb095` already. CI note: the previous `Build` failure on `ef2bb095` was the 5.5h full build dying in the test-container image pulls; this branch already syncs `dev` (`bd09f6d9`, which includes the quay.io MinIO mirror fix #12287). The new head `2036ad77` should build clean — the changes are javadoc, docs, and two INFO log statements only. -- 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]
