spoorthibasu commented on PR #4360:
URL: https://github.com/apache/flink-cdc/pull/4360#issuecomment-4190249987

   Thanks for the detailed review, @fcfangcc, this was a very helpful catch.
   
   You’re right there were two issues here:
   
   1. Original bug:
   Schema changes could split writes within a checkpoint, but all batches were 
committed into a single Iceberg snapshot with the same sequence number, so 
equality-deletes could not suppress earlier data.
   Fixed by flushing per-table on schema change and committing each batch as a 
separate snapshot.
   
   2. Parallelism:
        •       Writer: flushTableWriter could return early and cause 
batchIndex drift across subtasks. Fixed by advancing the counter before the 
early return.
        •       Committer: Wrappers were committed individually, so same 
batchIndex data from different subtasks could produce multiple snapshots. Fixed 
by grouping by batchIndex and merging into a single snapshot.
   
   Retry safety:
   Only the final non-empty batch writes flink-max-committed-checkpoint-id. 
Intermediate batches carry flink.batch-index and flink.checkpoint-id, so 
retries resume from the next uncommitted batch instead of skipping.
   
   The changes are in this commit for reference:
   
https://github.com/apache/flink-cdc/pull/4360/changes/d42c29021a4bc8f2d20e140b73373aa0472eb08a
   
   I also looked at your implementation. Using explicit per-snapshot markers to 
track batch boundaries makes sense. One concern is retry behavior: if the 
committer crashes after batch 0 but before batch 1, the retry would see the 
checkpoint id property on the batch 0 snapshot and skip the entire checkpoint, 
leaving batch 1’s data uncommitted.
   
   In this approach, only the final non-empty batch sets that property, so 
retries resume from the next uncommitted batch instead of skipping the 
checkpoint entirely.
   
   Happy to discuss further, and I would appreciate your feedback on this 
approach.


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