feat(transaction): add unreferenced_files detection for expire-snapshots - #3038
feat(transaction): add unreferenced_files detection for expire-snapshots#3038dhruvarya-db wants to merge 6 commits into
Conversation
Add `iceberg::transaction::unreferenced_files`, which computes the files reachable only from a set of expiring snapshots — manifest lists, manifests, data/delete files, and statistics — so a caller can delete them after the expire-snapshots metadata commit. It mirrors Java `ReachableFileCleanup`: the result is the reference-count difference `files(expired) - files(retained)`, so a file still held by any retained snapshot is never returned. Data and delete files are only collected when `gc.enabled` is set, matching `drop_table_data`, since they can be shared across tables. Manifests are read through `ManifestFile::load_manifest`, so encrypted manifests are handled transparently. Any manifest-list or manifest read failure aborts the call, rather than risking deletion of a live file. This is metadata analysis only; nothing is deleted here. A follow-up will wire it into the commit path to perform the actual file deletion.
Only live (added/existing) entries reference a file. A retained snapshot's manifest can carry a data file as a Deleted entry, and counting that as reachable made the anti-join treat the file as still live — so a file that was deleted by a retained snapshot but had been live in an expired one was never reported for deletion. Filter to `is_alive()` entries, matching Java's `liveEntries()` and the scan planner's handling.
There was a problem hiding this comment.
Nice PR, thanks for working on this.
I don't think gc.enabled=false should make metadata files eligible for physical deletion. I read it as a hard safety boundary for garbage collection: snapshot expiration may update table metadata to drop snapshots, but it should not delete any files from storage. In Java terms, this would correspond to CleanupLevel.NONE, not METADATA_ONLY.
The assumption that manifest lists, manifests, and statistics files are always table-private does not hold for all catalogs.gc.enabled=false is commonly used specifically to protect those orphaned references.
Could unreferenced_files return an empty UnreferencedFiles whenever GC is disabled? If metadata-only physical deletion is desirable, I think that should require a separate explicit opt-in rather than being implied by gc.enabled=false. The metadata update that expires snapshots can still proceed; only physical cleanup would be suppressed.
Java currently throws if gc flag is false, I'm working on a PR at the moment to align with the behaviour I've described above. WDYT?
Main was merged in, bringing apache#2989 which made `gc_enabled` a getter method rather than a public field. Call it with parentheses.
# Conflicts: # crates/iceberg/public-api.txt
Treat gc.enabled=false as a hard garbage-collection boundary (Java's CleanupLevel.NONE): report nothing to delete. Previously only data/delete files were suppressed while manifests, manifest lists, and statistics were still returned, on the assumption they are table-private — but that does not hold for all catalogs, and gc.enabled=false is commonly used precisely to protect such orphaned references. Snapshot expiry can still rewrite metadata; physical cleanup is fully suppressed. Any metadata-only deletion should be a separate explicit opt-in. collect_reachable no longer needs the gc flag, since it now runs only when gc is enabled.
|
Thanks for the review @xanderbailey . The suggestion makes sense, I have updated |
3 participants
Which issue does this PR close?
Part of #2145.
The metadata side of expire-snapshots is already in place from three merged PRs: #2591 added the
ExpireSnapshotsAction, #2664 made it honour thehistory.expire.*table properties by default, and #2667 removed statistics-file metadata for the expired snapshots. Those all editmetadata.json(dropping snapshots and their statistics) but none of them delete anything from storage, so the manifest lists, manifests, and data files that only the expired snapshots referenced are left behind.What changes are included in this PR?
This PR adds the detection step that finds those files, without deleting anything.
iceberg::transaction::unreferenced_files(table, expired_snapshot_ids)reads the manifest lists and manifests of the expiring snapshots and returns the files reachable only from them, grouped by kind (manifest lists, manifests, data files, delete files, and statistics). It mirrors Java'sReachableFileCleanup: the result is the reference-count differencefiles(expired) - files(retained), so anything a surviving snapshot still points at is never returned. Content files (data and deletes) are only collected whengc.enabledis set, matchingdrop_table_data, since they can be shared across tables; the table-private metadata is always collected. Manifests are read throughManifestReader, so encrypted manifests are handled transparently.Any read failure aborts the whole call: a manifest list or manifest that can't be read — for a retained or an expired snapshot alike — leaves the reachable set incomplete, and continuing could delete a file that is still live. This matches Java's
ReachableFileCleanup.This is metadata analysis only — nothing is deleted here. A follow-up PR will wire this into the commit path and perform the actual file deletion.
Are these changes tested?