nsivabalan opened a new pull request, #20310:
URL: https://github.com/apache/hudi/pull/20310
### Describe the issue this Pull Request addresses
`hoodie.archive.beyond.savepoint=true` (added in 0.12.0, #5837) lets
archival skip savepointed commits and archive the commits after them. That
leaves a hole in the active timeline: the savepointed commit stays active, and
the commits after it don't. Most of the read and write path doesn't handle the
hole.
`getFirstNonSavepointCommit()` decides which commits are savepointed by
looking for SAVEPOINT instants in the timeline it's given. The file system
view, cleaner, index lookups and log readers all pass `getWriteTimeline()`,
which has no SAVEPOINT instants. So the savepointed commit counts as the start
of active history, and a commit in the hole is treated as neither active nor
archived. Its file slices are considered uncommitted
(`HoodieFileGroup#isFileSliceCommitted`).
Reproduced on master with a Spark functional test on COW. All four
combinations behave the same: table version 6 and 9, each with the BLOOM and
SIMPLE index.
- C1 inserts into partition 1 and is savepointed.
- C2 inserts 10 records into partition 2, in a file group no later commit
touches.
- Four more commits into partition 3 archive C2, while C1 stays active.
| Step | Expected | Actual |
|---|---|---|
| Snapshot read | 60 rows | **50**: C2's rows are missing |
| Upsert of C2's keys | update C2's file group | **new file group**: the
index can't see the old one |
| Delete savepoint C1, archive past it | 100 rows / 100 keys | **110 rows /
100 keys**: 10 duplicates appear |
Related: #11405 (cleaner never cleans files after the savepoint, same root
cause), #18686 (feature undocumented), #14630 / #15675 (restore and time travel
with holes), #20273 (incremental reads patched for holes).
The feature would need every boundary check reworked, and with the LSM
timeline the notion of an archived timeline is going away anyway. This PR
removes support instead of patching each caller.
### Summary and Changelog
Writers can no longer set `hoodie.archive.beyond.savepoint=true`. Archival
always stops at the earliest savepoint, as it does by default.
- `HoodieArchivalConfig.ARCHIVE_BEYOND_SAVEPOINT` and its builder method are
deprecated (`deprecatedAfter("1.3.0")`), with an updated description. The key
is kept.
- `HoodieWriteConfig` validation fails fast if it's set to `true`. `false`
is still accepted, so configs that set it explicitly keep working.
- `TimelineArchiverV1` and `TimelineArchiverV2` drop the
archive-beyond-savepoint branch.
`HoodieWriteConfig#shouldArchiveBeyondSavepoint()` is removed.
- `TimelineUtils.getEarliestInstantForMetadataArchival` drops its
`shouldArchiveBeyondSavepoint` parameter. Metadata table archival now always
stops at the data table's first active commit. On a table that already has a
hole, that is the savepointed commit, which is the conservative choice.
- Restore guard: `restoreToSavepoint` used to reject a restore only when the
restoring client had the config set. It now checks the table itself: if any
commit after the savepoint is archived, it throws `HoodieRollbackException`,
before the metadata table can be deleted. A restore would otherwise leave
behind files from archived commits after the savepoint.
- Tests:
- archive-beyond-savepoint cases removed from `TestHoodieTimelineArchiver`
and `TestTimelineUtils`
- added `TestHoodieWriteConfig#testArchiveBeyondSavepointIsRejected`
- replaced `testRestoreWithSavepointBeyondArchival` with
`testRestoreRejectedWhenCommitsAfterSavepointAreArchived`
Not in this PR: the read-side handling of holes
(`getFirstNonSavepointCommit` callers, #20273) stays as it is. Tables that
already have a hole keep it until the savepoint is deleted and archival moves
past it.
### Impact
Writers with `hoodie.archive.beyond.savepoint=true` now fail at write config
validation, with a message telling them to remove it. Once it's removed, the
active timeline keeps every commit from the earliest savepoint onward. Users
with long-lived savepoints will see a larger active timeline. Deleting old
savepoints restores normal archival.
The config key is kept, but setting it to true is now rejected, so the PR
title is marked breaking (`!`).
### Risk Level
low. Archival can only archive less than before. Restore is now rejected in
one more case: archived commits after the savepoint, where it used to leave
files behind. Ran `TestHoodieWriteConfig`, `TestTimelineUtils`,
`TestHoodieTimelineArchiver` and the new restore test locally (Spark 3.5).
### Documentation Update
The config description is updated. The website's config page is generated
from it. Release notes should call out the breaking change and the migration
step: remove the config, and delete old savepoints if the active timeline grows.
### Contributor's checklist
- [x] Read through [contributor's
guide](https://hudi.apache.org/contribute/how-to-contribute)
- [x] Enough context is provided in the sections above
- [x] Adequate tests were added if applicable
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
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]
