feat: Drop persistent-store cache after FDv2 in-memory store init#373
Open
feat: Drop persistent-store cache after FDv2 in-memory store init#373
Conversation
With FDv2 the in-memory store retains every flag and segment, so the persistent-store wrapper's cache becomes dead weight once setBasis flips the active store to memory. Drop the cache at that point to reclaim the in-memory footprint and to eliminate the CacheForever leak. The wrapper now holds its cache behind atomic.Pointer[cache.Cache]; each method does a single Load at the top and works with that snapshot, and DropCache is a one-shot Swap. The store coordinator captures any DropCache-capable persistent store via interface assertion and invokes the drop between the active-store switch and the persistent Init call, so the wrapper's Init does not rebuild a cache we are about to discard.
Apply De Morgan's law to two conditional expressions in Init and Upsert that staticcheck (QF1001) flagged, and remove the unused hasInfiniteCache helper. Behavior is unchanged.
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.
Summary
With FDv2 the in-memory store retains every flag and segment, so the persistent-store wrapper's
go-cacheinstance becomes dead weight oncesetBasisflips the active store to memory. This change drops the cache at that point to reclaim the in-memory footprint and to eliminate theCacheForeverleak.persistentDataStoreWrappernow holds its cache behindatomic.Pointer[cache.Cache]. Every method does a singleLoadat the top of the call and works with that snapshot, so concurrent reads never block on a mutex and the previous status-poller-vs-Store.mu race is closed by atomic semantics.DropCacheis a one-shotSwap(nil)followed byFlushon the returned cache; safe to call multiple times.DropCache-capable persistent store via aninterface{ DropCache() }assertion and invokes the drop insetBasis, betweens.active = s.memoryStoreand the persistentInit()call. Order matters: dropping first means the wrapper'sInitskips the cache-rebuild loop on the cache we are about to discard. Customsubsystems.DataStoreimplementations that don't satisfy the interface continue to work unchanged.CacheTime/CacheSeconds/CacheForever/NoCachingnow notes that under FDv2 the cache only governs the bootstrap window and may be deprecated in a future major version. No public API change.-raceclean.Note
Medium Risk
Touches core data-store caching and initialization paths, adding atomic cache swapping and new lifecycle behavior; mistakes could cause stale reads or missed cache writes during concurrency.
Overview
Drops the persistent-store in-process cache once FDv2 switches to the in-memory store, reclaiming memory and avoiding long-lived
CacheForevercaches after initialization.The persistent store wrapper now keeps its
go-cachebehind anatomic.Pointerand addsDropCache()(also called onClose()), updating all cache-using methods to operate on a single loaded snapshot and to tolerate concurrent cache removal.The FDv2
Storecoordinator detects persistent stores that implementDropCache()and calls it during full-basis initialization (not deltas) before persisting viaInit(). Tests were added/expanded to cover cache dropping semantics and coordinator invocation, and persistent-store cache configuration docs now clarify it only affects the bootstrap window under FDv2.Reviewed by Cursor Bugbot for commit c245085. Bugbot is set up for automated code reviews on this repo. Configure here.