Skip to content

Backfill history.expire.max-ref-age-ms with Snapshot Expiration - #708

Merged
mkuchenbecker merged 15 commits into
linkedin:mainfrom
mkuchenbecker:mkuchenbecker/bdp-106337-default-max-ref-age
Sep 8, 2026
Merged

mkuchenbecker merged 15 commits into
linkedin:mainfrom
mkuchenbecker:mkuchenbecker/bdp-106337-default-max-ref-age

Conversation

@mkuchenbecker

@mkuchenbecker mkuchenbecker commented Sep 2, 2026

Copy link
Copy Markdown
Collaborator

Summary

Use Iceberg's native reference expiration during snapshot-expiration maintenance.

New tables receive a seven-day history.expire.max-ref-age-ms default during creation. Snapshot expiration backfills the same property when it is missing from a legacy table, then delegates branch and tag retention to Iceberg. Existing table-level and reference-specific maximum ages remain authoritative.

Changes

  • Client-facing API Changes
  • Internal API Changes
  • Bug Fixes
  • New Features
  • Performance Improvements
  • Code Style
  • Refactoring
  • Documentation
  • Tests

Testing Done

  • Manually Tested on local docker setup. Please include commands ran, and their output.
  • Added new tests for the changes made.
  • Updated existing tests to reflect the changes made.
  • No tests added or updated. Please explain why. If unsure, please feel free to ask for help.
  • Some other form of testing like staging or soak time in production. Please explain.

Added real Iceberg table tests that verify:

  • Snapshot expiration backfills the seven-day maximum reference age for a legacy table.
  • Native Iceberg expiration removes stale branch and tag references.
  • A live branch remains when its snapshot is within the backfilled seven-day maximum reference age.
  • A configured table-level maximum reference age is preserved and keeps live branch and tag references and their snapshots.
  • New tables receive the seven-day default while an existing configured value is preserved.

The OperationsTest suite passes for the Iceberg 1.2 and Iceberg 1.5 application variants. The table repository tests and affected Spotless checks also pass.

Additional Information

  • Breaking Changes
  • Deprecations
  • Large PR broken into smaller PRs, and PR plan linked in the description.

mkuchenbecker and others added 3 commits September 2, 2026 08:59
Persist history.expire.max-ref-age-ms during snapshot-expiration maintenance when a table has not configured the property. Preserve explicit table-level values and cover both behaviors across the shared Iceberg application variants.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Apply the seven-day maximum reference age through an in-memory metadata
view. Reconstruct expiration commands after commit conflicts so explicit
table settings win without an extra table-property commit.

BDP-106337

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Apply the configured maximum reference age during snapshot maintenance,
defaulting to seven days when unset. Remove eligible branches before
snapshot expiration in the same Iceberg transaction so released snapshots
can be cleaned without mutating table properties.

BDP-106337

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Comment thread apps/spark/src/main/java/com/linkedin/openhouse/jobs/spark/Operations.java Outdated
Inject a clock into operations so reference-expiration tests advance time
deterministically. Enable expired-file cleanup after branch references are
removed.

BDP-106337

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Comment thread apps/spark/src/main/java/com/linkedin/openhouse/jobs/spark/Operations.java Outdated
@mkuchenbecker
mkuchenbecker marked this pull request as ready for review September 2, 2026 23:48
mkuchenbecker and others added 3 commits September 2, 2026 16:57
Create a branch on an old snapshot, advance the maintenance clock beyond
the default reference age, and verify the branch and released snapshot are
removed by the real Iceberg operation.

BDP-106337

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Verify the branch exists before snapshot maintenance so the integration test
proves the operation removes an established branch reference.

BDP-106337

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replace mocked reference-selection tests with real Iceberg table scenarios
for the seven-day default, table-level retention, branch-level expiration,
and released snapshot cleanup.

BDP-106337

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@mkuchenbecker mkuchenbecker changed the title Default Iceberg reference expiration to seven days Transactionally expire branches with Snapshot Expiration Sep 3, 2026
Comment thread apps/spark/src/main/java/com/linkedin/openhouse/jobs/spark/Operations.java Outdated
Comment thread apps/spark/src/main/java/com/linkedin/openhouse/jobs/spark/Operations.java Outdated
Comment thread apps/spark/src/main/java/com/linkedin/openhouse/jobs/spark/Operations.java Outdated
Comment thread apps/spark/src/main/java/com/linkedin/openhouse/jobs/spark/Operations.java Outdated
mkuchenbecker and others added 2 commits September 3, 2026 10:59
Report the number of branch references removed after the snapshot-expiration
transaction commits.

