perf: Improve dev server startup 50s->1s using streaming + REST upsert for variation names by davidbrackbill · Pull Request #733 · launchdarkly/ldcli

davidbrackbill · 2026-07-09T15:05:27Z

Problem

The LD CLI dev server starts by fetching all flags in the project, which paginates the GET /api/v2/flags/{projKey} list endpoint at 100 flags/page serially, taking up to ~50s in CI for Catamorphic.

launchdarkly/gonfalon#66623

Fix

Initial health check sync uses the stream already used for AllFlagsState
Flag variation names are not included in the above stream, so we load those in the background off the hot path
The variation names load in parallel, pulling page 0 to learn the total count and page size, then fetching the remaining pages concurrently, bounded at six concurrent requests.

Suggested by Dan

fix(dev-server): fetch flag pages concurrently instead of serially #732 (comment)

Outcome

The local healthcheck goes from 42 seconds to 1 second
All variation names are loaded within 10 seconds (first page within 2 seconds).

dev_server_brr.mov

Claude claude@anthropic.com

Note

Medium Risk
Changes core dev-server sync and persistence for available variations (streaming vs REST, async DB writes, merge on resync); startup is faster but names can be briefly stale and behavior differs for holdouts/unresolvable flags.

Overview
Dev server project sync no longer blocks on a full serial REST flag list. Flag state and variation values now come from a single SDK streaming connection (with a custom datastore hook that captures raw variation values). The blocking GetAllFlags / recursive pagination path is removed in favor of GetFlagsPage and GetFlag for targeted REST use.

Variation display names are filled asynchronously via FillVariationNames, kicked off after create/patch/sync. It pages the flags API (100 per page, up to 6 concurrent pages), upserts names per flag into SQLite through UpsertAvailableVariationsForFlags, skips work when nothing is pending-, and marks stragglers unresolvable- when REST never returns them. Resync merges existing names by value so lazy fills are not wiped.

Initial sync stores value-only variations with pending-* placeholder IDs until the background job runs; holdout flags are omitted from stored variations. The UI polls availableVariations until pending- IDs disappear.

^{Reviewed by Cursor Bugbot for commit 98ebef0. Bugbot is set up for automated code reviews on this repo. Configure here.}

fetchAvailableVariations used to require a slow paginated REST call (GET /api/v2/flags/{projKey}) just to get variation values. Those values are already present in the same streaming connection dev-server opens for AllFlagsState - a custom DataStore decorator now captures them off that connection instead, so CreateProject/UpdateProject no longer make any REST call on the sync path at all. Known gap, not yet addressed: variation name/description only exist in the REST representation, never in the streaming payload (verified against a live account). This commit does not resolve names - flags will show raw values in the override picker instead of friendly names until a follow-up adds that back (still deciding between a background bulk resync vs. per-page lookups - see PR discussion). Measured against the real staging default project: time to healthcheck-ready dropped from ~10s (parallel REST pagination) to ~1-3s.

Values come off the streaming connection (fast, unchanged). Names, which only exist in the REST flag representation, are now filled in the background: paginate GET /flags 6 pages concurrently and upsert each page's names as it returns, so the override picker's raw values pick up friendly names shortly after sync without any REST on the sync path. Kicked off (detached from the request context) from the add-project and patch handlers and from startup sync via FillVariationNamesAsync, a var so tests can stub the background work. The UI re-polls availableVariations until no pending- placeholder ids remain. Claude <claude@anthropic.com>

- Skip the whole REST fan-out when a prior fill already resolved every variation (nothing pending), so a steady-state resync makes zero REST calls; only a new/changed flag reopens it. - Exclude holdout flags (`-ld-holdout`) from stored variations and from the picker list - they're experimentation plumbing, never resolve from REST (filtered out), and would otherwise sit permanently pending. Claude <claude@anthropic.com>

Claude <claude@anthropic.com>

cursor

Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.

There are 5 total unresolved issues (including 3 from previous reviews).

