dragonknightk opened a new pull request, #4201: URL: https://github.com/apache/iceberg-python/pull/4201
Closes #2604 # Rationale for this change `expire_snapshots` only rewrites the table metadata, so files that only expired snapshots referenced stay on the file system. Trino and Spark both clean those up. This adds an opt-in `clean_expired_files()` step, following the approach agreed in the issue: after the expiration has committed, compare the expired snapshots against the snapshots left in the table and delete the difference, without listing the object store. ### How files are chosen Similar to Iceberg Java's `ReachableFileCleanup`, but more conservative: 1. **Candidates:** entries (including DELETED entries) of manifests used by the expired snapshots and by no snapshot left in the table. 2. **Subtract:** if there are candidates, read the live entries of every manifest left in the table and remove their paths. 3. **Abort on read failure:** if any manifest list or manifest cannot be read, nothing is deleted. Read cost: - When snapshots are selected, the manifest lists of the expired and the remaining snapshots are read. - A table written only with fast appends reads no manifests, because no manifest goes away. - Reads run in parallel on the shared executor, and only file paths are kept in memory. ### Concurrent ref changes Without a check, the catalog would drop a branch or tag that another writer created on a snapshot being expired, and the cleanup would delete files that ref still needs. So `commit()` reloads the latest table metadata first: | Change by another writer | Result | |---|---| | Before the reload: a branch or tag points at a selected snapshot | `ValueError`, nothing is expired | | After the reload: a branch or tag other than `main` is moved or removed | commit fails (`AssertRefSnapshotId`), nothing is deleted | | After the reload: an append to `main` | expiration succeeds (`main` is not asserted) | | After the reload: a new branch or tag on a selected snapshot, or `main` reset to one | **not detected**: the catalog drops that ref with the snapshot, and its files are deleted | ### `gc.enabled` differs from Java Java rejects any expiration when `gc.enabled` is false, because its expiration deletes files by default. Here, expiring without `clean_expired_files()` only changes the metadata, so it stays allowed; only `clean_expired_files()` raises a `ValueError`. This matches the consensus in the issue that deleting files is opt-in. The value is read from the latest table metadata. ## Are these changes tested? Yes, in `tests/table/test_expire_snapshots.py`: - **Default:** no call, or `enabled=False` after enabling, deletes nothing. - **Deleted vs kept:** see the table under user-facing changes, including files still used by retained snapshots, tags, or snapshots another writer committed. - **Read cost:** a shared manifest is read once and only for live entries; a fast-append history reads no manifests. - **Concurrent refs:** every row of the table above except the undetected one, including a table object refreshed after the selection. - **Failures:** commit failure deletes nothing; a failed delete is skipped and logged; a corrupt retained manifest stops the cleanup while the expiration stays committed; autocommit and `gc.enabled` checks. ## Are there any user-facing changes? Yes. `ExpireSnapshots` gets a new `clean_expired_files(enabled: bool = True)` method: ```python table.maintenance.expire_snapshots().clean_expired_files().older_than(cutoff).commit() ``` After the expiration has committed: | Deleted | Kept | |---|---| | Data and delete files of the expired snapshots that no remaining snapshot references as live | Files any remaining snapshot references as live | | Manifests and manifest lists of the expired snapshots that no remaining snapshot uses | Previous metadata files | | Statistics and partition statistics files of the expired snapshots | Statistics and partition statistics files the metadata after the commit still references | | | Orphan files no snapshot ever referenced | On failure: - A file whose deletion raises an `OSError` is skipped and a warning is logged. - A manifest list or manifest that cannot be read: no files are deleted. - Any other exception: files not deleted yet are kept. - In both of the last two cases, a warning with the traceback is logged and the expiration stays committed. The expired snapshots are gone from the metadata, so the cleanup cannot be rerun; leftover files can only be removed by an orphan file cleanup in another engine, such as Spark's `remove_orphan_files` procedure. Requirements (otherwise `commit()` raises a `ValueError` before anything is expired): - an autocommit transaction - `gc.enabled = true` in the latest table metadata - no branch or tag in the latest table metadata pointing at a selected snapshot Without `clean_expired_files()`, or with `enabled=False`, `expire_snapshots` behaves as before. The expire-snapshots section of `mkdocs/docs/api.md` documents the new option. ## AI Disclosure Human-led contribution. Codex and Grok Bot, working for dragonknightk, assisted with this change. All submitted content was reviewed by a human before submission. -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
