zhuxiangyi commented on PR #10105: URL: https://github.com/apache/paimon/pull/10105#issuecomment-5815663818
Thanks for confirming the session-checkpoint fix. On the timestamp boundary: reproduced first (`testChainOverwriteReplayWithSnapshotBranchCommitAtTheSameTime`, a later snapshot-branch commit carrying the overwrite's millisecond had its new row deleted), then fixed in 27290b3 by recording the exact position: - `CommitCallback` gets a `beforeOverwrite(ManifestCommittable)` hook (default no-op), called once before an overwrite is published. `ChainTableOverwriteCommitCallback` uses it to record the snapshot branch's latest snapshot id (0 if none) in a property of the delta overwrite's snapshot, so it is published atomically with it. - `retry` takes that snapshot as the boundary; no time comparison is left. It fails closed when the overwrite recorded no position (e.g. published before this change) or when the recorded snapshot has expired, and does nothing when the branch was empty. - If the snapshot branch cannot be read, the overwrite now fails before it is published, so there is nothing for a replay to redo. - One inherent limit, noted in the code: a snapshot-branch commit landing between reading that id and publishing the delta snapshot (a few milliseconds) counts as later and is kept; the existing `call` path's truncate has the same window. Also, while auditing coverage I found and fixed one more case of the kind you flagged for compaction: an overwrite-kind rewrite of the snapshot branch (e.g. a rescale) before the retry let it succeed with the superseded rows still there; such rewrites now make the retry fail explicitly (`testChainOverwriteReplayRefusesAfterSnapshotBranchRewrite`). Tests: new cases for the missing position, the empty branch and an unreadable branch; the chain tests now fail the cleanup by making the snapshot branch read-only instead of pointing it at a missing branch, since the overwrite reads that branch first. All mutation-checked. Core is green apart from the Docker-based `PostgresqlCatalogTest`; `SparkChainTableITCase`, `CompactChainTableProcedureTest`, the Flink chain ITCases and the sink suites on Spark 3.2 / 3.4 / 3.5 / 4.0 / 4.1 pass. The two failures on the previous CI run were a 403 from Maven Central and a MySQL CDC timeout, unrelated to this change. -- 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]
