Conversation
Task 1 of the refcounted-generation retirement plan. Pin current EL BaseRoSnapshots refcount/reclaim behavior before extracting the core: - I1 visibility-only recompute keeps hidden-but-dirty segments on disk - I3 drain gate + oldest->current stacked reclaim order - pinned-read survives concurrent retire (hazard pointer + drain gate) Production code unchanged; existing tests remain the oracle.
New payload-opaque visibleGenerations[P] / generation[P] machinery that both the EL block-snapshot and CL caplin-state readers will share. The core touches only refcnt/retired/next/oldest and the shared DirtySegment; keying, payload iteration and watermarks live entirely in each consumer's P. acquire/release (hazard-pointer pin + drain-gated reclaim), publish (eager reclaim inline under lock), and reclaim (off-lock, release-path timing) mirror the EL recalc tail byte-for-byte. Focused visibleGenerations[int] unit tests prove immediate delete on a drained publish, deferral while pinned, and the hazard-pointer re-check.
Replace the string-keyed caplin state-snapshot collections with a first-class CaplinStateType enum (like snaptype.Enum). KeyValueGetters, dirty, the visible sync.Map, CaplinStateView.roTxs, and the Get/VisibleSegment/VisibleSegments/ coveredRangesForType signatures are now enum-indexed. String() returns the exact kv.* table constant embedded in the .seg name, so file discovery and DB access stay name-compatible (no re-dump on an existing datadir). Reader bridges (state_accessors, antiquary), DumpCaplinState planning, and the schema/publishable-check paths map string<->enum at the boundary, so reads and file naming are byte-identical. Pure representation refactor, no behavior change.
…hots; consolidate lock
Split the two drain-gated dispositions in the refcounted-generation core: retired files are unlinked on drain; detached files only have their fds closed (kept on disk). CaplinStateSnapshots dirty-removal now flows entirely through generation publish: - RemoveOverlaps retires covered files to the outgoing generation and unlinks them only once no reader pins it (temp View pin forces the drain, so with no other reader they unlink by return). No more eager off-lock unlink that could race a live Beacon-API/historical reader. - Close and OpenList stale-cleanup are close-only (never unlink): closeWhatNotInList becomes detach-only detachNotInList; OpenList does one publish after opening the new files so no reader ever sees a transient set missing them.
…ermarks + concurrent live-removal Review follow-ups on the caplin-state refcount work: - CaplinStateView.roTxs was allocated on every View() (33 non-owning RoTx wrappers) but never read: the read path goes through the pinned generation payload directly, and Close() no longer touches it. Removed the field and the per-View build loop; LS/SegFileNames now range the pinned payload segments. Removes an allocation from the hot Get() path. - Add watermark coverage (BlocksAvailable/IndicesMax/SegmentsMax and idxAvailabilityFrom) — the I4 min-across-configured-types computation had none. - Add a -race concurrent readers-vs-RemoveOverlaps test for the CL side. - Drop a leftover commented-out segments block; note the visibility/retirement core in db/agents.md.
…clusion, not unlink-while-open
…core Address review on #22400: drop the retired/detached split the CL side added on top of the extracted core — the Aggregator and the block path have only `retired`. Close now closes fds directly guarded by refcnt (never unlinks), OpenList/OpenFolder retire files gone from disk, and RemoveOverlaps hands covered subsets to publish as retired (drain-gated reclaim does close+unlink). Rename to match the Aggregator: `gens` field -> `_visibleFiles`, the current generation pointer -> `visible`, `currentPayload()` -> `current()`, and drop `publishDisposing`. The init node is `empty` (current and oldest alias it), mirroring the Aggregator's visible/oldestVisible setup.
The Close fd-close guard checked only the outgoing current generation's refcnt. An older generation can still be pinned by a live view and reference the same DirtySegment pointers, so closing their fds directly niled a Decompressor out from under that reader (use-after-close). Gate the direct close on the whole generation chain having drained (oldest == current after publishing the empty generation) instead. Strictly safer — it can only leave more fds open at shutdown, never close one early — and still never unlinks. Applied to both the block path and caplin state so they stay aligned; add a caplin regression test for the pinned-older-view case.
|
Also hardened `Close` in a follow-up commit (independent of the review points): the fd-close guard checked only the outgoing generation's refcnt, but an older generation can still be pinned by a live view and reference the same segments — closing their fds directly was a use-after-close. It now gates on the whole generation chain having drained (`oldest == current`). Applied to both the block path and caplin state so they stay aligned; added a caplin regression test. Still never unlinks. |
| // drained reports whether no generation is pinned: after a publish, the chain has collapsed to | ||
| // the single current generation, so no reader can still reach an older generation's segments. | ||
| // Caller must hold lock (oldest is lock-guarded). | ||
| func (g *visibleGenerations[P]) drained() bool { |
| // reclaimRetiredLocked walks the oldest->visible chain from the head, collecting the retired | ||
| // files of every fully-drained generation older than the current one. Caller must hold lock; | ||
| // the returned files are closed and unlinked by the caller off-lock. | ||
| func (g *visibleGenerations[P]) reclaimRetiredLocked() (toRemove []*DirtySegment) { |
| retired := s.detachNotInList(fileNames) | ||
| defer s.recalcVisibleFiles(retired) // LIFO: runs before Unlock, so publish holds the lock |
| // generation can reference these same segments, so closing them would nil a Decompressor | ||
| // out from under that reader. At shutdown leaking the fds is preferable to that | ||
| // use-after-close. | ||
| if s._visibleFiles.drained() { |
There was a problem hiding this comment.
mmm... seems it's bug:
-
Close/Remove files can only "last alive reader"
-
"last alive reader" it's not "latest generation" - it can have any generation (can
begin1 hour ago) -
Reader is last when
refcnt == 0. See in aggif v.refcnt.Add(-1) == 0 { -
BaseRoSnapshotsit's not aReader- it'sdbobject. It creating readers by.View()method -
In
kv_mdbx.go:DBandAggregatorobjects:db.Closewaiting for alive readers to complete - and then "just close all files without any checks". See:Agg: a.background.Wait()(background concurrent.ClosingWaitGroup).DB: db.waitTxsAllDoneOnClose(). In BlockFiles: i plan to pin View object inside temporal.Tx object - so it will rely on underlying guards. In Caplin - not sure what right to do - maybeconcurrent.ClosingWaitGroupmaybe something else.
By "wait on close" i mean: "cancel root context and wait for rotx to finish"
…ws cleanup TestCaplinStateCloseKeepsFdsForPinnedOlderGeneration leaves the pinned generation's segment fds open (Close keeps them open by design while a view pins an older generation); on Windows t.TempDir cleanup then can't unlink the still-mmapped index file. Release the pin and close the segments before returning. POSIX was unaffected — it unlinks open files fine.
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.
|
Closing — superseded on main. The generic refcounted visible-generation retirement substrate this set out to build now exists, built from the db/state side instead of extracted from BaseRoSnapshots: #21397 (refcnt on visibleFiles), #22246 (block refcnt → per-visibleFiles, like state), #22365 (retire only visible files), #22661 (db/mvcc RetireReason + canDelete, drain-gated reclaim). That unifies block↔state retiring from the opposite direction to this PR, and the latest-generation reclaim here carries the last-alive-reader ≠ latest-generation bug noted in review. The one piece still unbuilt — live-safe RemoveOverlaps for caplin state snapshots (the prerequisite for the state merge tier) — will be re-cut as a smaller PR on top of the mvcc substrate rather than shipping a separate visibleGenerations[P]. |
Caplin state snapshots can't safely reclaim files while live readers exist.
RemoveOverlapsunlinks directly and is documented offline-only, because state views are not refcounted — a live Beacon-API/historical reader that already selected a segment can racecloseAndRemoveFilesand get a nil/panic/corrupt read. EL block snapshots already solve this with refcounted visible generations; state lacked the equivalent, which blocks live snapshot merging.Changes
BaseRoSnapshotsinto a generic, payload-opaquevisibleGenerations[P](hazard-pointer pin, oldest→current drain-gated reclaim, per-generation retired chain). A retired file is unlinked only after every generation that referenced it drains torefcnt == 0.BaseRoSnapshotsonto the core — block behavior unchanged; the existing generation tests (snapshots_race_test.go,snapshots_test.go) are the oracle and stay green under-race.CaplinStateTypeenum and key state visibility off it as[]VisibleSegments— same shape as blocks — replacing themap[string]keying.String()preserves the exact on-disk type names, so existing datadirs are unaffected.CaplinStateViewnow pins/releases a generation instead of reloading the shared set; the state lock split (dirtyLock/visibleLock/dirtySegmentsLock) is consolidated onto one lock guarding dirty mutation and generation publish/reclaim.RemoveOverlapsretires via publish + drain-gated reclaim (unlinks by return when no reader pins the retired generation, defers while one does); shutdown (Close) and re-open stale-cleanup close fds only and never unlink.Prerequisite for the caplin state snapshot merge tier (which needs safe live file removal), and the shared visibility/refcount substrate for later unifying block and state retiring. Draft pending review.