SEZ9 commented on PR #11932:
URL: https://github.com/apache/seatunnel/pull/11932#issuecomment-5564016419
@DanielLeens thanks for the thorough re-review of `b6c96b97356d`, and for
tracing the `dev`-sync merge (`b6c96b973`) file by file rather than treating it
as a no-op. Your read that the only real overlap is
`IncrementalSourceReader.java` picking up `af0a647d2` (#11271), and that the
pruning step in `addSplits()` does not interfere with this PR's schema-restore
path, matches my understanding of how the two changes are ordered, so I have no
objection there.
Two things before I consider this fully settled:
1. Your comment appears to have been cut off mid-sentence right after
`hasRestoredCheckpointMetadata()` (true only when the split carries
`checkpointDataType`/…). Could you post the rest of that call-order trace? I
want the full argument for why #11271's pruning and this PR's restore of the
evolved schema cannot race or observe each other's partial state on record,
since that is exactly the interaction I would worry about.
2. You note the core mechanism findings (the
`AtomicReference.getAndSet(null)` race, the
empty-list/legacy-restore/resolver-gate unification, and the JDBC-sink
findings) were closed at `d9bc23443d72`. Two items from the earlier scope are
not mentioned in your summary, and I'd like an explicit status on each:
- The docs/upgrade note for the JDBC sink checkpoint state change (schema
now persisted in `JdbcSinkState`, non-exactly-once writer now emitting state).
Is that note in the PR now, and if so where?
- `Collector.restoreSchema` being a no-op on Flink/Spark, so the
connector-level restore in `IncrementalSourceReader` only takes effect on Zeta.
Was this addressed, or is the intent to document it as a Zeta-only behavior for
now?
Once those two are confirmed (and the truncated trace is in), I'm
comfortable with the current head from my side.
<!-- streview-comment:850 -->
--
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]