BDP-106337

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Integrate upstream snapshot-expiration changes while preserving stale branch cleanup and metadata-only file handling by default.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
teamurko
teamurko previously approved these changes Sep 4, 2026
@cbb330

cbb330 commented Sep 4, 2026

Copy link
Copy Markdown
Collaborator

Iceberg already executes this impl on the expireSnapshots commit.

The only real behavior change is our 7-day default vs iceberg's forever.

The extra removeBranch pass duplicates computeRetainedRefs, skips tags, and splits one atomic expire into two commits

Can we instead do this:

  1. ensure all tables have max-ref-ages set in table properties on create
  2. disable removing it
  3. set guardrails on updating it
  4. and then let iceberg's default snapshot expiration act on it?
  5. backfill it to existing tables (or update the commit path to add it if it isn't there from create)

This would fix the ability for us to handle tags.

Also, there is a remaining TODO: which is for branches that commit often. they would not be expired using this flag. we need to expire those if they are missing a common ancestor with main (tying branch-forked-from-main-age to SE history policy)

Stamp the seven-day maximum reference age on new tables and backfill it when snapshot maintenance encounters a legacy table. Delegate branch and tag expiration to native Iceberg reference handling.

BDP-106337

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
mkuchenbecker added a commit that referenced this pull request Sep 4, 2026
## Stack

- Base: [PR 708](#708)
- **This PR**

## Summary

Replace the custom branch-reference removal pass from PR 708 with
Iceberg's native reference expiration.

Snapshot-expiration maintenance backfills
`history.expire.max-ref-age-ms` to seven days when the property is
missing. New tables receive the same default during creation, while an
existing configured value is preserved. Iceberg then expires eligible
branches and tags through its standard snapshot-expiration commit.

## Changes

- [ ] Client-facing API Changes
- [ ] Internal API Changes
- [x] Bug Fixes
- [ ] New Features
- [ ] Performance Improvements
- [ ] Code Style
- [x] Refactoring
- [ ] Documentation
- [x] Tests

## Testing Done

- [ ] Manually Tested on local docker setup. Please include commands
ran, and their output.
- [x] Added new tests for the changes made.
- [x] Updated existing tests to reflect the changes made.
- [ ] No tests added or updated. Please explain why. If unsure, please
feel free to ask for help.
- [ ] Some other form of testing like staging or soak time in
production. Please explain.

A real Iceberg table integration test removes the property to simulate a
legacy table, creates a branch and tag on an older snapshot, runs
snapshot expiration, and verifies that the property is backfilled and
both references expire. The integration test passes with Iceberg 1.2 and
Iceberg 1.5.

The full `OperationsTest` suite passes for both application variants.
`OpenHouseInternalRepositoryImplTest` verifies the table-creation
default and preservation of a configured value. The affected Spotless
checks pass.

# Additional Information

- [ ] Breaking Changes
- [ ] Deprecations
- [ ] Large PR broken into smaller PRs, and PR plan linked in the
description.

Co-authored-by: mkuchenbecker <mkuchenbecker@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
mkuchenbecker and others added 3 commits September 4, 2026 12:29
Add real-table coverage proving snapshot expiration retains a branch whose
snapshot is within the backfilled seven-day maximum reference age.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Exercise table-level reference expiration, longer branch-specific retention,
and the SparkActions file-cleanup path with a real Iceberg table.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Explain the contract exercised by each reference-expiration and table-creation
test added for BDP-106337.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@abhisheknath2011

abhisheknath2011 commented Sep 8, 2026

Copy link
Copy Markdown
Member

Iceberg already executes this impl on the expireSnapshots commit.

The only real behavior change is our 7-day default vs iceberg's forever.

The extra removeBranch pass duplicates computeRetainedRefs, skips tags, and splits one atomic expire into two commits

Can we instead do this:

  1. ensure all tables have max-ref-ages set in table properties on create
  2. disable removing it
  3. set guardrails on updating it
  4. and then let iceberg's default snapshot expiration act on it?
  5. backfill it to existing tables (or update the commit path to add it if it isn't there from create)

This would fix the ability for us to handle tags.

Also, there is a remaining TODO: which is for branches that commit often. they would not be expired using this flag. we need to expire those if they are missing a common ancestor with main (tying branch-forked-from-main-age to SE history policy)

Thanks @cbb330 for the details!

@mkuchenbecker as the PR is updated now, can we update the PR description/headline as well before merging?

@mkuchenbecker mkuchenbecker changed the title Transactionally expire branches with Snapshot Expiration Backfill history.expire.max-ref-age-ms with Snapshot Expiration Sep 8, 2026
@mkuchenbecker
mkuchenbecker merged commit 252478b into linkedin:main Sep 8, 2026
1 check passed
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.

4 participants