blocks files: move refcnt from per-file to per-visibleFiles object (like state files) - #22246
Merged
Merged
Conversation
AskAlexSharov
changed the title
AskAlexSharov
changed the title
db/snapshotsync conflict resolution: main's #22172 extracted the per-segment `DirtySegment.refcount` close model into shared helpers and wired caplin to them. This branch removed per-segment refcount in favour of the generation model (snapshotVisible.refcnt), so the RoSnapshots close path keeps detachNotInList + generation reclamation, while the shared CloseSegmentsNotInList/closeAndDropNotProtected helpers are retained (they dedup caplin's own close code) minus the refcount check, matching caplin's existing no-per-reader-protection behaviour. Kept main's generic FindOpenSegment/ClassifyOpenErr helpers and the OpenList dedup.
Close() deferred recalcVisibleFiles, so the empty generation was published last — after the refcnt check and the segment-close loop. Since View() pins the current generation lock-free (acquireVisible's hazard-pointer re-check), a concurrent View() could load the still-current outgoing generation, increment its refcnt, pass the re-check (s.visible unchanged because the swap was deferred), and be handed segments Close was closing → use-after-close. Publish the empty generation synchronously before reading the outgoing generation's refcnt (captured as prev): a concurrent View() that pinned the outgoing generation now fails its re-check and retries onto the empty one, and any reader that pinned it earlier is counted in prev.refcnt, which we honor. Matches the synchronous mutate-then-publish order Delete already uses.
AskAlexSharov
changed the title
awskii
approved these changes
Jul 9, 2026
AskAlexSharov
added a commit
that referenced
this pull request
Jul 9, 2026
…e_36 Picks up blk_rc_36 now merged to main (#22246) plus Gloas CL (#22091). The three db/snapshotsync conflicts were purely the #22343 rename (this branch renamed snapshotsync.RoSnapshots -> BaseRoSnapshots; main's finalized blk_rc_36 kept the old name): took main's canonical reclamation code (which already includes the Close TOCTOU fix) and re-applied the rename in snapshots.go, merger.go and snapshots_race_test.go.
awskii
added a commit
that referenced
this pull request
Jul 22, 2026
Main #22246 landed its own EL refcounted visible-generation retirement core (snapshotVisible), so adopt it for the block path as-is and drop this branch's generic visibleGenerations[P] and int CaplinStateType. Caplin keeps the reader-safe retirement it added — RemoveOverlaps defers the unlink while a live view pins the generation, and Close is drain-gated so it never closes fds an older pinned view still references — but now as a concrete caplin-local generation core over main's string-keyed model, preserving #21901's per-type dump planner and the string type API.
13 of 30 tasks
awskii
added a commit
that referenced
this pull request
Aug 6, 2026
…ments input Close read the refcount of the immediately-outgoing generation only, but generations share *DirtySegment values and reclaimRetiredLocked walks the whole oldest->current chain precisely because an older one can still be pinned. A reader holding G1 across an intervening republish of G2 therefore had its segments closed under it: Decompressor and indexes went nil, so reads returned nothing or panicked in MakeGetter. Gate the close on the whole chain instead. The defect predates this branch (the gate came in with #22246) and hits EL blocks and bor/heimdall the same way; caplin riding BaseRoSnapshots just widens the exposure. openSegments scheduled one OpenIdxIfNeed per name, so a repeated name raced two goroutines on the same segment's index slice - a torn slice header plus a double recsplit.OpenIndex whose loser leaks. Deduplicate at that single funnel, which covers OpenList, OpenFolder and OpenSegments alike. No current caller passes duplicates.
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.
porting the
Aggregator's visibleFiles's refcount (lock-free, generation-chained bundle-refcount) filereclamation to the block files (
db/snapshotsync)What
Replaces the per-file two-atomic reclamation on the shared
DirtySegment(
refcount atomic.Int32+canDelete atomic.Bool, withRoTx.Closedoingif refcount==0 && canDelete { closeAndRemoveFiles() }) with an MVCC-style,oldest-reader-watermark scheme, exactly as
db/state'sAggregatoralready does:snapshotVisiblebecomes a generation node (refcnt,retired,next), forming anoldest→newest chain;
RoSnapshots.oldestVisibleis the reclaim head.acquireVisible), instead ofper-file increments — closes the
visible.Load()→pin window.generation that referenced it has drained. No file is unlinked while a reader still
mmaps it (fixes the eager-unlink hazard in
RemoveOverlaps, incl. the Windowsmmap-unlink case).
Design notes
dirtyFilesLock:dirtyLockguards bothdirtyand the generation chain.recalcVisibleFiles(alignMin, retired)is called withdirtyLockheld, so a mutator's dirty change and the bundle publish are one atomic step.RemoveOverlapsmirrorscleanAfterMerge: opens aView, retires the subsumedsegments under the lock, and the real unlink (each segment's
.seg+ indexes) happensoff-lock at
View.Close— or defers to the true watermark if another reader still holdsthe generation. No manual
removeOldFilesof tracked files.CaplinSnapshots,CaplinStateSnapshots) construct every segmentfrozen, so the per-file machinery was already inert there; they only migrate off theremoved atomics (a dead
canDeleteguard drop). No generation chain added to theappend-only frozen stores.
Tests
snapshots_race_test.go: deferred-unlink-while-View-open, pending-retired protection,and a
-racereaders-vs-retire stress that asserts the chain collapses and no files leakafter drain.
db/snapshotsync/...,freezeblocks,polygon/heimdall,polygon/bridgepass under-race.