Skip to content

Serve replicator-mode flag checks only once the cache is ready - #204

Open
ryanechternacht wants to merge 1 commit into
mainfrom
ryan/replicator-cache-ready-gate
Open

ryanechternacht wants to merge 1 commit into
mainfrom
ryan/replicator-cache-ready-gate

Conversation

@ryanechternacht

Copy link
Copy Markdown
Member

Depends on SchematicHQ/schematic-replicator#143. Do not merge before it. That PR changes /ready so ready: true means the cache is complete for its cache_version, which is what this gate relies on.

Problem

In replicator mode, single and bulk flag checks both went through DatastreamClientAdapter.CheckFlag, and it read the replicator cache before looking at readiness. What happened next depended on whether the replicator was ready and what the cache held:

  • Not ready, and the flag plus every requested company/user cached: evaluated from a cache that might still be loading.
  • Not ready, and the flag cached but the company or user missing: threw, so Schematic fell back to the API.
  • Ready, and the flag missing: returned FlagNotFound with false, with no API fallback. Go falls back to the API here.
  • Ready, and the company or user missing: evaluated with what the cache held.

So a check made while the replicator was still loading could give a wrong verdict from a partial cache. Whether it did depended on which entities happened to be written yet.

Change

  • DatastreamClientAdapter.IsCacheReady() and Schematic.IsCacheReady() are new. In replicator mode they return the replicator readiness from the health poll. Outside replicator mode the adapter returns true, matching Go, because websocket mode fills its own cache and fetches misses on demand. Schematic.IsCacheReady() returns false when datastream is not in use.

  • Schematic.UseDatastreamCache() is one private helper, _datastreamClient != null && _datastreamClient.IsCacheReady(). It replaces the _datastreamClient != null check in both CheckFlagWithEntitlement (which CheckFlag calls) and CheckFlags with keys. If the cache is not ready, both skip the cache and take the existing API path, which falls back to flag defaults if the API fails.

  • DatastreamClientAdapter.CheckFlag also throws before reading the cache when IsCacheReady() is false, so a direct caller of the public adapter can't read an unready cache either. When ready, it evaluates from the cache like Go: a flag missing from the cache throws so the caller falls back to the API (previously it returned FlagNotFound), and a missing company or user evaluates with what the cache holds.

  • IsReplicatorReady() / IsReplicatorHealthy() keep their behavior. Their doc comments now say they report replicator readiness in replicator mode and point to IsCacheReady. IsConnectedAsync reports websocket state and was never the replicator readiness signal in this SDK (it stays false in replicator mode). Its doc comment now says so and points to IsCacheReady.

  • Health poll (ReplicatorHealthService): it already parsed the body on any status, including a 503. Two fixes:

    • It now records cache_version only when the response has a non-empty one. Before, a response without one set the version to null.
    • A request timeout (which HttpClient raises as TaskCanceledException) is now handled as a failed poll: not ready, last version kept. Before, it was rethrown as if it were shutdown and logged at Error level by the loop.

    Readiness still requires HTTP 200 and ready: true, the existing behavior. Go reads only the ready field. The two differ only for a 503 with ready: true, which the replicator does not send.

  • README replicator section: the SDK serves from the cache only once the replicator reports ready. Before that, flag checks use the API.

Websocket mode is unchanged. Track's local metric update is not touched.

Parity with Go

This matches SchematicHQ/schematic-go#240: an IsCacheReady that returns true outside replicator mode, and one shared gate (useDataStreamCache() in Go, UseDatastreamCache() here) at the single and bulk flag-check branches. One difference remains. C# bulk CheckFlags still evaluates each key through the same adapter method rather than in one rules-engine call. Both fall back to the API for the whole set if any key fails, so the results match.

There is no credit-lease prewarm or other non-flag-check cache read on main in this SDK. The only other datastream cache write is Track's metric update, which is out of scope.

Overlap with open PRs

Tests

New src/SchematicHQ.Client.Test/Datastream/ReplicatorCacheReadyTests.cs. It covers Datastream/, which is already in .fernignore. The tests seed the cache under the versioned flag key and the company key lookups the replicator writes. They use a controllable health service and a fake API that counts single and bulk requests:

  • Not ready: CheckFlag, CheckFlagWithEntitlement and CheckFlags all use the API and return its values, and the replicator cache is read zero times.
  • Not ready with the API failing: single and bulk return flag defaults.
  • Ready: single and bulk evaluate from the cache with no API calls and agree on value, flag, company and reason. The same holds with the company not cached.
  • Ready with the flag not cached: single and bulk fall back to the API.
  • Switching readiness moves bulk checks from the API to the cache.
  • IsCacheReady follows replicator readiness, and is true outside replicator mode.
  • Health poll:
    • A 503 with {"ready": false, "cache_version": "vX"} sets not ready and records vX.
    • A response without cache_version keeps the previous one.
    • An unreachable URL, a timeout or an unparseable body sets not ready and keeps the previous version.

Against the old code, 6 of these tests fail: the not-ready single and bulk test, the adapter not-ready test, the readiness switch, the ready/flag-not-cached fallback, the missing cache_version test and the timeout test.

CI steps run locally with .NET SDK 9.0.109: library and test builds in Release, and dotnet test, 452 passed. One run hit a failure in SimultaneousFlushes_DoNotInterfere, an event buffer timing test this PR does not touch. It passed on rerun and in the full-suite rerun.

🤖 Generated with Claude Code

In replicator mode, flag checks now read the replicator cache only after
the replicator reports ready (HTTP 200 with ready: true). Before that,
CheckFlag, CheckFlagWithEntitlement and CheckFlags skip the cache and use
the API. Once ready, both evaluate from the cache like the Go SDK, and a
flag missing from the cache falls back to the API.

Single and bulk flag checks share one gate, Schematic.UseDatastreamCache(),
which requires datastream and IsCacheReady(). DatastreamClientAdapter.CheckFlag
also refuses to read the cache while it is not ready.

Add IsCacheReady() to DatastreamClientAdapter and Schematic. It reports
replicator readiness in replicator mode and is true in websocket mode,
matching Go. The health poll now records cache_version only when a
response carries a non-empty one, and treats a request timeout as a
failed poll.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
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