^{❌ 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 ef79a4a. Configure here.}

Flags the REST list never returns (non-standard purposes like ai_graph) stayed pending- forever, so hasPendingVariations was always true and the resync gate never skipped. After a complete fill pass, flip any still- pending variation to a terminal unresolvable- id; the gate and FE poll only watch for pending-, and mergeVariationNames preserves the marker across resyncs. Claude <claude@anthropic.com>

errgroup.WithContext returns a context canceled once Wait returns, which killed the post-pass reconcile read (context canceled) so stragglers never got marked. The page goroutines never return errors, so a plain errgroup with the original context is correct. Claude <claude@anthropic.com>

nieblara · 2026-07-09T19:26:50Z

for the visually inclined!
before:

after:

drewinglis-work

Automated deep review (8 finder angles, per-finding adversarial verification). 10 findings below as inline comments, ranked roughly by severity: 8 confirmed, 2 plausible-but-mechanics-verified.

Also noted but below the severity cap: Api.GetFlag is added/implemented/mocked with zero callers (looks like the abandoned targeted-fetch path); the fill refetches every page and rewrites every flag even when the pending gate already knows which flags need names; UpsertAvailableVariationsForFlags duplicates the INSERT/marshal loop from InsertAvailableVariations; the pending- prefix is an unshared cross-language wire contract (Go const vs bare TS literal); offset := offset is redundant under Go 1.23; and ~14 new lines exceed 120 chars.

drewinglis-work · 2026-07-09T22:02:01Z

