Repository navigation
feat: Let each data source decide whether FDv2 writes its data to a persistent store - #390
Open
kinyoklion wants to merge 2 commits into
Open
kinyoklion wants to merge 2 commits into
kinyoklion wants to merge 2 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 not persisted by default. TestData.ShouldPersist(true) opts in, and covers later updates as well as the initial data. Before this change, TestData was persisted. - 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.
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 8a329f5. Configure here.
kinyoklion
marked this pull request as ready for review
October 30, 2026 23:49
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 when it builds the source, through the internal interfaceIDataSourcePersistence, and gives it to the adapter. The adapter applies it to the change sets it creates from full data sets and upserts alike. A configurer that does not implement the interface is persisted.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(true)opts in. Before this change, it was persisted.The 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.
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 neither for its initial data nor for an update by default.TestDatawithShouldPersist(true)is written, initial data and updates.TestDataflag.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.This is the
dotnet-server-sdk-v9version of #389. It differs only in theTestDatadefault: onmain,TestDatastays persisted by default andShouldPersist(false)opts out; for the next major version it is not persisted by default, like the file data source. The file data source builder conflicts with the shared file data engine from #376 only in its using directives and in where the new method sits.Opened from termag
Note
Overview
Fixes FDv2 read-write persistent stores overwriting shared LaunchDarkly data when file or test sources act as initializers/synchronizers (including reloads and post-outage recovery).
Change sets and full data sets now carry an internal
ShouldPersistflag (default persist).WriteThroughStorewrites to the backing store only when the flag is set; non-persisted data still lives in memory and drives reads.InMemoryDataStoreremembers the latest payload’s flag forExportAll, andPersistentStoreWrapperskips re-syncing after an outage when that export is not persistable.Sources declare intent via
IDataSourcePersistence:FileDataSourceBuilder.ShouldPersist(true)andTestData.ShouldPersist(true)opt in; file and test data default to not persisted in this major version. FDv2 LaunchDarkly paths and other configurators still persist. FDv1-style sources get the flag from the configurer throughDataSourceUpdatesV2ToV1Adapter(including upserts, unlike Java’s reference behavior). The legacyConfigurationBuilder.DataSourcepath is unchanged.Reviewed by Cursor Bugbot for commit 89b6e0b. Bugbot is set up for automated code reviews on this repo. Configure here.