Repository navigation
feat: Let each data source decide whether FDv2 writes its data to a persistent store - #389
Open
kinyoklion wants to merge 3 commits into
Open
kinyoklion wants to merge 3 commits into
kinyoklion wants to merge 3 commits into
Conversation
…ersistent store The FDv2 data system wrote the data of every initializer and synchronizer to a persistent store in read-write mode, and wrote the memory store back after a store outage. The data system spec requires each source to state whether its data is persisted (requirements 1.1.7 and 1.2.8), so that, for example, file data does not replace the LaunchDarkly data in a shared store. Change sets and full data sets now carry an internal persist flag. The write- through store writes a change set to the persistent store only if the flag is set, the memory store remembers the flag of the data it holds, and store recovery leaves the persistent store as it is when the memory store holds data that must not be persisted. An FDv1-style source states its flag on its configurer, and the data system applies it to the source's full data sets and upserts alike: - The file data source is not persisted by default. FileDataSourceBuilder. ShouldPersist(true) opts in. - TestData is persisted by default, as before. TestData.ShouldPersist(false) opts out, and covers later updates as well as the initial data. - FDv2 polling and streaming, the FDv1 fallback synchronizer, and custom data sources are persisted, as before. The FDv1 data system is unchanged: its data store receives all data.
Member
Author
|
bugbot review |
1 similar comment
Member
Author
|
bugbot review |
The transactional persistent store spec (requirement 1.4.3.2) has the data system track a single, most recent persistence indicator, and requires the write back whenever that indicator allows it (requirement 1.4.3). The memory store let a partial payload only clear its flag, so a persisted delta on a basis that was not persisted would have skipped the write back. Use the flag of the latest change set, as the spec, Java and Go do.
kinyoklion
marked this pull request as ready for review
October 30, 2026 23:49
… wraps theirs The data system read the persistence flag only from the configurer it was given, so an application configurer that delegates to FileData.DataSource() made file data persisted again. The file and test data sources now also put the flag on the full data sets they deliver. The adapter persists a full data set only if both its configurer and the data set allow it, and an upsert takes the flag of the last full data set, the data it changes. Also from the review of this change: - The TestData docs said a change to ShouldPersist applies to clients created after it. The data system reads it each time it starts the source, which a running client also does. - The mixed-sequence test now checks the store's Init count: the mock store's Init replaces all its data, so the old assertion could not fail. - WriteThroughStore.Upsert says why it ignores the flag, the recovery skip is logged at Info, and the read-write mode docs name the FDv1 fallback, custom data sources and the write back after an outage.
Member
Author
|
bugbot review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 982d5c9. Configure here.
This was referenced Nov 2, 2026
This branch has not been deployed
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
In the FDv2 data system with a persistent store in read-write mode, the SDK wrote the data of every initializer and synchronizer to the store, and wrote all of the memory store back to it after an outage. A file data source used as an initializer or synchronizer therefore replaced the LaunchDarkly data in a shared Redis, DynamoDB, or Consul store, on start, on every reload, and again on every store recovery.
The data system spec requires each data source to say whether its data may be persisted, and the SDK to mirror only that data: requirement 1.1.7 for initializers and 1.2.8 for synchronizers (
specs/DATASYSTEM-data-system/v2/README.md). Its example is the file initializer, which does not persist. The decision is not based on the payload selector, because the FDv1 fallback synchronizer has no selector and its data must still be persisted (the spec's definition of a basis notes that neither a file data source nor the FDv1 fallback synchronizer can provide one).The Java SDK is the complete reference implementation, and this change follows its design, with one difference described below.
How it works
ChangeSetandFullDataSetcarry an internal persist flag. It is stored inverted, so that a defaulted struct, or one built with an existing constructor, means "persist". The public API of both types is unchanged.WriteThroughStorewrites a full data set or a change set to the persistent store only if its flag is set. Data that is not persisted still goes to the memory store, which becomes the store that evaluations read from, as before.InMemoryDataStoreremembers the flag of the latest full or partial change set and returns it fromExportAll. Like the transactional persistent store spec (requirement 1.4.3.2), it tracks a single, most recent indicator, not one per item.PersistentStoreWrapper.PollAvailabilityAfterOutageskips the write when the exported data must not be persisted. The store still reports that it has recovered, and keeps its contents.DataStoreSorterkeeps the flag when it sorts a data set or change set.How a data source states its flag
FDv2 polling and streaming create their change sets with the flag set.
Every other source is an FDv1-style source run through
DataSourceUpdatesV2ToV1Adapter. An FDv1Upsertcall has no data set that could carry a flag. So the data system reads the flag from the source's configurer each time it builds the source, through the internal interfaceIDataSourcePersistence, and gives it to the adapter. A configurer that does not implement the interface is persisted.The file and test data sources also put the flag on the full data sets they deliver, so that their data stays out of the store when an application's own configurer wraps theirs and hides the interface. The adapter persists a full data set only if both the configurer and the data set allow it, and an upsert takes the flag of the last full data set, the data it changes.
This is where the change differs from Java. Java's adapter copies the flag from the full data set but always sets it on upserts, so a Java
TestDataconfigured withshouldPersist(false)keeps its initial data out of the store but writes every later update to it. After one update the memory store's flag says "persist", so the next store recovery writes the initial data as well.Defaults
DataSystemComponents.FDv1Polling())FileData.DataSource()FileDataSourceBuilder.ShouldPersist(true)opts inTestDataTestData.ShouldPersist(false)opts outThe FDv1 data system is unchanged: its data store, persistent or not, receives all data, including the file data source's. The flag is read only by the FDv2 data system.
Behavior change: an application that uses the file data source as an FDv2 initializer or synchronizer with a read-write persistent store no longer has the file data written to the store.
FileDataSourceBuilder.ShouldPersist(true)restores that.Mixed sequences
TestDatastarts with a full data set, and an FDv2 synchronizer that follows a basis without a selector (the file and test data sources provide none) requests a full transfer.Testing
FDv2DataSystemPersistenceTestruns real clients against a mock persistent store, for each case:ShouldPersist(true)is written.TestDatais written by default, initial data and updates.TestDatawithShouldPersist(false)is written neither for its initial data nor for an update.Initcount shows it, because the mock store'sInitreplaces all of its data.TestDatabehind an application's own wrapping configurer is not written.TestDataflag.TestData's data set.The contract tests do not cover this. The harness persistence suites have no test of file or test data, and this repository runs them with
enable_persistence_tests: 'false'; the contract test service has no persistent store support. A separate change enables them, and the harness gains tests of these cases.#390 makes the same change on
dotnet-server-sdk-v9, whereTestDatais not persisted by default.Found in review, not changed here
A multi-agent review of this change found three gaps in code it does not change. Java has the same three:
PollAvailabilityAfterOutageand writes an empty data set. Consul'sInitdeletes every key under the prefix that is not in the data, so this empties a shared Consul store.WriteThroughStorestops applying the rest of the delta and throws, so the SDK reports a store error and skips the flag change events.Opened from termag
Note
Overview
The FDv2 data system no longer blindly mirrors every initializer/synchronizer (and post-outage memory snapshots) into a read-write persistent store.
FullDataSetandChangeSetnow carry an internalShouldPersistflag (default persist), andWriteThroughStore,InMemoryDataStoreexport, andPersistentStoreWrapperrecovery honor it so file/test data can stay memory-only while LaunchDarkly traffic still writes through.Source configuration: new internal
IDataSourcePersistence;FileDataSourceBuilder.ShouldPersistdefaults off (opt-in to write file loads/reloads);TestData.ShouldPersistdefaults on (opt out withfalse). File and test sources also attach the flag on full payloads so wrapped configurers cannot accidentally persist.DataSourceUpdatesV2ToV1AdapterANDs configurer + dataset flags on inits and ties FDv1-style upserts to the last full dataset flag (differs from Java forTestDatawith persist false).Behavior change: file data used as an FDv2 initializer/synchronizer with a shared store no longer replaces Redis/Dynamo/Consul data unless
ShouldPersist(true). FDv1ConfigurationBuilder.DataSourcepath unchanged.Reviewed by Cursor Bugbot for commit 982d5c9. Bugbot is set up for automated code reviews on this repo. Configure here.