DanielLeens commented on PR #11960:
URL: https://github.com/apache/seatunnel/pull/11960#issuecomment-5412547198

   Thanks both for working through the relationship with #11912 in the open — 
that's exactly the kind of discussion that should happen before either lands, 
since merging both as-is would leave the project maintaining two different 
schema-evolution protocols for the same coordination problem.
   
   From my own read of this PR's diff (not a full comparative review of #11912, 
which I haven't traced code-level), I can confirm the framing dybyte gave is 
accurate for #11960's side: this PR does fully retire `LocalSchemaCoordinator` 
and the coordinator-based dispatch, replaces it with the checkpointed 
data-plane protocol described above (control messages piggybacked on 
`SeaTunnelRow.getOptions()`, one elected DDL-owner subtask per table, `Flink 
managed ListState` for applied sequences/latest schema/out-of-order 
controls/pending rows), and constrains `incremental.parallelism = 1` on the 
source side as a startup guard rather than supporting genuinely parallel 
readers. So yes — per-table ownership with N sink writers gated behind one 
owner, not N independent writer-local refreshes.
   
   Given #11912 is "Related to #11785" and this PR closes #11958, and both are 
actively reworking the same 
`BroadcastSchemaSinkOperator`/checkpoint-coordination surface, I'd agree with 
dybyte's suggestion: this is a case for the maintainers/community to align on 
one coordination model first (via the linked issues or a dev@ thread) rather 
than two competing implementations racing to merge independently. I don't think 
that's something I can arbitrate from a code review alone — it's a design 
decision for the project, not a correctness question about either diff.
   
   That said, this doesn't change my earlier review: independent of how the 
#11912/#11960 overlap gets resolved, Issue 1 from my review (missing 
`incompatible-changes.md` entry for the checkpoint/savepoint state-format 
change) still applies to this PR as it stands and remains a blocker before 
merge.


-- 
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]

Reply via email to