zhengxiang378928908-code commented on PR #12937: URL: https://github.com/apache/seatunnel/pull/12937#issuecomment-6648033974
Thanks, @DanielLeens. Agreed: resuming a partly committed split checkpoint depends on the engine calling `restoreCommit`. From the code (neither Flink recovery path was run for this follow-up): - Flink 1.13/1.15 (`seatunnel-translation-flink-common`): `FlinkGlobalCommitter.filterRecoveredCommittables` returns an empty list, so recovered global committables never reach the sink. - Flink 1.20: `FlinkSimpleAggregatedCommitter` is a per-subtask Sink V2 `Committer`. Recovered pending committables reach `commit()`, not `restoreCommit()`, so groups already in the table are not skipped. These adapter paths are the same on `dev`, and `commit()` there does not check whether a checkpoint is already committed either, so the limitation is not specific to split checkpoints. **Why this commit has no Flink guard:** `JobContext` exposes the job id, the job mode and whether checkpointing is enabled, but no engine type, and the `SinkWriter.Context` interface does not expose one either. Checking for engine-specific context classes would tie the connector to engine internals, so a supported check would need a separate change. Pushed `0200968c6`, docs and Javadoc only, no runtime change: - `flush_row_size` docs (EN/ZH): resuming a partly committed split checkpoint is stated as Zeta only, including splits caused by schema changes. They describe the Flink behavior (1.13/1.15 can leave the missing snapshots of recovered global committables uncommitted, 1.20 can commit files of snapshots already in the table again) and say this is not checked at job start. - `incompatible-changes.md` (EN/ZH): the migration note says to restore such a checkpoint on Zeta. - Javadoc on `IcebergAggregatedCommitter.restoreCommit` with the same boundary. This documents the restriction but does not implement the fail-fast part of Option B. Is a documented Zeta-only boundary enough for this PR, or should an adapter fix land first? If the latter, I can open a separate issue for the two Flink recovery paths. On CI: this follow-up changes no Redis code; the Build on the new head will show whether `Redis7Test` passes. -- 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]
