Skip to content

Add opt-in expiry for stale snapshot refs - #3760

Open
1fanwang wants to merge 1 commit into
apache:mainfrom
1fanwang:fix-expire-refs-max-ref-age
Open

1fanwang wants to merge 1 commit into
apache:mainfrom
1fanwang:fix-expire-refs-max-ref-age

Conversation

@1fanwang

@1fanwang 1fanwang commented Aug 7, 2026 •

Copy link
Copy Markdown

Rationale for this change

Age-based snapshot expiration leaves stale branches and tags holding snapshots. This adds an opt-in step to expire those refs so their snapshots can expire too.

Ref age uses the snapshot's timestamp and its retention limit or the table-property fallback. Main never expires; dangling refs are removed. Calls are order-independent.

Builds on #3246.

Are these changes tested?

Python 3.12, PyArrow 25.0.1, SQLAlchemy 2.0.54. The probe reuses the committed test helper to append real data and create an expired branch in SQLite. It enables ref expiry only when that API exists.

From the PR checkout, install with pip install -e '.[pyarrow,sql-sqlite]' pytest. Save as probe.py:

Probe, commands and raw output
from datetime import datetime, timezone
from os import environ
from pyiceberg.catalog.sql import SqlCatalog
from tests.table.test_expire_snapshots import _table_with_expired_branch

root = environ["PROOF_ROOT"]
catalog = SqlCatalog("proof", uri=f"sqlite:///{root}/catalog.db", warehouse=f"file://{root}")
table, _ = _table_with_expired_branch(catalog_with_warehouse=catalog, max_ref_age_ms=1, namespace="proof")
expiration = table.maintenance.expire_snapshots()
if hasattr(expiration, "remove_expired_refs"):
    expiration.remove_expired_refs()
expiration.older_than(datetime.now(timezone.utc)).commit()
print(f"snapshots={len(table.metadata.snapshots)} refs={sorted(table.metadata.refs)}")

Compare the previous expiration implementation with this PR:

$ mkdir proof-before proof-after
$ F=pyiceberg/table/update/snapshot.py
$ git checkout 68898e5a -- "$F"
$ PROOF_ROOT="$PWD/proof-before" python probe.py
snapshots=2 refs=['audit', 'main']
$ git checkout 52542453 -- "$F"
$ PROOF_ROOT="$PWD/proof-after" python probe.py
snapshots=1 refs=['main']
$ python -m pytest tests/table/test_expire_snapshots.py -q
30 passed in 1.90s

Tests also cover unexpired refs, the default behavior, property fallback, main protection and chaining order. REST/Hive cases are in the diff, not rerun here.

Are there any user-facing changes?

Yes, opt-in ref expiry and history.expire.max-ref-age-ms, documented in the API guide. Default retention remains effectively unlimited.

AI disclosure: GitHub Copilot CLI (GPT-6 Astra) wrote this change and its tests, and an automated coordinator reviewed the diff and test output before submission.

Copilot AI lite review requested due to automatic review settings August 7, 2026 09:10

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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-ref max-ref-age-ms with fallback to history.expire.max-ref-age-ms.
  • Make older_than() resolve at commit time so chaining order with remove_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 thread pyiceberg/table/__init__.py Outdated
Comment on lines +216 to +217
MAX_REF_AGE_MS = "history.expire.max-ref-age-ms"
MAX_REF_AGE_MS_DEFAULT = sys.maxsize

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in ca43097. sys.maxsize is 2**31-1 on a 32-bit build, so the default would have been ~25 days rather than unbounded — pinned to 2**63-1 to match Java's Long.MAX_VALUE.

Comment thread tests/table/test_expire_snapshots.py Outdated

Returns the reloaded table and the snapshot id the branch pins.
"""
catalog_with_warehouse.create_namespace("expire_refs")

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in ca43097 — each test now uses its own namespace. The collision does not actually occur today, since catalog_with_warehouse calls destroy_tables() on teardown, but the tests should not depend on that for isolation.

@1fanwang
1fanwang force-pushed the fix-expire-refs-max-ref-age branch from 7a86150 to ca43097 Compare August 7, 2026 17:45
@1fanwang 1fanwang closed this Aug 24, 2026
@1fanwang 1fanwang reopened this Aug 29, 2026
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
1fanwang force-pushed the fix-expire-refs-max-ref-age branch from ca43097 to 5254245 Compare August 29, 2026 06:14
@1fanwang 1fanwang closed this Aug 29, 2026
@1fanwang 1fanwang reopened this Aug 29, 2026
@1fanwang 1fanwang changed the title Enforce max-ref-age-ms when expiring snapshots Add opt-in expiry for stale snapshot refs Sep 24, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants