1fanwang opened a new pull request, #3760: URL: https://github.com/apache/iceberg-python/pull/3760
<!-- Relates to #737 --> # Rationale for this change `create_branch()` and `create_tag()` accept `max_ref_age_ms` and write it to table metadata, but nothing in pyiceberg acts on it. That leaks storage. A live ref protects its snapshot from expiry, so a stale ref pins that snapshot and its ancestors indefinitely. The spec makes ref removal step 2 of the [snapshot retention policy](https://iceberg.apache.org/spec/#snapshot-retention-policy): > 2. Remove any refs (other than main) where the referenced snapshot is older than `max-ref-age-ms` Java implements this in `RemoveSnapshots.computeRetainedRefs()`. This adds `ExpireSnapshots.remove_expired_refs()` with the same semantics: a ref's age comes from the timestamp of the snapshot it points at, compared against its own `max-ref-age-ms` or the new `history.expire.max-ref-age-ms` table property. `main` never expires, and a ref whose snapshot is gone is removed. It is opt-in, matching the existing builder idiom, so current behavior is unchanged. `older_than()` now resolves its snapshot set at commit time. Otherwise `older_than(dt).remove_expired_refs()` would drop the ref but keep the snapshot it had pinned, since that ref was still protected when `older_than()` ran. The two calls are now order-independent. Out of scope: spec steps 4 and 5, `max-snapshot-age-ms`, and `min-snapshots-to-keep` during ancestor traversal. # Prior art [#3246](https://github.com/apache/iceberg-python/pull/3246) proposed this in April and was closed by the stale bot without review. This PR uses the same design. It differs by reading the table property the spec names as the default instead of taking a required argument, and by using a UTC clock. # Are these changes tested? Integration tests in `tests/integration/test_snapshot_operations.py` run against the REST catalog and Hive metastore from `dev/docker-compose-integration.yml`. They cover an expired branch being removed and its snapshot reclaimed, plus a branch inside its retention window surviving and continuing to protect its snapshot. ``` test_remove_expired_refs[session_catalog_hive] PASSED test_remove_expired_refs[session_catalog] PASSED test_remove_expired_refs_keeps_unexpired_branch[session_catalog_hive] PASSED test_remove_expired_refs_keeps_unexpired_branch[session_catalog] PASSED ``` Against unpatched `pyiceberg/` these four fail with `AttributeError: 'ExpireSnapshots' object has no attribute 'remove_expired_refs'`. Unit coverage in `tests/table/test_expire_snapshots.py` covers the `memory`, `sql`, and `sql_without_rowcount` catalogs: expired branch removed, unexpired branch kept, table-property fallback, `main` exempt, order independence, and behavior unchanged without the opt-in. `prek run -a` is clean. # Are there any user-facing changes? Additive: - `ExpireSnapshots.remove_expired_refs()`. - `TableProperties.MAX_REF_AGE_MS` (`history.expire.max-ref-age-ms`), defaulting to no expiry, matching Java's `Long.MAX_VALUE`. - An "Expiring Branches and Tags" section in `mkdocs/docs/api.md`. Refs are removed only when `remove_expired_refs()` is called explicitly. -- 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]
