feat: add stale-on-error Redis recovery - #121
Draft
lan17 wants to merge 2 commits into
Draft
Conversation
36 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Add opt-in stale-on-error recovery from physically retained Redis values.
F = ttlSec[CacheLayer.REMOTE]remains the logical freshness age.M = staleOnErrorMaxAgeSecis the absolute maximum recovery age.age < F.age < M.Closes #117
Contract and flow
All source rejections qualify in v1, including synchronous throws, arbitrary rejection values, and
FallbackTimeoutError. A recovery read gets its own effectiveremoteReadTimeoutMsbudget. Existing single-flight behavior means one leader performs the source attempt and at most one recovery read; process/request followers share the same result. A recovered value may be memoized only inside an already-enabled request-local scope.Configuration
0: explicitly disables an inherited policy.0 < F < M <= 31,536,000seconds.Mdisables only stale recovery for that invocation, preserves valid fresh Redis caching, and records existing configuration-error telemetry.DialCacheKeyConfig.disabled()explicitly sets the field to0.The invocation's once-resolved
F/Msnapshot governs the whole flight. Lowering a boundary takes effect immediately. RaisingMcannot resurrect or extend a key written with a shorter physical TTL; only a later successful write receives the longer retention.Redis protocol and adapters
RedisReadRequest.maxAgeMsis now required. Both read Lua scripts:TIME;age >= maxAgeMs;created_at > watermark.Writes keep the existing key/frame and use physical
PX Fwhen recovery is off orPX Mwhen it is on. Tracked watermark retention continues to derive from the physical value retention, so an opted-in write keeps the watermark for approximatelyM + 60s.Bundled node-redis and Valkey GLIDE adapters pass the age bound atomically. Custom semantic clients must now declare
enforcesMaxAge: true; DialCache checks the marker at construction so an old compiled JavaScript adapter cannot silently ignore logical age. Packed TypeScript, ESM, and CommonJS negative fixtures cover this migration guard.Failure, invalidation, and shadow safety
F; a successful clean-miss shadow fill uses physicalMso future serving reads have the configured reservoir.Observability
Add one bounded optional observer:
Outcomes are
served,miss,read_error,read_timeout, anddeserialization_error. Prometheus exposesdialcache_stale_recovery_counter; Datadog exposesdialcache.stale_recovery.count. Existing fallback errors and duration remain truthful even when stale data ultimately reaches the caller. Observer throws/rejections remain isolated.Rollout and rollback
The current v1 key/frame is intentionally reused, so rollout must be readers-first:
0;Monly for selected use cases;An old reader would treat retained
F..Mdata as fresh. Setting the policy back to0stops new extended-retention writes but does not delete prior ones, so an old binary cannot be restored safely until the largest previously enabledMhas elapsed since the finalPX Mwrite, or affected keys are isolated/removed. Fleets that require mixed-version or immediate rollback safety must use a new key/frame version instead.Performance and resource evidence
The checked-in no-threshold harness reports final-script CPU, outage traffic, coalescing, memory, expiration, watermark residency, and opt-in eviction pressure. The following loopback Docker runs used node-redis, 2,000 sequential iterations,
F=60s,M=300s, and 100-way coalescing:The logical-stale miss still performs a full
GET, but does not return the payload, so it was cheaper than a fresh hit in these sequential runs. The design issue contains the direct old-script versus timestamp-aware comparison: roughly+0.6–0.7 usfixed Redis CPU per present read in that separate synthetic setup. Treat all figures as directional rather than production capacity promises.Every 100-way coalescing run produced one source rejection, two Redis reads, and one stale-recovery metric. Tracked probes observed value
PTTL ~= Mand watermarkPTTL ~= M + 60s; a 1-second tracked value expired while its watermark remained. On isolated Redis 6.2 with4 MiB maxmemory,allkeys-lru, and 500 benchmark-owned 16 KiB pressure keys, the pressure-local counter recorded 348 evictions and the retained stale read became a safe miss. The harness changes no Redis configuration, cleans only its random namespace, gives all owned keys finite TTLs, and has a hard watchdog plus bounded cleanup.Validation
Added regression coverage for retained-key opt-outs, dark-ramp safety, successful source refreshes, exact-
Mexpiry during source failure, per-flight policy snapshots, runtimeFchanges, and complete served-recovery telemetry.corepack pnpm checkcorepack pnpm test:integrationpnpm benchmark:stale-on-errorDIALCACHE_BENCH_ITERATIONS=1000 DIALCACHE_BENCH_FANOUT=100 pnpm benchmark:request-localgit diff --checkIndependent final core and benchmark reviews: clean