Skip to content

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

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 (HTTP 200) means the cache is complete for its cache_version, and a replicator that is still loading answers 503 with ready: false and a cache_version in the body. This gate relies on that meaning.

Problem

Ruby already routed replicator-mode flag checks through a readiness check, but only indirectly, and the health poll didn't match the new /ready contract:

  • check_flag_with_entitlement and check_flags (with keys) each gated the cache path on @datastream_client&.connected?, which in replicator mode happens to return replicator readiness. Single and bulk behaved the same (not ready: API path; ready: cache, with API fallback when evaluation errors), but the gate lived in two places and depended on connected? meaning something other than its name.
  • The health poll only read the body on a 2xx. Any other status set not ready without parsing the body, so a 503 from a replicator still loading its cache never recorded its cache_version.
  • On a 2xx, the poll recorded cache_version even when the body had none, which replaced the known version with an empty string.

Change

  • DataStream::Client#cache_ready? is new. In replicator mode it returns the ready field from the last health poll. Outside replicator mode it returns true, as in Go.
  • SchematicClient#use_datastream_cache? is the one gate that both check_flag_with_entitlement (and so check_flag) and check_flags call. It is connected? && cache_ready?. In replicator mode connected? and cache_ready? report the same readiness. In websocket mode cache_ready? is true, so the gate is connected?, exactly as before.
  • If the cache is not ready, both skip the cache and take the existing API path, which falls back to the flag default if the API fails. If it is ready, both evaluate from the cache as before, including the API fallback when evaluation errors (e.g. flag not in cache).
  • The health poll parses the JSON body whatever the HTTP status, sets readiness from ready, and records cache_version only when it is non-empty. A failed poll (connection error, timeout, or a body that isn't a JSON object) sets not ready and keeps the last known cache_version. Fail-closed on an unreachable replicator is unchanged; it is still an open question in #143.
  • connected? behaves the same. Its doc comment now says that in replicator mode it reports replicator readiness, and points to cache_ready?.
  • The README replicator section describes cache readiness.

track's local metric update is untouched. It still gates on connected?. A separate PR (ryan/replicator-track-offline) removes that gate, matching SchematicHQ/schematic-go#239. It edits only the track method, which this PR does not touch, so the two should merge cleanly.

Ruby has no credit-lease prewarm or other non-flag-check cache read besides track, so nothing else needed a decision here.

Parity with Go

This matches SchematicHQ/schematic-go#240: cache_ready? mirrors IsCacheReady() (true outside replicator mode), and use_datastream_cache? mirrors useDataStreamCache() at both the single and bulk branches. One difference: Go's websocket mode doesn't gate flag checks on the connection, while Ruby's always has. The Ruby gate keeps connected? so websocket mode is unchanged.

Tests

New Replicator mode - cache readiness gate block in test/custom.test.rb. It runs a real replicator-mode SchematicClient over a Redis stand-in seeded the way the replicator writes (flags under flags:<version>:<key>, the company under company:<version>:<id> plus a key lookup), with WebMock serving /ready and the API. The cache and the API disagree on both flags, so each result shows which one answered.

  • Not ready (503 {"ready": false, "cache_version": "v1"}): check_flag and check_flags both call the API and return its values. With the API failing, both return flag defaults.
  • Ready: both evaluate from the cache with no API call and agree with each other. A flag missing from the cache still falls back to the API for both.
  • Health poll: a 503 with {"ready": false, "cache_version": "vX"} sets not ready and records vX, and a later 200 with ready sets ready. An unreachable URL and an unparseable body each set not ready and keep the previous cache_version. A 200 with no cache_version keeps the previous one. cache_ready? is true outside replicator mode.

Existing mocks of the DataStream client now also define cache_ready?.

Locally, in a ruby:3.3 container to match CI: bundle exec rubocop (1140 files, no offenses) and bundle exec ruby -I lib -I test test/custom.test.rb (200 runs, 423 assertions, 0 failures, 0 errors) pass.

🤖 Generated with Claude Code

Add DataStream::Client#cache_ready?, which in replicator mode reports the
replicator's `ready` field and is always true outside replicator mode.
check_flag_with_entitlement and check_flags now share one gate,
use_datastream_cache?, so single and bulk checks can't drift.

The replicator health poll now parses the JSON body whatever the HTTP
status, so a 503 from /ready still records its cache_version. It only
records a non-empty cache_version, and a failed poll (connection error,
timeout, unparseable body) sets not ready and keeps the last version.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@ryanechternacht

Copy link
Copy Markdown
Member Author

Rebased onto main after #65 (credit leases). The only conflict was two sets of new methods added at the same spot in lib/schematic/datastream/client.rb; both are kept. Note that #65's client-lease check path in check() reads the DataStream cache without the cache-ready gate, the same as Go's checkWithClientLease and Node's lease path. Whether lease checks should also gate on cache readiness is an open question across SDKs (Python #135 gates it), so this PR leaves it as is. Tests: 812 runs and 201 custom runs pass, rubocop clean.

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