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.
**Core issue.** Hudi decides whether a file's commit is committed by where
the commit sits relative to the timeline, not by looking it up in the archive:
```
committed = commit is on the active timeline
OR commit < start of the active timeline (so it must be
archived)
```
That only works if everything before the start is archived and everything
after it is active. Archival beyond savepoint breaks that:
```
archived: C2 C3 C4 │ active: C1(savepoint) ░░hole░░ C5 C6
↑
start of the active timeline = C1
```
- C3 is not on the active timeline.
- C3 is not before C1.
- So C3 is treated as **uncommitted**, even though it committed successfully.
`getFirstNonSavepointCommit()` is meant to skip C1, but it only can when it
sees C1's SAVEPOINT instant. The file system view
(`AbstractTableFileSystemView#refreshTimeline`), cleaner, index lookups and log
readers all pass `getWriteTimeline()`, which has no SAVEPOINT instants. So the
start stays at C1, and file slices written in the hole are hidden
(`HoodieFileGroup#isFileSliceCommitted`). Every symptom follows from this one
wrong boundary: rows missing from reads, duplicate inserts from upserts, files
the cleaner never removes, and broken incremental reads and restores.
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]
