heye1005 commented on PR #7715: URL: https://github.com/apache/paimon/pull/7715#issuecomment-4358890786
> Hi @heye1005 It is better to fix it reading full ref files for latest snapshot. Thanks for the review! Updated the PR based on your suggestion. The root cause here is that `safelyGetAllSnapshots()` lists the snapshot directory first, then reads each file. If expiration deletes the snapshot file (along with its manifest-list) between these two steps, the read silently fails and returns an empty list — all old data files then get treated as orphans. This PR has two changes: 1. Replaced the list-then-read in `safelyGetAllSnapshots()` with `latestSnapshot()` + ID range traversal. Expiration never deletes the latest snapshot, so the snapshot file read is safe. But the manifest-list of that snapshot can still be deleted before we read it. 2. Added `collectLatestSnapshotFiles()` — re-reads the latest snapshot right before deletion and removes its referenced files from candidates. By then expiration is usually done, so the current latest snapshot's manifest-list is intact. This is applied to Local, Flink, and Spark subclasses. This doesn't 100% eliminate the race — if expiration happens to delete the latest snapshot's manifest-list during the pre-deletion check, it would still fail. But that window is just milliseconds, so practically very unlikely. To fully solve this, I think we'd need either some form of locking between orphan clean and expiration, or delay manifest-list deletion (similar to how orphan clean already requires data files to be older than `olderThanMillis`). Happy to discuss further. -- 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]
