Skip to content

feat: Let each data source decide whether FDv2 writes its data to a persistent store - #390

Open
kinyoklion wants to merge 2 commits into
dotnet-server-sdk-v9from
rlamb/fdv2-persist-flag-v9
Open

kinyoklion wants to merge 2 commits into
dotnet-server-sdk-v9from
rlamb/fdv2-persist-flag-v9

Conversation

@kinyoklion

@kinyoklion kinyoklion commented Oct 30, 2026 •

Copy link
Copy Markdown
Member

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

  • ChangeSet and FullDataSet carry 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.
  • WriteThroughStore writes 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.
  • InMemoryDataStore remembers the flag of the latest full or partial change set and returns it from ExportAll. Like the transactional persistent store spec (requirement 1.4.3.2), it tracks a single, most recent indicator, not one per item.
  • PersistentStoreWrapper.PollAvailabilityAfterOutage skips the write when the exported data must not be persisted. The store still reports that it has recovered, and keeps its contents.
  • DataStoreSorter keeps 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 FDv1 Upsert call 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 interface IDataSourcePersistence, 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 TestData configured with shouldPersist(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

Source Persisted
FDv2 polling and streaming yes
FDv1 fallback synchronizer (DataSystemComponents.FDv1Polling()) yes
A custom data source yes, as before
FileData.DataSource() no; FileDataSourceBuilder.ShouldPersist(true) opts in
TestData no; TestData.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

  • A file initializer followed by a LaunchDarkly synchronizer's basis: the basis is written to the store and replaces whatever the store held, and a recovery after it writes the memory store.
  • A delta whose flag differs from the basis it applies to: the write-through store writes or skips each change set by its own flag, and recovery follows the flag of the latest change set, as the spec says. A persisted delta can only follow a basis that was not persisted if a source sends a delta without a basis of its own. A file source never sends deltas, TestData starts 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

  • FDv2DataSystemPersistenceTest runs real clients against a mock persistent store, for each case:
    • A file initializer is not written.
    • A file synchronizer's reload is not written.
    • File data with ShouldPersist(true) is written.
    • TestData is written neither for its initial data nor for an update by default.
    • TestData with ShouldPersist(true) is written, initial data and updates.
    • A custom FDv1-style data source is written.
    • The FDv1 fallback synchronizer's data is written.
    • A LaunchDarkly basis after a file initializer is written, and the file data is not.
    • The FDv1 data system with a file data source still writes to the store.
  • Unit tests for the write-through store, the memory store's flag of the latest change set, recovery with data that must not be persisted and with persisted data that replaced it, the adapter, the sorter, and the flag on FDv2 polling and streaming change sets.
  • To check that the tests detect the defects they target, each of eight parts of the fix was reverted on its own, and every reversion failed at least one test:
    • The write-through check.
    • The adapter's upsert flag, reverted to Java's behavior.
    • The recovery skip.
    • The memory store's flag for partial payloads.
    • Reading the configurer's flag.
    • The sorter.
    • The TestData flag.
    • The file data default.
  • The full server SDK test suite passes on net8.0 (2006 tests on this branch). The SDK builds for netstandard2.0, net462, and net8.0.

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-v9 version of #389. It differs only in the TestData default: on main, TestData stays persisted by default and ShouldPersist(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 ShouldPersist flag (default persist). WriteThroughStore writes to the backing store only when the flag is set; non-persisted data still lives in memory and drives reads. InMemoryDataStore remembers the latest payload’s flag for ExportAll, and PersistentStoreWrapper skips re-syncing after an outage when that export is not persistable.

Sources declare intent via IDataSourcePersistence: FileDataSourceBuilder.ShouldPersist(true) and TestData.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 through DataSourceUpdatesV2ToV1Adapter (including upserts, unlike Java’s reference behavior). The legacy ConfigurationBuilder.DataSource path is unchanged.

Reviewed by Cursor Bugbot for commit 89b6e0b. Bugbot is set up for automated code reviews on this repo. Configure here.

…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.
@kinyoklion

Copy link
Copy Markdown
Member Author

bugbot review

1 similar comment
@kinyoklion

Copy link
Copy Markdown
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.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ 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
kinyoklion marked this pull request as ready for review October 30, 2026 23:49
@kinyoklion
kinyoklion requested a review from a team as a code owner October 30, 2026 23:49

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant