feat(cache): cold-miss single-flight + opt-in cross-process locking (LAB-519) - #77
feat(cache): cold-miss single-flight + opt-in cross-process locking (LAB-519)#7727Bslash6 wants to merge 3 commits into
Conversation
…LAB-519) A cold L1+L2 miss previously executed the wrapped function once per concurrent caller — N cold callers meant N upstream calls, N L2 GETs (N billed misses under metered-misses), and N L2 writes. In-process (always on): concurrent wrap() calls for one cache key share a single in-flight promise covering the L2 read, compute, and write. The L2 read must be inside the flight — the GET-miss is the billed event, so deduping only the compute would still bill N misses. Cross-process (opt-in, stampede.distributedLock): wires the previously caller-less LockableBackend capability around the miss path, mirroring cachekit-py's acquire_lock flow — acquire, double-check L2, compute, write, release. Contested waiters retry the lock on an interval rather than polling get() (a poll GET against a still-cold key is itself a billed miss), then fall through to computing after lockWaitMs: the lease is best-effort mitigation, never a correctness gate. Lock calls stay outside the reliability executor so lock failures neither stack retry latency nor open the circuit breaker for data operations. Deliberately no general admission-control cap beyond L1's maxConcurrentRefreshes: on Node's single-threaded event loop concurrent misses don't compete for threads, single-flight collapses the per-key herd, and distinct-key floods are bounded by backend timeouts plus the circuit breaker. Decision recorded on StampedeConfig. Co-authored-by: multica-agent <github@multica.ai>
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 13 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (7)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
@coderabbitai review |
✅ Action performedReview finished.
|
Port LAB-519 single-flight + distributed-lock cold path onto the cache-core.ts extraction from LAB-595 (#78): CacheImpl moved to the shared core, so the single-flight map, stampede config validation, and resolveMiss/resolveUnderLock/computeAndStore land there (now shared with the Workers entrypoint). Backend construction stays in the runtimes — CacheRuntime.resolveBackend grows an optional stampede param so the Node runtime can select cachekitioWithLocking for apiKey configs; the Workers runtime is deliberately untouched (lock-capable backends remain an explicit construction there). StampedeConfig export moves to exports-common.ts (platform-neutral surface).
…lab-517-wire-metrics Stacks LAB-517 on LAB-519 per the LAB-761 merge plan (metrics observe the cache, so they land on top; when #77 merges this PR's diff collapses to metrics-only). Ports the metrics wiring onto the cache-core.ts extraction from LAB-595: - Instrumentation (run/execute/instrument, hit/miss/L1-stats recording) moves into the shared CacheImpl in cache-core.ts, composing with the LAB-519 single-flight cold path — followers share one recorded miss. - cache-core imports metrics/prometheus.js TYPES only (prom-client is Node-only and dynamic import still enters esbuild's resolve graph); the collector is injected via a new optional CacheRuntime.createMetrics hook, with an inline no-op fallback, so Workers degrade to no-ops the same way a missing prom-client peer does on Node. - resolveBackend gains an l1Telemetry getter param so the Node runtime auto-wires the SaaS X-CacheKit-L1-* metricsProvider from live counters, composed with the LAB-519 lockable-wrapper selection. - setLogger/CachekitLogger move to exports-common.ts: logger.js is in the shared graph (background-refresh, invalidation) and Workers apps need the same error sink control. Prometheus exports stay Node-only. - intents metrics?: boolean | MetricsConfig widening ports to intents-core.ts (type-only import, workers-safe).
|
Resolved the merge conflict with main (LAB-595 extracted Verified locally: type-check, lint, format, full vitest (571 passed incl. the single-flight suite). Merge-sequencing context: LAB-761. |
|
Auto-rebase merged |
|
Auto-rebase attempted: merging Blocked at push: the repo's Merge commit is ready locally but not pushed (nothing changed on the remote branch). Once the pre-commit config is fixed, a re-run of this automation (or a manual push) will pick it up cleanly. |
|
Attempted auto-rebase onto |
|
Auto-rebase attempted: merging Blocked at the test gate this time: This reproduces the same failure mode documented in earlier auto-rebase comments on this PR, now traced to a concrete cause (missing Rust toolchain in the automation sandbox) rather than something about this merge. Leaving unpushed per the "gates must pass" rule rather than judging it a safe exception — needs a human to either add a Rust toolchain to the sandbox or verify off-sandbox before this can go through automatically. |
# Conflicts: # packages/cachekit/README.md
|
Resolved conflict with |
Kody Review CompleteGreat news! 🎉 Keep up the excellent work! 🚀 Kody Guide: Usage and ConfigurationInteracting with Kody
Current Kody ConfigurationReview OptionsThe following review options are enabled or disabled:
|
![kodus-27b[bot]](https://avatars.githubusercontent.com/u/223823690?s=60&v=4){kind=small}
1 participant
Closes LAB-519.
Problem
The SDK prevented refresh stampedes (SWR version tokens +
refreshingKeys+maxConcurrentRefreshes) but not cold-miss stampedes:wrap()'s cold path was a bareawait fn(...args), so N concurrent cold callers all executed the function, all issued the L2 GET (N billed misses under metered-misses), and all wrote L2. TheLockableBackendcapability (acquireLock/releaseLock) existed with zero callers in the live cache path.Changes
In-process single-flight (always on) — concurrent
wrap()calls for one cache key share a single in-flight promise. The flight covers the L2 read, the compute, and the write: the GET-miss is the billed event, so deduping only the compute would still bill N misses. A failed flight rejects every waiter and is evicted, so the next call retries fresh.Cross-process distributed lock (opt-in,
stampede.distributedLock) — wiresLockableBackendaround the miss path, mirroring cachekit-py'sacquire_lockflow (wrapper.py): acquire → double-check L2 → compute → write → release. SinceacquireLocknever blocks on contention (LAB-240), contested waiters retry the lock on an interval bounded bylockWaitMs, then fall through to computing — deliberately not pollingget(), because on a metered-misses backend every poll GET against a still-cold key is itself a billed miss. Lock failures degrade to an unlocked compute (best-effort mitigation, never a correctness gate), and lock calls stay outside the reliability executor so they neither stack retry latency nor open the circuit breaker for data ops. Config-based SaaS backends automatically get the lockable wrapper when the opt-in is set; an opt-in on a lock-incapable backend throwsConfigurationErrorrather than silently doing nothing.Admission control beyond
maxConcurrentRefreshes: evaluated and declined (recorded onStampedeConfig+ README): on Node's single-threaded event loop concurrent misses don't compete for threads, single-flight collapses the per-key herd (the amplification vector metered-misses punishes), and distinct-key miss floods are bounded by backend timeouts + circuit breaker. A global semaphore would add queueing latency and a tuning knob without a failure mode it prevents. Revisit only with evidence of backend connection exhaustion.Tests
10 new tests in
cache.single-flight.test.ts: herd shares one read/compute/write; distinct keys don't share; flight eviction after settle; shared rejection + fresh retry; two cache instances over one backend simulating two processes (contested waiter picks up the winner's write via post-grant double-check —calls === 1); permanent contention falls through to compute;acquireLockthrowing degrades to compute; lock released when compute throws; config validation. Full suite: 566 passed, 1 skipped (pre-existing), lint + format + type-check green.Docs
Package README: stampede-protection section + feature bullet. Protocol
sdk-feature-matrix.mdand docs.cachekit.io follow in companion PRs (matrix cell for TS stampede moves from refresh-path-only to full).