Enforce max-ref-age-ms when expiring snapshots - #3760
Open
1fanwang wants to merge 1 commit into
Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds support for expiring stale snapshot refs (branches/tags) based on max-ref-age-ms, bringing PyIceberg’s snapshot expiration behavior closer to Iceberg’s retention policy semantics and preventing refs from pinning snapshots indefinitely.
Changes:
- Add
ExpireSnapshots.remove_expired_refs()to drop stale refs based on per-refmax-ref-age-mswith fallback tohistory.expire.max-ref-age-ms. - Make
older_than()resolve at commit time so chaining order withremove_expired_refs()does not affect which snapshots become eligible for expiration. - Add unit tests and API documentation for expiring branches/tags and the new table property.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| tests/table/test_expire_snapshots.py | Adds coverage for ref expiry semantics (expired/unexpired refs, opt-in behavior, order independence, table-property fallback, main exemption). |
| pyiceberg/table/update/snapshot.py | Implements ref expiry staging and defers older_than() evaluation to commit to make builder chaining order-independent. |
| pyiceberg/table/init.py | Introduces TableProperties.MAX_REF_AGE_MS and its default. |
| mkdocs/docs/api.md | Documents how to expire branches/tags and reclaim pinned snapshots. |
Suppressed comments (2)
tests/table/test_expire_snapshots.py:409
- This test creates a fixed namespace name in a session-scoped warehouse; if another test already created it, create_namespace will raise NamespaceAlreadyExistsError. Use create_namespace_if_not_exists (or randomize the namespace) to keep the suite order-independent.
catalog_with_warehouse.create_namespace("expire_refs")
tests/table/test_expire_snapshots.py:433
- This test creates a fixed namespace name in a session-scoped warehouse; if another test already created it, create_namespace will raise NamespaceAlreadyExistsError. Use create_namespace_if_not_exists (or randomize the namespace) to keep the suite order-independent.
catalog_with_warehouse.create_namespace("expire_refs")
Comment on lines
+216
to
+217
| MAX_REF_AGE_MS = "history.expire.max-ref-age-ms" | ||
| MAX_REF_AGE_MS_DEFAULT = sys.maxsize |
|
|
||
| Returns the reloaded table and the snapshot id the branch pins. | ||
| """ | ||
| catalog_with_warehouse.create_namespace("expire_refs") |
create_branch() and create_tag() accept max_ref_age_ms and write it to table metadata, but nothing in pyiceberg acts on it. Since a live ref protects its snapshot from expiry, a stale ref pins that snapshot and its ancestors indefinitely. Add ExpireSnapshots.remove_expired_refs(), implementing step 2 of the spec's snapshot retention policy: drop refs other than main whose referenced snapshot is older than max-ref-age-ms, falling back to the new history.expire.max-ref-age-ms table property. Refs pointing at a snapshot that no longer exists are removed too. older_than() resolves its snapshot set at commit time so that snapshots released by remove_expired_refs() are reclaimed in the same commit regardless of the order the two are chained in. Signed-off-by: 1fanwang <1fannnw@gmail.com>
1fanwang
force-pushed
the
fix-expire-refs-max-ref-age
branch
from
August 7, 2026 17:45
7a86150 to
ca43097
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Rationale for this change
create_branch()andcreate_tag()acceptmax_ref_age_msand 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:
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 ownmax-ref-age-msor the newhistory.expire.max-ref-age-mstable property.mainnever 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. Otherwiseolder_than(dt).remove_expired_refs()would drop the ref but keep the snapshot it had pinned, since that ref was still protected whenolder_than()ran. The two calls are now order-independent.Out of scope: spec steps 4 and 5,
max-snapshot-age-ms, andmin-snapshots-to-keepduring ancestor traversal.Prior art
#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.pyrun against the REST catalog and Hive metastore fromdev/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.Against unpatched
pyiceberg/these four fail withAttributeError: 'ExpireSnapshots' object has no attribute 'remove_expired_refs'.Unit coverage in
tests/table/test_expire_snapshots.pycovers thememory,sql, andsql_without_rowcountcatalogs: expired branch removed, unexpired branch kept, table-property fallback,mainexempt, order independence, and behavior unchanged without the opt-in.prek run -ais 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'sLong.MAX_VALUE.mkdocs/docs/api.md.Refs are removed only when
remove_expired_refs()is called explicitly.