Skip to content

feat: Hand over the data store to the new client on re-anchor#736

Merged
keelerm84 merged 2 commits into
feat/concurrent-keysfrom
mk/sdk-2542/store-handover
Jul 7, 2026
Merged

feat: Hand over the data store to the new client on re-anchor#736
keelerm84 merged 2 commits into
feat/concurrent-keysfrom
mk/sdk-2542/store-handover

Conversation

@keelerm84

@keelerm84 keelerm84 commented Jul 6, 2026

Copy link
Copy Markdown
Member

Summary

First slice carved out of #720 (the re-anchor mechanism, SDK-2542), split for reviewability. This is the store-handover piece and is independent of the credential and relayenv changes -- it adds a latent capability that the synchronous re-anchor will use in a later PR.

SSERelayDataStoreAdapter.Build now reuses its existing wrapper when one is already present, so a concurrent-keys re-anchor can hand the populated, initialized store to the new anchor's client instead of building a fresh, empty one -- no empty-store window, no re-sync. The wrapper is refcounted: acquire() bumps the holder count on each Build reuse and Close() decrements, tearing down the underlying store only on the final release. This keeps a retiring client's Close() from pulling the store out from under the new anchor. A fully-closed wrapper refuses re-acquisition, so Build rebuilds rather than resurrecting a dead store.

Tests

  • Inverts the H1/H5 re-anchor PoC tests from asserting the pre-fix (store wiped) behavior to asserting handover. H6/H7 are intentionally left as-is; they invert with the synchronous re-anchor change in a later slice.
  • Adds TestRealClient_HandoverPreservesUnderlyingStore -- a real-client spike verifying ld.LDClient.Close() does not tear down the shared underlying store.
  • Adds store_rebuild_after_close_test.go -- a refcount-contract regression covering the "fully-closed wrapper refuses re-acquisition" path.

Split sequence

This is PR 1 of a 4-slice split of #720: store handover (this PR) -> relayenv refactors -> credential rotator deferred-flip API -> synchronous re-anchor core.


Note

Medium Risk
Changes core data-store lifecycle on SDK client swap (refcounted Close and Build reuse); impact is limited to the store adapter layer in this slice, with broad regression and race tests, but mistakes could cause use-after-close or double-close on persistent stores.

Overview
Adds store handover so a concurrent-keys re-anchor can stand up a new upstream LDClient without wiping the in-memory feature store.

SSERelayDataStoreAdapter.Build now returns the existing streamUpdatesStoreWrapper when one is already parked, using acquire() to bump a holder refcount instead of building a fresh empty wrapper. Close() on the wrapper only tears down the underlying store on the final release, so a retiring client’s shutdown does not close the store while the new anchor still holds it. Fully closed wrappers refuse re-acquisition so a later Build creates a new wrapper rather than resurrecting a dead store.

Re-anchor PoC tests H1/H5 are flipped to expect handover (same store instance, data preserved through env_context). New coverage includes real-LDClient handover, rebuild-after-full-close, and refcount idempotency / handover-vs-Close race tests (intended for -race).

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

SSERelayDataStoreAdapter.Build reuses its existing wrapper when one is
already present, so a concurrent-keys re-anchor hands the populated,
initialized store to the new anchor's client instead of building a fresh,
empty one -- no empty-store window, no re-sync.

The wrapper is refcounted: acquire() bumps the holder count on each Build
reuse and Close() decrements, tearing down the underlying store only on the
final release. This keeps a retiring client's Close() from pulling the store
out from under the new anchor. A fully-closed wrapper refuses re-acquisition
so Build rebuilds rather than resurrecting a dead store.

Inverts the H1/H5 re-anchor PoC tests from asserting the pre-fix (store
wiped) behavior to asserting handover, and adds a real-client spike test and
a refcount-contract regression test. H6/H7 remain as-is pending the
synchronous re-anchor change.
@keelerm84 keelerm84 marked this pull request as ready for review July 6, 2026 17:21
@keelerm84 keelerm84 requested a review from a team as a code owner July 6, 2026 17:21
keelerm84 added a commit that referenced this pull request Jul 6, 2026
A stray or duplicate Close on a fully-released streamUpdatesStoreWrapper drove
refCount negative and re-closed the underlying store (double-releasing a
persistent store's connection pool). Close now short-circuits once closed.

Adds a refMu-guarded currentRefCount() accessor and concurrency tests:
idempotency, an 8-way concurrent double-close, and a Build-reuse/Close race
(all -race clean). Found by multi-agent review of #736.
A stray or duplicate Close on a fully-released streamUpdatesStoreWrapper drove
refCount negative and re-closed the underlying store (double-releasing a
persistent store's connection pool). Close now short-circuits once closed.

Adds a refMu-guarded currentRefCount() accessor and concurrency tests:
idempotency, an 8-way concurrent double-close, and a Build-reuse/Close race
(all -race clean). Found by multi-agent review of #736.
@keelerm84 keelerm84 force-pushed the mk/sdk-2542/store-handover branch from 5938a18 to e598fa9 Compare July 6, 2026 20:05
@keelerm84 keelerm84 merged commit 211cec1 into feat/concurrent-keys Jul 7, 2026
26 of 27 checks passed
@keelerm84 keelerm84 deleted the mk/sdk-2542/store-handover branch July 7, 2026 12:09
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