SEZ9 commented on PR #11912: URL: https://github.com/apache/seatunnel/pull/11912#issuecomment-6379566619
Agree with the analysis in the thread on `SupportCoordinatedSchemaEvolutionSink`: since `MultiTableSink` itself implements the interface, a plain `instanceof` check would return true for every multi-table sink regardless of whether its child sinks support coordination, so the composite needs an explicit method to report "all children support it" to the callers. The method should stay. Please add a short javadoc on that interface method explaining exactly this (why `instanceof` alone is not sufficient and how a composite sink is expected to answer). While you are in that file, it would be good to also fold in the related point from the earlier review: the requirement that writers implement `SupportSchemaRefreshSinkWriter` is currently javadoc-only, so either enforce it (e.g. a check when the sink is wired into the coordinated flow, failing fast at startup) or at minimum document the runtime behaviour when a connector only partially implements the contract. Remaining asks from the previous review that are still open: - `MultiTableSink`: make `SchemaChangeApplier` creation lazy (on first DDL for a given table) instead of eagerly creating one per table in every parallel writer; this addresses the SPI-contract concern, the resource footprint (parallelism × table count), and the overhead on jobs that never use coordinated schema evolution. - `createSchemaChangeApplier(TablePath)`: resolve the target sink deterministically when multiple logical tables map to the same physical table, and make the applier map keying consistent (logical vs physical path). - `SchemaChangeApplier` SPI javadoc: state the identifier-safety expectations for implementations executing source-derived DDL. - Docs: a concept/connector-dev-guide update for coordinated schema evolution, plus an upgrade note on checkpoint/savepoint restore for schema-evolution jobs on Flink. Once those are in, I'll take another pass. <!-- streview-comment:1997 --> -- 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]