+
+// FillVariationNamesAsync is a var so tests can stub out the background work.
+var FillVariationNamesAsync = func(ctx context.Context, projectKey string) {
+	go FillVariationNames(context.WithoutCancel(ctx), projectKey)


Confirmed — process crash on transient network error. This detached goroutine has no panic recovery, and Retry429s (adapters/internal/api_util.go:34) dereferences res.StatusCode without a nil check. On a transport-level failure (DNS, connection refused), the ldapi client's Execute returns (nil, nil, err), so res is nil and the deref panics — and an unrecovered panic in a bare go goroutine terminates the whole dev-server process. Pre-PR these REST calls ran on request/startup paths where errors surfaced as errors.

A second panic in the same unrecovered goroutine: *v.Id at line 126 (ldapi.Variation.Id is *string with omitempty, no non-nil guarantee).

Suggested fix: defer recover() (with a log) in the goroutine, plus an err != nil || res == nil check in Retry429s before touching res.StatusCode.

drewinglis-work · 2026-07-09T22:02:01Z

+
+	// Streaming never carries names, so keep any names already resolved for
+	// a variation instead of wiping them out on every resync.
+	existing, err := store.GetAvailableVariationsForProject(ctx, projectKey)


Confirmed — fill/resync race can revert or permanently poison variation names. This read-merge-write is not atomic with the concurrent FillVariationNames job: the read (GetAvailableVariationsForProject, plain non-tx query) and store.UpdateProject (DELETE-all + reinsert in its own tx) bracket a window in which the fill's per-flag upserts commit and are then silently wiped back to pending-*. There is no locking anywhere in model/db.

Worse interleaving: if a resync wipes rows back to pending-* after the fill fetched its pages but before markUnresolvableVariations runs, flags that do exist in REST get flipped to unresolvable-*; hasPendingVariations then gates out all future fills and mergeVariationNames carries the unresolvable- ids across every subsequent resync — stuck until some variation goes pending again.

Related: FillVariationNamesAsync has no per-project singleflight, so startup sync + a PATCH run duplicate concurrent fills, multiplying these interleavings.

drewinglis-work · 2026-07-09T22:02:01Z

+	merged := make([]FlagVariation, len(fresh))
+	for i, fv := range fresh {
+		merged[i] = fv
+		for _, existing := range existingByFlagKey[fv.FlagKey] {


Confirmed — merge can assign duplicate ids and silently drop a variation. The value-equality match doesn't consume matched existing rows and never de-dupes the resulting ids, while the schema declares UNIQUE (project_key, flag_key, id) ON CONFLICT REPLACE (db/sqlite.go:542).

Constructible case: stored rows still pending [pending-0="a", pending-1="b"]; upstream prepends a variation so the fresh stream yields [pending-0="new", pending-1="a", pending-2="b"]. The merge copies existing id pending-0 onto "a" while "new" keeps its index-based pending-0 → two rows with the same id → ON CONFLICT REPLACE erases one, and a variation disappears from availableVariations with no error.

Matching by variation index (which pending-N already encodes) instead of by value would avoid both this and the wrong-name-on-value-swap cases.

drewinglis-work · 2026-07-09T22:02:01Z

+	if err != nil {
+		return nil, 0, errors.Wrapf(err, "unable to get flags page (offset %d) from LD API", offset)
+	}
+	return flags.Items, int(flags.GetTotalCount()), nil


Plausible (mechanics verified) — totalCount is optional and defaults to 0, which terminally marks flags 101+ unresolvable. TotalCount is *int32 with omitempty (the repo's own ld-openapi.json requires only items/_links), and GetTotalCount() returns 0 when nil — there's no GetTotalCountOk check or len(Items) fallback. If it's ever 0 on a >100-flag project, the offset loop in FillVariationNames never runs, incomplete stays false, markUnresolvableVariations permanently rewrites flags 101+ to unresolvable-*, and the pending gate blocks every future fill.

Same terminal outcome for flags that shift across page boundaries mid-fill (concurrent fixed offsets against a total snapshotted once): skipped flags are marked unresolvable with no retry. The deleted GetPaginatedItems followed _links.next and had neither failure mode. A cheap guard (GetTotalCountOk, or continue-while-page-is-full) would close this.

drewinglis-work · 2026-07-09T22:02:01Z

+            `/dev/projects/${selectedProject}?expand=availableVariations`,
+          ),
+        );
+        if (!res.ok || cancelled) {


Confirmed — one failed poll ends polling forever. This !res.ok early return and the catch below never reschedule setTimeout; the only re-arm is on the success path, and nothing else restarts the loop (the effect's only dep is selectedProject). A single transient 500/network error while ids are still pending- leaves the UI showing placeholders indefinitely, even though the server resolves names seconds later. Rescheduling (counting the attempt) on failure paths would fix it.

drewinglis-work · 2026-07-09T22:02:01Z

+        const stillPending = Object.values(variations).some((forFlag) =>
+          forFlag.some((v) => v._id.startsWith('pending-')),
+        );
+        if (stillPending && attempts < 20) {


Confirmed — silent 40s give-up, below the workload this PR targets. After 20 attempts × 2s the loop stops with names still pending — no console output, no state flag, no UI signal. The background fill does the same paged REST fetch that previously took ~50s on large projects (plus 429 backoff), so it can outlast the window; users then see pending- placeholders until a manual reload. Consider polling until resolved with backoff, or surfacing a "names still loading" state when the cap is hit.

drewinglis-work · 2026-07-09T22:02:01Z

+        const json: { availableVariations?: Record<string, FlagVariation[]> } =
+          await res.json();
+        const variations = json.availableVariations ?? {};
+        setAvailableVariations(variations);


Confirmed — stale-response race on project switch. cancelled is checked before await res.json() but not after it, so a poll in flight when the user switches projects still calls setAvailableVariations with the old project's data. availableVariations is keyed only by flag key and rendered for whatever project is selected, so the old project's names/values show against the new project's flags. Re-check cancelled after the await (the pattern the flag-fetch effects in this file already use).

drewinglis-work · 2026-07-09T22:02:01Z

+)
+
+// FillVariationNamesAsync is a var so tests can stub out the background work.
+var FillVariationNamesAsync = func(ctx context.Context, projectKey string) {


Confirmed — import and backup-restore never schedule the fill. FillVariationNamesAsync is called from exactly three sites (post_add_project.go:33, patch_project.go:19, sync.go:58). But POST .../import copies variation ids verbatim (post_import_project.go:44-49 → import_project.go:60-77) and restore is a raw sqlite file swap (db/sqlite.go:423-441) — so a snapshot taken mid-fill (or after a failed fill) reinstates pending-N ids with nothing to resolve them. Startup sync only covers the --project-configured project, and the UI polls 20× against ids that will never change. Driving the fill from wherever pending variations are persisted (rather than per-call-site) would close the whole class.

drewinglis-work · 2026-07-09T22:02:01Z

+
+	first, total, err := api.GetFlagsPage(ctx, projectKey, fillPageSize, 0)
+	if err != nil {
+		log.Printf("variation name fill: initial page failed for %q: %v", projectKey, err)


Confirmed — hard error downgraded to one log line with no retry. On main, a flag-fetch failure failed CreateProject loudly (the deleted test Returns error if it can't fetch flags). Now a page-0 failure logs to stderr and returns: no retry, no backoff, nothing re-triggers until a manual PATCH/POST or restart. Scenario: token valid for the SDK stream but lacking REST flag-read — project creation reports success while variations show pending- placeholders indefinitely, and the UI gives up silently after ~40s. At minimum a bounded retry with backoff here would cover transient failures.

drewinglis-work · 2026-07-09T22:02:01Z

+    return (
+      flagEntries
+        // Holdout flags are experimentation plumbing, not overridable app flags.
+        .filter(([flagKey]) => !flagKey.endsWith('-ld-holdout'))


Confirmed — holdout rule narrowed to a key-suffix heuristic, now encoded three divergent ways. On main the only holdout exclusion was the semantic REST filter purpose:all+!(holdout). At PR head the rule exists as (1) that REST filter (adapters/api.go:60), (2) the Go const holdoutFlagKeySuffix skipping streamed variations (model/project.go:22,157), and (3) this hard-coded -ld-holdout literal.

Behavior change: a legitimate user flag keyed e.g. promo-ld-holdout was visible and overridable on main; now the UI hides it and the streaming pass skips its variations, so it can't be overridden from the dev-server UI. The suffix and semantic rules already disagree on that case, and the unshared literal drifts independently. Filtering once server-side (ideally by purpose, which the REST fill already sees) would keep one encoding.

davidbrackbill added 3 commits July 8, 2026 16:19

cursor Bot reviewed Jul 9, 2026

View reviewed changes

Comment thread internal/dev_server/model/fill_variation_names.go

Comment thread internal/dev_server/model/sync.go

Comment thread internal/dev_server/model/fill_variation_names.go

Comment thread internal/dev_server/model/fill_variation_names.go

davidbrackbill changed the title ~~dev-server: variation values from streaming + background name fill~~ perf: Improve dev server startup 50s->1s using streaming + REST upsert for variation names Jul 9, 2026

style: apply prettier formatting to Flags.tsx

ef79a4a

Claude <claude@anthropic.com>

cursor Bot reviewed Jul 9, 2026

View reviewed changes

Comment thread internal/dev_server/model/fill_variation_names.go

Comment thread internal/dev_server/db/sqlite.go

davidbrackbill added 2 commits July 9, 2026 08:31

davidbrackbill requested a review from dmashuda July 9, 2026 15:35

davidbrackbill marked this pull request as ready for review July 9, 2026 15:35

davidbrackbill requested review from drewinglis-work and nieblara July 9, 2026 15:45

davidbrackbill mentioned this pull request Jul 9, 2026

fix(dev-server): fetch flag pages concurrently instead of serially #732

Closed

drewinglis-work reviewed Jul 9, 2026

View reviewed changes

Uh oh!

Conversation

davidbrackbill commented Jul 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Fix

Outcome

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

nieblara commented Jul 9, 2026

Uh oh!

drewinglis-work left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

davidbrackbill commented Jul 9, 2026 •

edited

Loading