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 ever 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, with no Python-side way 
to recover. Any branch-per-job pattern hits this: a job creates an audit branch 
with a TTL, fails before publish, and the TTL never fires.
   
   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 does this in `RemoveSnapshots.computeRetainedRefs()`. Python does not.
   
   Adds `ExpireSnapshots.remove_expired_refs()` with the Java semantics: age 
measured from the referenced snapshot's timestamp, against the ref's own 
`max-ref-age-ms` or the new `history.expire.max-ref-age-ms` table property. 
`main` never expires; a ref pointing at a missing snapshot is dropped.
   
   Opt-in, matching the existing builder idiom (`by_id`, `older_than`), so 
current behavior is unchanged.
   
   `older_than()` now resolves at commit time. Previously 
`older_than(dt).remove_expired_refs()` dropped the ref but kept the snapshot it 
had pinned, since the ref was still protected when `older_than()` ran. Order no 
longer matters.
   
   Out of scope: spec steps 4–5 (`max-snapshot-age-ms`, `min-snapshots-to-keep` 
during ancestor traversal).
   
   # Prior art
   
   [#3246](https://github.com/apache/iceberg-python/pull/3246) by 
@alessandro-nori proposed this in April and was closed by the stale bot without 
human review. I found it after writing this; we converged on the same shape, 
and first credit is theirs. Two differences: it takes a required 
`default_max_ref_age_ms` argument where this reads the table property the spec 
names as the default, and it uses naive `datetime.now()` where this uses the 
existing UTC helper.
   
   @alessandro-nori — happy to close this in favour of a revived #3246 if you 
would rather carry it.
   
   # Are these changes tested?
   
   6 new tests in `tests/table/test_expire_snapshots.py`, each across the 
`memory`, `sql`, and `sql_without_rowcount` catalogs: expired branch removed 
and its snapshot reclaimed, unexpired branch kept, table-property fallback, 
`main` never removed, order independence, and a guard that behavior is 
unchanged without the opt-in.
   
   Reverting the source while keeping the tests fails 12; the 3 that still pass 
are the backward-compat guard.
   
   Full unit suite 3823 passed / 3 skipped, `prek run -a` clean. Also run end 
to end against a real SQLite catalog and local warehouse — real table, real 
commits, real branch, no mocks:
   
   <details><summary>Live before/after</summary>
   
   ```
   branch TTL = 1 ms, waiting 500 ms before expiry
   
   BEFORE (current behavior)
     refs present       : ['audit-run-1', 'main']
     expired branch     : STILL PRESENT
     pinned snapshot    : STILL PINNED
     snapshots retained : 2
   
   AFTER  (.remove_expired_refs())
     refs present       : ['main']
     expired branch     : removed
     pinned snapshot    : reclaimed
     snapshots retained : 1
   ```
   
   Chaining order, patched build:
   
   ```
   refs_first         -> ref=removed  pinned_snapshot=reclaimed
   older_than_first   -> ref=removed  pinned_snapshot=reclaimed
   ```
   
   Before the `older_than()` change, `older_than_first` gave 
`pinned_snapshot=kept`.
   
   </details>
   
   # Are there any user-facing changes?
   
   Additive only:
   
   - New `ExpireSnapshots.remove_expired_refs()`.
   - New `TableProperties.MAX_REF_AGE_MS` (`history.expire.max-ref-age-ms`), 
defaulting to no expiry, matching Java's `Long.MAX_VALUE`.
   - New "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]

Reply via email to