Skip to content

feat: Complete initialization when initializers return data without a selector - #441

Open
kinyoklion wants to merge 3 commits into
v7from
rlamb/fdv2-init-without-selector
Open

feat: Complete initialization when initializers return data without a selector#441
kinyoklion wants to merge 3 commits into
v7from
rlamb/fdv2-init-without-selector

Conversation

@kinyoklion

@kinyoklion kinyoklion commented Sep 1, 2026

Copy link
Copy Markdown
Member

Summary

The FDv2 data system now completes initialization once all initializers have run and any of them provided data, even when that data has no selector. Previously only a basis with a selector completed initialization from the initializer phase; selector-less data was applied to the store but readiness waited on a synchronizer.

This follows the updated data system spec: Requirement 1.1.5 (a basis produced by the initialization process initializes the memory store) and Requirement 1.3.11 (an initialized data system drives SDK readiness). Init termination (Requirement 1.1.3) is unchanged: a selector-less basis does not stop the loop early. Java and .NET already behave this way; Go was the outlier.

How readiness is decided:

  • Store.Apply (internal) now reports whether it applied data. The data system tracks that at the orchestration level, as Java and .NET do. Both the readiness signal and InitializationSucceeded (the internal method the blocking client constructor consults; replaces TargetAvailability) are keyed on "a data source provided data that was applied", so they cannot disagree.
  • A basis that carries no data, or data that cannot be applied, is treated as an initializer that failed: the SDK logs it, moves to the next initializer, and otherwise lets the synchronizers decide. For LaunchDarkly's own sources this input is a protocol violation (a none intent or 304 in reply to a request with no basis); it is reachable from third-party initializers.
  • A persistent store is a data store, not a data source. Data persisted by a previous SDK instance keeps evaluations working and keeps Initialized() true, but does not make client construction report success when every data source has failed. A configuration with no data sources (daemon mode) is successful as-is, unchanged. FDv1 semantics are unchanged: success still requires the data source to have initialized.
  • When the initializer phase completes initialization, the data source status is set to Valid before the readiness signal (parity with Java). A nil constructor error therefore implies a Valid status.

Behavior changes relative to v7:

  1. An initializer-only configuration with selector-less data (for example a file initializer alone) now succeeds; it previously returned ErrInitializationFailed.
  2. A synchronizer-only configuration that delivers selector-less data (for example the file data source as a synchronizer) now succeeds; it previously returned ErrInitializationFailed.
  3. An initializer response that carries data together with an FDv1 fallback directive now completes initialization with that data (matching .NET and spec sections 1.1.3 and 1.6.2), rather than waiting for the FDv1 synchronizer.
  4. Status is Valid as soon as initialization completes from initializer data. One consequence: an application that waited for Valid to learn that a synchronizer had refreshed selector-less initializer data no longer has that signal; a public data availability API (REFRESHED vs CACHED, spec Requirement 1.3.5) would be the right way to expose it.
  5. A malformed selector-ful initializer payload no longer fails construction immediately; like other unusable initializer results it defers to the remaining sources.

Known and deliberately out of scope (pre-existing): daemon mode leaves the data source status at Initializing; Initialized() counts persistent-store data (Java and .NET do not); the streaming synchronizer's first connection cannot be cancelled by Close() while it is dialing; a delta before any basis is a protocol violation and is not special-cased; the wording of spec Requirement 1.3.11 item 3 (populated persistent store) should be clarified with the spec owners.

Validated with the full unit suite, golangci-lint, and contract test harnesses v2 and v3 including the persistence suites against redis, consul, and dynamodb. Each new end-to-end test fails against the behavior it replaces (the v7 base or an earlier revision of this branch) and passes with these changes.

@kinyoklion
kinyoklion marked this pull request as ready for review September 2, 2026 20:20
@kinyoklion
kinyoklion requested a review from a team as a code owner September 2, 2026 20:20
}

