Conversation
…he initializer Introduce a CachedFlagStore interface in the subsystems package that provides read access to cached flag data by evaluation context. Add this as a nullable field to DataSourceBuildInputs and wire it through from FDv2DataSourceBuilder using PerEnvironmentData. This plumbing enables the upcoming FDv2 cache initializer to load persisted flags without depending on package-private types. Made-with: Cursor
…uilderImpl Add FDv2CacheInitializer that loads persisted flag data from the local cache as the first step in the initializer chain. Per CONNMODE 4.1.2, the result uses Selector.EMPTY and persist=false so the orchestrator continues to the polling initializer for a verified selector. Cache miss and no-store cases return interrupted status to move on without delay. Add CacheInitializerBuilderImpl in DataSystemComponents and comprehensive tests covering cache hit, miss, no store, exceptions, and shutdown behavior. Made-with: Cursor
Prepend the cache initializer to all connection modes per CONNMODE 4.1.1. Every mode now starts with a cache read before any network initializer, providing immediate flag values from local storage while the polling initializer fetches fresh data with a verified selector. Update initializer count assertions in DataSystemBuilderTest and FDv2DataSourceBuilderTest to reflect the new cache initializer. Made-with: Cursor
aaron-zeisler
force-pushed
the
aaronz/SDK-2070/cache-initializer
branch
from
363887c to
563ec22
Compare
| * allowing the cache initializer to load stored flags without depending on | ||
| * package-private types. | ||
| */ | ||
| public interface CachedFlagStore { |
There was a problem hiding this comment.
Is it acceptable for this interface to be public? It's public because it's been added to the DataSourceBuildInputs class, which is public. As part of the "Inputs" class, it gets passed down into DataSourceBuilder.build(), where it's used to build the cache initializer.
There was a problem hiding this comment.
PerEnvironmentData is the concrete class that implements this interface.
| * A cache miss is reported as an {@link FDv2SourceResult.Status#interrupted} status, | ||
| * causing the orchestrator to move to the next initializer without delay. | ||
| */ | ||
| final class FDv2CacheInitializer implements Initializer { |
There was a problem hiding this comment.
Here's a summary of how the spec requirements and the js-core implementation were used when developing this class:
| Decision | Source | Reasoning |
|---|---|---|
Selector.EMPTY on cache result |
CONNMODE 4.1.2 | Cache is unverified; empty selector tells the orchestrator to continue to the polling initializer for a real selector |
persist=false on ChangeSet |
CONNMODE 4.1.2 | Don't re-write data we just read from cache |
Cache miss = interrupted |
js-core pattern | Fast failure so the orchestrator immediately moves on; interrupted is the correct signal (not terminalError, which would stop the chain) |
fdv1Fallback=false always |
Logic | Cache is local storage, no server headers are involved |
Nullable cachedFlagStore |
Testing pragmatism | Test contexts don't have a persistent store; graceful degradation avoids test setup burden |
In FDv2, the FDv2CacheInitializer handles cache loading as the first step in the initializer chain, making the cache load in ContextDataManager.switchToContext() redundant. Add a skipCacheLoad parameter to ContextDataManager and a setCurrentContext() method so that the FDv2 path sets the context without reading from cache, while the FDv1 path continues to load cached flags immediately. Made-with: Cursor
| } else { | ||
| // FDv1: load cached flags immediately while the data source fetches from the network. | ||
| contextDataManager.switchToContext(context); | ||
| } |
There was a problem hiding this comment.
We might be able to replace the if (config.dataSource instanceof FDv2DataSourceBuilder) { with something from ConnectivityManager here. I think ConnectivityManager has an internal variable that describes "FDv1 or FDv2?", but that internal variable isn't exposed via a getter function. Let me know if you think that's cleaner.
|
@tanderson-ld The 5th (and currently last) commit updates the code in LDClient and ContextDataManager. In FDv1, flags are loaded from the cache in ContextDataManager's constructor and also in LDClient's "identify" flow. But in FDv2, loading from cache in those places is redundant now that we have the cache initializer. I could defer this commit to a separate pull request if you feel that's cleaner. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 70bd195. Configure here.
| )); | ||
| table.put(ConnectionMode.BACKGROUND, new ModeDefinition( | ||
| // TODO: Arrays.asList(cacheInitializer) — add once implemented | ||
| Collections.<DataSourceBuilder<Initializer>>emptyList(), |
There was a problem hiding this comment.
OFFLINE mode cache miss causes initialization failure regression
High Severity
Adding the cache initializer to ConnectionMode.OFFLINE causes initialization to fail when there's no cached data. Previously, OFFLINE had zero initializers and zero synchronizers, so FDv2DataSource.start() hit the hasAvailableSources()=false fast path and reported immediate success (VALID). Now, on cache miss the initializer returns interrupted, anyDataReceived stays false, no synchronizers exist, and maybeReportUnexpectedExhaustion fires — reporting DataSourceState.OFF with a failure and calling tryCompleteStart(false, ...). This leaves isInitialized() returning false for a mode where the user explicitly chose not to connect.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 70bd195. Configure here.
1 participant
Goal
Implement the FDv2 cache initializer so that cached flag data is loaded from local storage as the first step in every connection mode's initializer chain, providing immediate flag values while the network initializer fetches fresh data.
Approach
The FDv1 production code already loads cached flags during
ContextDataManager.switchToContext(). In FDv2, this cache loading is modeled as a dedicatedInitializer— the first in the chain — per CONNMODE spec 4.1.1. The implementation follows the same patterns asFDv2PollingInitializer(executor-based async, shutdown future race viaLDFutures.anyOf).A
CachedFlagStoreinterface bridges thesubsystemspackage with the package-privatePerEnvironmentData, similar to howSelectorSourcebridgesTransactionalDataStore. This is wired throughDataSourceBuildInputsand adapted inFDv2DataSourceBuilder.makeInputs().Key Behaviors
CHANGE_SETwithSelector.EMPTYandpersist=false, so the orchestrator applies the data immediately but continues to the polling initializer for a verified selector from the serverinterruptedstatus, causing the orchestrator to move to the next initializer without delayfdv1Fallbackis alwaysfalsesince the cache is local (no server headers involved)interruptedNot In Scope
Requirements
Related issues
Provide links to any issues in this repository or elsewhere relating to this pull request.
Describe the solution you've provided
Provide a clear and concise description of what you expect to happen.
Describe alternatives you've considered
Provide a clear and concise description of any alternative solutions or features you've considered.
Additional context
Add any other context about the pull request here.
Note
Medium Risk
Changes FDv2 startup/identify flag-loading flow by introducing a new cache initializer in the default pipeline and adjusting
ContextDataManagercontext switching; incorrect wiring could affect initial flag values or initialization sequencing.Overview
Adds an FDv2 cache initializer (
FDv2CacheInitializer) that reads persisted flag data (via newCachedFlagStore) and emits an immediate fullChangeSetwithSelector.EMPTY/persist=false, or an interrupted status on cache miss/no store/errors.Wires cached-flag access through
DataSourceBuildInputsandFDv2DataSourceBuilder, and updates the default FDv2 mode table so every connection mode runs the cache initializer first.Adjusts
LDClient/ContextDataManagerso FDv2 paths do not eagerly load cached flags on context changes (cache load is now handled by the initializer), while FDv1 behavior remains the same; updates/extends tests accordingly (including a newFDv2CacheInitializerTest).Reviewed by Cursor Bugbot for commit 70bd195. Bugbot is set up for automated code reviews on this repo. Configure here.