From 916cb8029043be2f8f70f7eca8d5b8737db06563 Mon Sep 17 00:00:00 2001 From: Todd Anderson Date: Wed, 27 May 2026 15:08:50 -0400 Subject: [PATCH 1/4] feat: Drop persistent-store cache after FDv2 in-memory store init Once the FDv2 in-memory store takes over as the active read source, the persistent-store wrapper's three caches are no longer consulted on reads and roughly double the in-memory footprint of flag data. Add an internal IDisableableCache capability interface implemented by PersistentStoreWrapper. The wrapper's DisableCache() atomically swaps each cache reference to null and then clears and disposes the captured instance. WriteThroughStore.MaybeSwitchStore() probes for the interface and invokes DisableCache() inside the existing _activeStoreLock, after the active read store has been flipped to the in-memory store. Existing null-guards at every cache touch site compose with the post-drop null fields without further changes. The PersistentDataStoreBuilder cache-config setters (NoCaching, CacheTime, CacheMillis, CacheSeconds, CacheMaximumEntries, CacheForever) remain functional during the bootstrap window for backward compatibility; XML documentation explains that under FDv2 they only govern that window. Naming note: the capability is named DisableCache rather than the ticket's DiscardCache so the verb conveys that the cache is off going forward, not just emptied. The interface is named IDisableableCache to mirror the existing ISettableCache precedent in the same namespace. --- .../PersistentDataStoreBuilder.cs | 11 ++ .../DataStores/PersistentStoreWrapper.cs | 29 +++-- .../Internal/DataSystem/WriteThroughStore.cs | 4 + .../src/Subsystems/IDisableableCache.cs | 22 ++++ .../PersistentStoreWrapperTestBase.cs | 103 +++++++++++++++ .../DataSystem/WriteThroughStoreTest.cs | 119 ++++++++++++++++++ 6 files changed, 281 insertions(+), 7 deletions(-) create mode 100644 pkgs/sdk/server/src/Subsystems/IDisableableCache.cs diff --git a/pkgs/sdk/server/src/Integrations/PersistentDataStoreBuilder.cs b/pkgs/sdk/server/src/Integrations/PersistentDataStoreBuilder.cs index 867e24b9..7098c059 100644 --- a/pkgs/sdk/server/src/Integrations/PersistentDataStoreBuilder.cs +++ b/pkgs/sdk/server/src/Integrations/PersistentDataStoreBuilder.cs @@ -25,6 +25,17 @@ namespace LaunchDarkly.Sdk.Server.Integrations /// .DataStore(myStore) /// .Build(); /// + /// + /// Note: under the FDv2 data system, the cache settings configured here + /// (, , + /// , , + /// , ) only govern the + /// brief bootstrap window before the in-memory store has received its + /// first full payload. Once the in-memory store takes over as the active + /// read source, the persistent-store cache is released and these settings + /// have no further effect. These options are kept for backward compatibility + /// and may be deprecated in a future major version. + /// /// public class PersistentDataStoreBuilder : IComponentConfigurer, IDiagnosticDescription { diff --git a/pkgs/sdk/server/src/Internal/DataStores/PersistentStoreWrapper.cs b/pkgs/sdk/server/src/Internal/DataStores/PersistentStoreWrapper.cs index dfd842b2..259c2227 100644 --- a/pkgs/sdk/server/src/Internal/DataStores/PersistentStoreWrapper.cs +++ b/pkgs/sdk/server/src/Internal/DataStores/PersistentStoreWrapper.cs @@ -2,6 +2,7 @@ using System.Collections.Generic; using System.Collections.Immutable; using System.Linq; +using System.Threading; using LaunchDarkly.Cache; using LaunchDarkly.Logging; using LaunchDarkly.Sdk.Internal; @@ -22,15 +23,15 @@ namespace LaunchDarkly.Sdk.Server.Internal.DataStores /// class adds the caching behavior that we normally want for any persistent data store. /// /// - internal sealed class PersistentStoreWrapper : IDataStore, ISettableCache + internal sealed class PersistentStoreWrapper : IDataStore, ISettableCache, IDisableableCache { private readonly IPersistentDataStore _core; private readonly DataStoreCacheConfig _caching; private readonly IDataStoreUpdates _dataStoreUpdates; private readonly Logger _log; - private readonly ICache _itemCache; - private readonly ICache> _allCache; - private readonly ISingleValueCache _initCache; + private ICache _itemCache; + private ICache> _allCache; + private ISingleValueCache _initCache; private readonly bool _cacheIndefinitely; private readonly List _cachedDataKinds = new List(); private readonly PersistentDataStoreStatusManager _statusManager; @@ -315,6 +316,22 @@ public bool Upsert(DataKind kind, string key, ItemDescriptor item) return updated; } + /// + /// Disables the internal cache. After this call, reads and writes go straight + /// to the underlying persistent store; subsequent invocations are no-ops. + /// + public void DisableCache() + { + var item = Interlocked.Exchange(ref _itemCache, null); + var all = Interlocked.Exchange(ref _allCache, null); + var init = Interlocked.Exchange(ref _initCache, null); + item?.Clear(); + all?.Clear(); + item?.Dispose(); + all?.Dispose(); + init?.Dispose(); + } + public void Dispose() { Dispose(true); @@ -326,9 +343,7 @@ private void Dispose(bool disposing) if (disposing) { _core.Dispose(); - _itemCache?.Dispose(); - _allCache?.Dispose(); - _initCache?.Dispose(); + DisableCache(); _statusManager.Dispose(); } } diff --git a/pkgs/sdk/server/src/Internal/DataSystem/WriteThroughStore.cs b/pkgs/sdk/server/src/Internal/DataSystem/WriteThroughStore.cs index ff6457df..a44f51f7 100644 --- a/pkgs/sdk/server/src/Internal/DataSystem/WriteThroughStore.cs +++ b/pkgs/sdk/server/src/Internal/DataSystem/WriteThroughStore.cs @@ -127,6 +127,10 @@ private void MaybeSwitchStore() lock (_activeStoreLock) { _activeReadStore = _memoryStore; + if (_persistentStore is IDisableableCache disableable) + { + disableable.DisableCache(); + } } } diff --git a/pkgs/sdk/server/src/Subsystems/IDisableableCache.cs b/pkgs/sdk/server/src/Subsystems/IDisableableCache.cs new file mode 100644 index 00000000..7f2ee9e6 --- /dev/null +++ b/pkgs/sdk/server/src/Subsystems/IDisableableCache.cs @@ -0,0 +1,22 @@ +namespace LaunchDarkly.Sdk.Server.Subsystems +{ + /// + /// Optional interface for data stores that can disable their internal cache. + /// + /// + /// This is currently for internal implementations only. + /// + internal interface IDisableableCache + { + /// + /// Disables the internal cache. After this call, the cache is no longer + /// consulted on reads and no longer populated by writes. + /// + /// + /// Implementations should clear, dispose, and dereference cache instances + /// so the memory can be reclaimed. The call must be idempotent: subsequent + /// invocations should be safe and have no further effect. + /// + void DisableCache(); + } +} diff --git a/pkgs/sdk/server/test/Internal/DataStores/PersistentStoreWrapperTestBase.cs b/pkgs/sdk/server/test/Internal/DataStores/PersistentStoreWrapperTestBase.cs index dbaa87aa..9a0598eb 100644 --- a/pkgs/sdk/server/test/Internal/DataStores/PersistentStoreWrapperTestBase.cs +++ b/pkgs/sdk/server/test/Internal/DataStores/PersistentStoreWrapperTestBase.cs @@ -640,6 +640,109 @@ public void StatusRemainsUnavailableIfStoreSaysItIsAvailableButInitFails() } } + [Fact] + public void DisableCacheIsIdempotent() + { + var wrapper = MakeWrapper(Cached); + wrapper.DisableCache(); + wrapper.DisableCache(); // second call must not throw + } + + [Fact] + public void DisableCacheIsSafeOnUncachedWrapper() + { + var wrapper = MakeWrapper(Uncached); + wrapper.DisableCache(); // no caches to release, must not throw + } + + [Fact] + public void GetAfterDisableCacheReturnsCurrentCoreState() + { + var wrapper = MakeWrapper(Cached); + var key = "flag"; + var itemv1 = new TestItem("itemv1"); + var itemv2 = new TestItem("itemv2"); + + _core.ForceSet(TestDataKind, key, 1, itemv1); + // Prime the cache. + Assert.Equal(itemv1.WithVersion(1), wrapper.Get(TestDataKind, key)); + + wrapper.DisableCache(); + + // Mutate the core behind the wrapper's back. If the cache is still + // serving reads we would see the stale itemv1. + _core.ForceSet(TestDataKind, key, 2, itemv2); + Assert.Equal(itemv2.WithVersion(2), wrapper.Get(TestDataKind, key)); + } + + [Fact] + public void GetAllAfterDisableCacheReturnsCurrentCoreState() + { + var wrapper = MakeWrapper(Cached); + var itemA = new TestItem("itemA"); + var itemB = new TestItem("itemB"); + + _core.ForceSet(TestDataKind, "keyA", 1, itemA); + // Prime the cache. + var primed = wrapper.GetAll(TestDataKind); + Assert.Single(primed.Items); + + wrapper.DisableCache(); + + _core.ForceSet(TestDataKind, "keyB", 1, itemB); + var afterDrop = wrapper.GetAll(TestDataKind); + Assert.Equal(2, afterDrop.Items.Count()); + } + + [Fact] + public void UpsertAfterDisableCacheWritesThroughToCoreOnly() + { + var wrapper = MakeWrapper(Cached); + var key = "flag"; + var itemv1 = new TestItem("itemv1"); + + wrapper.DisableCache(); + + Assert.True(wrapper.Upsert(TestDataKind, key, itemv1.WithVersion(1))); + // The write must have landed in the core, and a subsequent Get + // must reach the core (not a repopulated cache). + Assert.True(_core.Data[TestDataKind].ContainsKey(key)); + Assert.Equal(itemv1.WithVersion(1), wrapper.Get(TestDataKind, key)); + } + + [Fact] + public void InitAfterDisableCacheWritesThroughToCoreWithoutRepopulatingCache() + { + var wrapper = MakeWrapper(Cached); + var itemA = new TestItem("itemA"); + var itemB = new TestItem("itemB"); + + wrapper.DisableCache(); + + var allData = new TestDataBuilder() + .Add(TestDataKind, "keyA", 1, itemA) + .Build(); + wrapper.Init(allData); + + // The init must have written to the core. + Assert.True(_core.Data[TestDataKind].ContainsKey("keyA")); + + // If the cache had repopulated, the next ForceRemove behind the + // wrapper's back would still yield itemA on Get. With cache + // disabled, the new core state is what we observe. + _core.ForceRemove(TestDataKind, "keyA"); + _core.ForceSet(TestDataKind, "keyA", 2, itemB); + Assert.Equal(itemB.WithVersion(2), wrapper.Get(TestDataKind, "keyA")); + } + + [Fact] + public void DisposeIsSafeAfterDisableCache() + { + var wrapper = MakeWrapper(Cached); + wrapper.DisableCache(); + wrapper.Dispose(); // must not throw, even though caches are already gone + } + private PersistentStoreWrapper MakeWrapper(CacheMode cacheMode) => MakeWrapper(new TestParams { CacheMode = cacheMode, PersistMode = PersistWithMetadata }); diff --git a/pkgs/sdk/server/test/Internal/DataSystem/WriteThroughStoreTest.cs b/pkgs/sdk/server/test/Internal/DataSystem/WriteThroughStoreTest.cs index 9a68081d..0daa7896 100644 --- a/pkgs/sdk/server/test/Internal/DataSystem/WriteThroughStoreTest.cs +++ b/pkgs/sdk/server/test/Internal/DataSystem/WriteThroughStoreTest.cs @@ -497,6 +497,115 @@ public void StoreSwitching_HappensOnlyOnce() #endregion + #region Cache Disable Tests + + [Fact] + public void Apply_FirstBasis_CallsDisableCacheOnPersistentStore() + { + var memoryStore = new InMemoryDataStore(); + var persistentStore = new MockDisableableCachePersistentStore(); + + using (var store = new WriteThroughStore(memoryStore, persistentStore, + DataSystemConfiguration.DataStoreMode.ReadWrite)) + { + store.Apply(CreateFullChangeSet()); + + Assert.Equal(1, persistentStore.DisableCacheCallCount); + } + } + + [Fact] + public void Apply_SubsequentApply_DoesNotCallDisableCacheAgain() + { + var memoryStore = new InMemoryDataStore(); + var persistentStore = new MockDisableableCachePersistentStore(); + + using (var store = new WriteThroughStore(memoryStore, persistentStore, + DataSystemConfiguration.DataStoreMode.ReadWrite)) + { + store.Apply(CreateFullChangeSet()); + + // Build a delta changeset to apply on top. + var item3 = new TestItem("item3"); + var deltaData = ImmutableList.Create( + new KeyValuePair>( + TestDataKind, + new KeyedItems(ImmutableDictionary.Empty + .Add("key3", new ItemDescriptor(30, item3))) + ) + ); + var delta = new ChangeSet( + ChangeSetType.Partial, + Selector.Make(2, "state2"), + deltaData, + null); + store.Apply(delta); + + Assert.Equal(1, persistentStore.DisableCacheCallCount); + } + } + + [Fact] + public void Init_FirstCall_CallsDisableCacheOnPersistentStore() + { + var memoryStore = new InMemoryDataStore(); + var persistentStore = new MockDisableableCachePersistentStore(); + + using (var store = new WriteThroughStore(memoryStore, persistentStore, + DataSystemConfiguration.DataStoreMode.ReadWrite)) + { + store.Init(CreateTestDataSet()); + + Assert.Equal(1, persistentStore.DisableCacheCallCount); + } + } + + [Fact] + public void Apply_ReadOnlyMode_StillCallsDisableCache() + { + var memoryStore = new InMemoryDataStore(); + var persistentStore = new MockDisableableCachePersistentStore(); + + using (var store = new WriteThroughStore(memoryStore, persistentStore, + DataSystemConfiguration.DataStoreMode.ReadOnly)) + { + store.Apply(CreateFullChangeSet()); + + // Reads bypass the persistent store in both modes post-switch, + // so the cache is dead weight regardless of mode. + Assert.Equal(1, persistentStore.DisableCacheCallCount); + } + } + + [Fact] + public void Apply_NonDisablerStore_DoesNotThrow() + { + var memoryStore = new InMemoryDataStore(); + var persistentStore = new MockPersistentStore(); // does not implement IDisableableCache + + using (var store = new WriteThroughStore(memoryStore, persistentStore, + DataSystemConfiguration.DataStoreMode.ReadWrite)) + { + // The probe must be a no-op for stores that don't implement the interface. + store.Apply(CreateFullChangeSet()); + Assert.True(persistentStore.WasInitCalled); + } + } + + [Fact] + public void Apply_WithoutPersistence_DoesNotThrow() + { + var memoryStore = new InMemoryDataStore(); + + using (var store = new WriteThroughStore(memoryStore, null, + DataSystemConfiguration.DataStoreMode.ReadWrite)) + { + store.Apply(CreateFullChangeSet()); // null _persistentStore -- probe must short-circuit + } + } + + #endregion + #region Selector Tests [Fact] @@ -959,6 +1068,16 @@ public void Dispose() } } + private class MockDisableableCachePersistentStore : MockTransactionalPersistentStore, IDisableableCache + { + public int DisableCacheCallCount { get; private set; } + + public void DisableCache() + { + DisableCacheCallCount++; + } + } + private class MockTransactionalPersistentStore : MockPersistentStore, ITransactionalDataStore { public bool WasApplyCalled { get; private set; } From 99bc8a61b7c1cb7a290769cb4e1cfef897e34792 Mon Sep 17 00:00:00 2001 From: Todd Anderson Date: Wed, 27 May 2026 15:29:03 -0400 Subject: [PATCH 2/4] refactor: use volatile flag for cache disable instead of atomic swap The first cut swapped the three cache field references to null via Interlocked.Exchange and relied on the existing null-guards at every cache touch site. That left a latent TOCTOU race: the guards re-read the field a second time inside each branch (for example `if (_initCache != null) { result = _initCache.Get(); }`), so a concurrent DisableCache could null the field between the check and the inner use and a reader would observe a NullReferenceException. On x86/x64 the strong memory model usually papers over the window, but it is not guaranteed and ARM is weaker. Switch to a volatile bool _cacheDisabled flag that each touch site checks before consulting the cache. The cache field references stay readonly, so they never go null mid-life and there is no TOCTOU on the field itself; only the flag changes, and volatile gives it acquire/release semantics. The LoadingCache-style loaders on the three caches would otherwise auto- repopulate after Clear(), so the explicit bypass at each touch site is required to keep the cache empty. DisableCache() now sets the flag with a release write and then Clear()s each cache to release the flag-data contents; it does not Dispose them. Dispose() keeps its original three-line cache-disposal pattern so concurrent readers that observe _cacheDisabled == false cannot end up calling a disposed cache. All existing wrapper and WriteThroughStore tests still pass (1591 / 1591 on net8.0). --- .../DataStores/PersistentStoreWrapper.cs | 60 ++++++++++++------- 1 file changed, 39 insertions(+), 21 deletions(-) diff --git a/pkgs/sdk/server/src/Internal/DataStores/PersistentStoreWrapper.cs b/pkgs/sdk/server/src/Internal/DataStores/PersistentStoreWrapper.cs index 259c2227..90e633d6 100644 --- a/pkgs/sdk/server/src/Internal/DataStores/PersistentStoreWrapper.cs +++ b/pkgs/sdk/server/src/Internal/DataStores/PersistentStoreWrapper.cs @@ -2,7 +2,6 @@ using System.Collections.Generic; using System.Collections.Immutable; using System.Linq; -using System.Threading; using LaunchDarkly.Cache; using LaunchDarkly.Logging; using LaunchDarkly.Sdk.Internal; @@ -29,9 +28,9 @@ internal sealed class PersistentStoreWrapper : IDataStore, ISettableCache, IDisa private readonly DataStoreCacheConfig _caching; private readonly IDataStoreUpdates _dataStoreUpdates; private readonly Logger _log; - private ICache _itemCache; - private ICache> _allCache; - private ISingleValueCache _initCache; + private readonly ICache _itemCache; + private readonly ICache> _allCache; + private readonly ISingleValueCache _initCache; private readonly bool _cacheIndefinitely; private readonly List _cachedDataKinds = new List(); private readonly PersistentDataStoreStatusManager _statusManager; @@ -40,6 +39,13 @@ internal sealed class PersistentStoreWrapper : IDataStore, ISettableCache, IDisa private ICacheExporter _externalCache; private volatile bool _inited; + + // Once true, the cache is bypassed on reads and writes; entries already in + // the cache have been Clear()-ed by DisableCache(). The cache objects + // themselves remain alive until Dispose() so that any reader currently + // holding a reference does not observe a disposed cache. Volatile so the + // bypass becomes visible to other threads without an explicit lock. + private volatile bool _cacheDisabled; internal PersistentStoreWrapper( IPersistentDataStoreAsync coreAsync, @@ -113,7 +119,7 @@ public bool Initialized() return true; } bool result; - if (_initCache != null) + if (_initCache != null && !_cacheDisabled) { result = _initCache.Get(); } @@ -164,7 +170,7 @@ public void Init(FullDataSet items) kindAndItems => PersistentDataStoreConverter.SerializeAll(kindAndItems.Key, kindAndItems.Value.Items) ); Exception failure = InitCore(new FullDataSet(serializedItems)); - if (_itemCache != null && _allCache != null) + if (_itemCache != null && _allCache != null && !_cacheDisabled) { _itemCache.Clear(); _allCache.Clear(); @@ -200,7 +206,7 @@ public void Init(FullDataSet items) { try { - var ret = _itemCache is null ? GetAndDeserializeItem(kind, key) : + var ret = (_itemCache is null || _cacheDisabled) ? GetAndDeserializeItem(kind, key) : _itemCache.Get(new CacheKey(kind, key)); ProcessError(null); return ret; @@ -216,7 +222,7 @@ public KeyedItems GetAll(DataKind kind) { try { - var ret = new KeyedItems(_allCache is null ? + var ret = new KeyedItems((_allCache is null || _cacheDisabled) ? GetAllAndDeserialize(kind) : _allCache.Get(kind)); ProcessError(null); return ret; @@ -251,7 +257,7 @@ public bool Upsert(DataKind kind, string key, ItemDescriptor item) } failure = e; } - if (_itemCache != null) + if (_itemCache != null && !_cacheDisabled) { var cacheKey = new CacheKey(kind, key); if (failure is null) @@ -285,7 +291,7 @@ public bool Upsert(DataKind kind, string key, ItemDescriptor item) } } } - if (_allCache != null) + if (_allCache != null && !_cacheDisabled) { // If the cache has a finite TTL, then we should remove the "all items" cache entry to force // a reread the next time All is called. However, if it's an infinite TTL, we need to just @@ -320,16 +326,25 @@ public bool Upsert(DataKind kind, string key, ItemDescriptor item) /// Disables the internal cache. After this call, reads and writes go straight /// to the underlying persistent store; subsequent invocations are no-ops. /// + /// + /// At the time of writing, it is likely that the cache was disabled when + /// another store took over as the active store and so reads may not even get + /// to this layer anymore. If they do, they do go straight to persistence if + /// this layer's DisableCache method was called. + /// public void DisableCache() { - var item = Interlocked.Exchange(ref _itemCache, null); - var all = Interlocked.Exchange(ref _allCache, null); - var init = Interlocked.Exchange(ref _initCache, null); - item?.Clear(); - all?.Clear(); - item?.Dispose(); - all?.Dispose(); - init?.Dispose(); + if (_cacheDisabled) return; + // Volatile write publishes the bypass flag with release semantics + // before the cache contents are cleared. Any reader observing + // _cacheDisabled == true will skip the cache call sites entirely; + // any reader still in flight will complete against the cache and + // we will just clear it underneath them. The cache instances stay + // alive until Dispose so concurrent readers never observe a + // disposed cache. + _cacheDisabled = true; + _itemCache?.Clear(); + _allCache?.Clear(); } public void Dispose() @@ -343,7 +358,9 @@ private void Dispose(bool disposing) if (disposing) { _core.Dispose(); - DisableCache(); + _itemCache?.Dispose(); + _allCache?.Dispose(); + _initCache?.Dispose(); _statusManager.Dispose(); } } @@ -452,8 +469,9 @@ private bool PollAvailabilityAfterOutage() } // Fall back to cache-based recovery if external store is not available/initialized - // and we're in infinite cache mode - if (_cacheIndefinitely && _allCache != null) + // and we're in infinite cache mode. Under FDv2 this branch is dead once + // DisableCache has run: the ICacheExporter path above supersedes it. + if (_cacheIndefinitely && _allCache != null && !_cacheDisabled) { // If we're in infinite cache mode, then we can assume the cache has a full set of current // flag data (since presumably the data source has still been running) and we can just From c3e813abe693596b27b43be7c11e578c18968c0b Mon Sep 17 00:00:00 2001 From: Todd Anderson Date: Wed, 27 May 2026 15:44:30 -0400 Subject: [PATCH 3/4] docs: clarify DisableCache leak window in code comment The prior comment claimed that any in-flight reader "will complete against the cache and we will just clear it underneath them, which is correct." That overstates the safety: an in-flight reader past the flag check can fire the cache's loader on a miss and store a fresh-from-core value back into the cache after our Clear(). Those entries are unreachable from any subsequent read (every future caller observes _cacheDisabled and bypasses) and hold fresh data, so they don't cause stale reads or torn observations -- but they do persist in the cache instance until Dispose() rather than being released immediately. Replace the comment with an honest description of the window, why it is correctness-safe, and what the bounded memory cost is. --- .../DataStores/PersistentStoreWrapper.cs | 26 ++++++++++++++----- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/pkgs/sdk/server/src/Internal/DataStores/PersistentStoreWrapper.cs b/pkgs/sdk/server/src/Internal/DataStores/PersistentStoreWrapper.cs index 90e633d6..3097c4e2 100644 --- a/pkgs/sdk/server/src/Internal/DataStores/PersistentStoreWrapper.cs +++ b/pkgs/sdk/server/src/Internal/DataStores/PersistentStoreWrapper.cs @@ -335,13 +335,25 @@ public bool Upsert(DataKind kind, string key, ItemDescriptor item) public void DisableCache() { if (_cacheDisabled) return; - // Volatile write publishes the bypass flag with release semantics - // before the cache contents are cleared. Any reader observing - // _cacheDisabled == true will skip the cache call sites entirely; - // any reader still in flight will complete against the cache and - // we will just clear it underneath them. The cache instances stay - // alive until Dispose so concurrent readers never observe a - // disposed cache. + // Publish the bypass flag with release semantics before clearing the + // cache contents. Any reader that starts after this point observes + // _cacheDisabled == true and skips the cache call sites entirely, + // going straight to the core. + // + // An in-flight reader that already passed the flag check before this + // write may still call into the cache after our Clear() and, on a + // miss, fire the cache's loader -- which queries the core and then + // stores the fresh result back. Those leftover entries are unreachable + // from any subsequent read (every future caller bypasses) and hold + // fresh, correct values, so they don't cause stale reads or torn + // observations. They simply persist in the cache instance until + // Dispose() reclaims them. In practice the window is microseconds at + // FDv2 init and bounded by the number of concurrent readers in + // wrapper.Get/GetAll at that exact moment. + // + // The cache instances themselves stay alive (the fields are readonly); + // Dispose() handles their final teardown, so concurrent readers never + // observe a disposed cache. _cacheDisabled = true; _itemCache?.Clear(); _allCache?.Clear(); From 828c265f45d6317f334529c697f90d1964f7a7c0 Mon Sep 17 00:00:00 2001 From: Todd Anderson Date: Wed, 27 May 2026 15:52:18 -0400 Subject: [PATCH 4/4] removing unhelpful comment --- .../DataStores/PersistentStoreWrapper.cs | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/pkgs/sdk/server/src/Internal/DataStores/PersistentStoreWrapper.cs b/pkgs/sdk/server/src/Internal/DataStores/PersistentStoreWrapper.cs index 3097c4e2..a7e07a9f 100644 --- a/pkgs/sdk/server/src/Internal/DataStores/PersistentStoreWrapper.cs +++ b/pkgs/sdk/server/src/Internal/DataStores/PersistentStoreWrapper.cs @@ -335,25 +335,6 @@ public bool Upsert(DataKind kind, string key, ItemDescriptor item) public void DisableCache() { if (_cacheDisabled) return; - // Publish the bypass flag with release semantics before clearing the - // cache contents. Any reader that starts after this point observes - // _cacheDisabled == true and skips the cache call sites entirely, - // going straight to the core. - // - // An in-flight reader that already passed the flag check before this - // write may still call into the cache after our Clear() and, on a - // miss, fire the cache's loader -- which queries the core and then - // stores the fresh result back. Those leftover entries are unreachable - // from any subsequent read (every future caller bypasses) and hold - // fresh, correct values, so they don't cause stale reads or torn - // observations. They simply persist in the cache instance until - // Dispose() reclaims them. In practice the window is microseconds at - // FDv2 init and bounded by the number of concurrent readers in - // wrapper.Get/GetAll at that exact moment. - // - // The cache instances themselves stay alive (the fields are readonly); - // Dispose() handles their final teardown, so concurrent readers never - // observe a disposed cache. _cacheDisabled = true; _itemCache?.Clear(); _allCache?.Clear();