//nolint:revive // Data system implementation.
func (f *FDv1) TargetAvailability() DataAvailability {

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The data system isn't controlling the availability in this case. Just if the data source has been initialized with some data.

If we add the ability to wait for a specific availability, that would potentially move this up a level.

If we did, we would want to differentiate between "cached in a store" and "data without selector".

@kinyoklion
kinyoklion force-pushed the rlamb/fdv2-init-without-selector branch from fcb3ea1 to db5a2bb Compare September 2, 2026 21:54

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

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

❌ 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 db5a2bb. Configure here.

}
if basis != nil && f.applyBasis(*basis, initializer.Name()) {
f.loggers.Infof("Applied payload from %s before falling back to FDv1", initializer.Name())
f.completeInitialization(closeWhenReady)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Fallback skips prior initializer success

Medium Severity

A later initializer that requests FDv1 fallback without its own data returns immediately, so the post-loop path that completes initialization from earlier selector-less data never runs. MakeCustomClient then blocks on the FDv1 synchronizer (or times out) even though the store is already populated and dataApplied is already true.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit db5a2bb. Configure here.

Comment on lines +252 to +262
// runInitializers runs each configured initializer in order until one provides a basis with a
// selector, the context is cancelled, or an initializer signals a fallback to FDv1. A basis
// without a selector is applied to the store and the loop continues to the next initializer.
// When all initializers have run, initialization is complete if any initializer's data was
// applied to the store, even if that data had no selector. A basis that carries no data, or
// data that cannot be applied, does not count; the synchronizers then decide.
// Returns (fallbackToFDv1, errorInfo): fallbackToFDv1 is true when an initializer asked the SDK
// to switch to FDv1; errorInfo describes the underlying error for status reporting when no FDv1
// fallback is configured (empty when the fallback accompanied a successful response). If fallback
// is signalled alongside a basis, that basis is applied before returning so evaluations can serve
// the server-provided data while the FDv1 synchronizer spins up.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Docs nit, no change in content — the information here is all worth keeping, but the form is a bit off from the rest of the repo.

Two things:

  1. Returns (fallbackToFDv1, errorInfo): is the only instance of that tuple framing in the repo (grep -rn "Returns (" returns just this line). Since the results are already named in the signature, the house style documents them by name in their own paragraphs — see MakeClient, which gives each outcome its own short paragraph ("If the connection succeeded, the first return value is..., and the error value is nil.").

  2. No paragraph breaks. Everything in internal/datasystem is 2–4 lines, and this block is now 11 in one run. The public API goes much longer than this but always splits with a bare // per outcome. Splitting the three concerns here — loop termination, the readiness rule, and the two results — makes it scannable without cutting anything.

Suggested change
// runInitializers runs each configured initializer in order until one provides a basis with a
// selector, the context is cancelled, or an initializer signals a fallback to FDv1. A basis
// without a selector is applied to the store and the loop continues to the next initializer.
// When all initializers have run, initialization is complete if any initializer's data was
// applied to the store, even if that data had no selector. A basis that carries no data, or
// data that cannot be applied, does not count; the synchronizers then decide.
// Returns (fallbackToFDv1, errorInfo): fallbackToFDv1 is true when an initializer asked the SDK
// to switch to FDv1; errorInfo describes the underlying error for status reporting when no FDv1
// fallback is configured (empty when the fallback accompanied a successful response). If fallback
// is signalled alongside a basis, that basis is applied before returning so evaluations can serve
// the server-provided data while the FDv1 synchronizer spins up.
// runInitializers runs each configured initializer in order, stopping early when one provides a
// basis with a selector, the context is cancelled, or an initializer signals a fallback to FDv1.
// A basis without a selector is applied to the store and the loop continues to the next
// initializer.
//
// Once all initializers have run, initialization is complete if any initializer's data was
// applied to the store, even if that data had no selector. A basis that carries no data, or data
// that cannot be applied, does not count; the synchronizers then decide readiness.
//
// fallbackToFDv1 is true when an initializer asked the SDK to switch to FDv1. If the fallback is
// signalled alongside a basis, that basis is applied before returning, so evaluations can serve
// the server-provided data while the FDv1 synchronizer spins up.
//
// errorInfo describes the underlying error for status reporting when no FDv1 fallback is
// configured. It is empty when the fallback accompanied a successful response.

@jsonbailey jsonbailey left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I left a nit on the comment, it just needs some paragraph breaks. The cursor comment about fallbacks should be looked at.

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.

2 participants