db/snapshotsync: remove overlapping caplin state snapshots on retire - #22256
Merged
Merged
Conversation
Caplin state snapshots had no on-disk overlap cleanup. A genesis-rooted superset (e.g. PendingDepositsDump 000000-020350) and an interior 50k subset chunk it covers both stayed on disk, so `snapshots retire` left a directory the publishable integrity check rejects as overlapping. Unlike block snapshots, caplin state has no merger to set canDelete, its segments are opened frozen so the refcount deletion path is skipped, and recalcVisibleFiles' subset check only hid a subset that sorted before its superset. Add CaplinStateSnapshots.RemoveOverlaps to delete subset files covered by a larger indexed segment, call it from doRetireCommand, make the recalcVisibleFiles subset check symmetric, and make closeAndRemoveFiles nil-safe for segments opened without an index.
awskii
requested review from
AskAlexSharov,
sudeepdino008 and
yperbasis
as code owners
July 6, 2026 07:20
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses on-disk overlap cleanup for Caplin state snapshots during snapshots retire, ensuring publishable integrity checks don’t fail when a genesis-rooted “superset” segment coexists with fully-covered subset segments.
Changes:
- Add
CaplinStateSnapshots.RemoveOverlaps()to delete fully-covered subset.seg/.idxpairs when a larger indexed segment of the same type exists. - Fix
CaplinStateSnapshots.recalcVisibleFiles()to symmetrically hide subset ranges regardless of sort order. - Add CLI wiring to invoke caplin-state overlap cleanup during retire, and add regression tests.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| db/snapshotsync/snapshots.go | Makes closeAndRemoveFiles tolerate nil index entries (but see review comment about closeIdx()). |
| db/snapshotsync/caplin_state_snapshots.go | Adds overlap-removal logic and improves subset hiding in visible segment recalculation. |
| db/snapshotsync/caplin_state_overlap_test.go | Adds tests reproducing and preventing caplin-state overlap and missing-index scenarios. |
| cmd/utils/app/snapshots_cmd.go | Calls caplin-state overlap cleanup from snapshots retire. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
AskAlexSharov
approved these changes
Jul 6, 2026
awskii
enabled auto-merge
July 6, 2026 08:39
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
Jul 6, 2026
AskAlexSharov
enabled auto-merge
July 6, 2026 12:27
This was referenced Jul 8, 2026
AskAlexSharov
added a commit
that referenced
this pull request
Jul 8, 2026
…retire (#22317) Cherry-pick of #22256 (`e21f40c3dbab`) to `release/3.5`. ## r3.5-specific adaptations Also cherry-picks its prerequisite #21901 (`270a20b084`, `coveredRangesForType`) — not present on release/3.5, needed for the test to typecheck. --------- Co-authored-by: Alex Sharov <AskAlexSharov@gmail.com> Co-authored-by: moskud <sudeepdino008@gmail.com>
AskAlexSharov
added a commit
that referenced
this pull request
Jul 10, 2026
…ts (#22323) Backport of #22294 to `release/3.5`, stacked on #22317 (its caplin_state_snapshots.go base — #21901/#22256). Includes the fsync-grace-period removal. Retarget base to release/3.5 once #22317 merges. --------- Co-authored-by: Alex Sharov <AskAlexSharov@gmail.com> Co-authored-by: moskud <sudeepdino008@gmail.com>
13 of 30 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Caplin state snapshots have no on-disk overlap cleanup. When a genesis-rooted superset (e.g.
PendingDepositsDump000000-020350) coexists with an interior 50k chunk it covers (020250-020300), both stay on disk andsnapshots retireleaves a directory the publishable integrity check rejects as overlapping. Unlike block snapshots, caplin state has no merger to mark constituents for deletion, its segments are openedfrozenso the refcount-based deletion path is skipped, and therecalcVisibleFilessubset check only hid a subset that sorted before its superset.Changes
CaplinStateSnapshots.RemoveOverlaps: deletes state.seg/.idxfiles fully covered by a larger indexed segment of the same type.doRetireCommand, after the blockRemoveOverlaps.recalcVisibleFilessubset check symmetric so interior/trailing subsets are hidden from the visible list.DirtySegment.closeAndRemoveFilesnil-safe for segments opened without an index.