feat(reliability): retry, circuit breaker, graceful degradation, single-flight (LAB-518) - #43
Merged
Merged
Conversation
…-miss single-flight (LAB-518) Brings cachekit-rs to reliability-tier parity with the Python and TypeScript SDKs: - New `reliability` feature (default, native-only): retry with truncated exponential backoff + jitter driven by the previously-dead BackendErrorKind::is_retryable() classification, and a closed/open/half-open circuit breaker with a rolling failure window. Composed as breaker(retry(op)) around every backend data operation, mirroring cachekit-ts's ReliabilityExecutor. Only retryable-kind errors count toward opening the circuit — permanent/auth errors are request-specific and must not cut off healthy traffic. - Presets: on by default in production/encrypted/io, off in minimal (ts preset posture). Builder: .reliability() / .no_reliability(). - New BackendErrorKind::CircuitOpen for fail-fast errors. - Graceful degradation in #[cachekit]: backend failures fail OPEN on the plain path (function runs uncached, matching cachekit-py); the secure path stays fail-CLOSED — encrypted workloads never silently degrade. - Cold-miss single-flight (CacheKit::single_flight, always available): per-key in-process async mutex dedups concurrent fills; with `reliability`, lock-capable backends (CachekitIO, Redis via the existing LockableBackend) additionally suppress fills cross-process through a distributed fill lock with poll-for-fill on contention. Leaders never re-check the cache — a re-check would be a second billable miss under metered-misses pricing. - Backend::as_lockable() default hook (None) overridden by CachekitIO and RedisBackend so dyn Backend can expose its lock capability. 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: 39 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 (6)
WalkthroughThe PR adds a native-only reliability feature with retries, circuit breaking, fail-open handling, and distributed single-flight cache fills. It extends backend capability discovery, builder presets, macro miss handling, public exports, documentation, and integration coverage. ChangesReliability contracts and builder wiring
Retry and circuit-breaker engine
Local and distributed single-flight
Macro failure and fill handling
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant CacheKitBuilder
participant ReliableBackend
participant CircuitBreaker
participant RetryPolicy
participant Backend
CacheKitBuilder->>ReliableBackend: wrap backend with reliability configuration
ReliableBackend->>CircuitBreaker: guard operation
CircuitBreaker->>RetryPolicy: execute retry policy
RetryPolicy->>Backend: perform backend operation
Backend-->>RetryPolicy: return result or error
RetryPolicy-->>CircuitBreaker: report outcome
CircuitBreaker-->>ReliableBackend: return result or circuit-open error
sequenceDiagram
participant CacheMacro
participant CacheKit
participant FlightMap
participant LockableBackend
participant CacheBackend
CacheMacro->>CacheKit: acquire single-flight guard
CacheKit->>FlightMap: lock computed key
FlightMap->>LockableBackend: acquire distributed fill lock
LockableBackend-->>CacheKit: leader or contested result
CacheKit->>CacheBackend: re-check or write filled value
CacheKit->>LockableBackend: release distributed lock
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
… (LAB-518) The circuit breaker's half_open_calls counter caps in-flight recovery probes, but a Success that had not yet crossed success_threshold never released its slot — only Neutral did. Any config with success_threshold > half_open_max_calls (all fields are pub, unvalidated) therefore wedged the breaker half-open permanently: slots fill, successes stall below threshold, and every call fails fast with CircuitOpen even against a recovered backend. The default 3/3 preset escaped only because threshold == cap. Mirror the Neutral arm and release the slot on a non-closing success; add a regression test with threshold(3) > cap(1). Surfaced by the crypto/protocol expert-panel review. Co-authored-by: multica-agent <github@multica.ai>
…fail-open, API cuts (LAB-518) Expert-panel review findings on PR #43: - CRIT: half-open probe slots leaked when a guarded future was cancelled (caller timeout/select!) or panicked before recording an outcome — leak half_open_max_calls of them and the breaker wedges half-open forever, CircuitOpen on every call even against a recovered backend. try_acquire now returns an RAII ProbePermit: complete() hands accounting to the outcome arms; Drop releases the slot (like Neutral, no transition) on every other exit path. The manual counter pairs that leaked twice are gone. Unit tests cover dropped-permit release and closed-state permits not touching half-open accounting; an integration test cancels a hanging probe via tokio::time::timeout and proves the breaker still recovers. - MAJ: the plain-path fail-open arm swallowed permanent/auth backend errors — a wrong API key silently ran uncached forever while looking healthy. Fail-open is now scoped to outage-class errors (retryable + CircuitOpen); permanent/auth propagate. secure unchanged (fail-closed on everything). - MIN: SingleFlight::awaiting_fill → wait_for_fill — it sleeps and mutates; the old name read as a pure predicate. - Cuts: no_reliability() deleted (opt out by passing a config with both layers None — build() skips the no-op wrap); CircuitState demoted to cfg(test) until LAB-101 consumes it at runtime. Co-authored-by: multica-agent <github@multica.ai>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/cachekit/src/flight.rs`:
- Around line 45-58: Document the distributed fill suppression ceiling
represented by FILL_LOCK_TIMEOUT_MS and FILL_POLL_BUDGET, explicitly noting that
fills exceeding the lock TTL may be recomputed concurrently after auto-expiry.
Keep the existing fail-open and owner-checked release behavior unchanged; only
add operational guidance or, if supported by the surrounding design, outline a
lock-renewal/heartbeat strategy.
In `@crates/cachekit/tests/macro_tests.rs`:
- Around line 359-379: Add a macro test alongside
macro_fails_open_when_backend_down that configures a low reliability failure
threshold, drives the backend through enough failures to open its circuit, and
then verifies the generated fail_open_arm plain path still executes uncached
successfully. Assert the operation result and run count after CircuitOpen is
reached, using the existing fail-open helpers and backend fixtures.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: a574450d-1233-4af2-b5cc-5fd5a361d116
📒 Files selected for processing (14)
README.mdcrates/cachekit-macros/src/lib.rscrates/cachekit/Cargo.tomlcrates/cachekit/src/backend/cachekitio.rscrates/cachekit/src/backend/mod.rscrates/cachekit/src/backend/redis.rscrates/cachekit/src/client.rscrates/cachekit/src/error.rscrates/cachekit/src/flight.rscrates/cachekit/src/intents.rscrates/cachekit/src/lib.rscrates/cachekit/src/reliability.rscrates/cachekit/tests/macro_tests.rscrates/cachekit/tests/reliability_tests.rs
…he 5s fill-lock ceiling (LAB-518) CodeRabbit round 2: the macro's fail-open guard special-cases BackendErrorKind::CircuitOpen but no test tripped a real breaker on the plain path — added one (threshold 1: transient fall-open opens the circuit, second call falls open on CircuitOpen). Also documented that FILL_LOCK_TIMEOUT_MS bounds cross-process suppression: fills slower than 5 s lose the lock mid-compute and may recompute concurrently (fail-open by design; owner-checked release prevents wrongful deletion). Co-authored-by: multica-agent <github@multica.ai>
Contributor
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
…liability-tier Conflict resolution notes: - Cargo.toml: main made tokio an optional dep for memcached/file spawn_blocking; this branch needs tokio-sync unconditionally for cold-miss single-flight. Kept ONE non-optional tokio (sync only) and dropped dep:tokio from the memcached/file feature lists — they still stack tokio/rt + tokio/time on top, same build result. - lib.rs / README: kept both sides' workers mutual-exclusion guards (reliability + memcached + file). - New File/Memcached backends implement no LockableBackend, so the default as_lockable() = None is correct — no overrides needed. Co-authored-by: multica-agent <github@multica.ai>
This was referenced Jul 24, 2026
Merged
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
1 participant
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.
Closes LAB-518 — reliability-tier parity for cachekit-rs (rank #3 of the LAB-275 cross-SDK feature-gap audit).
What
A Rust user behind a flaky backend now gets the same posture as py/ts:
BackendErrorKind::is_retryable()classification. Permanent/auth errors propagate on first failure. Defaults: 3 attempts, 100 ms base, 5 s cap, jitter ×[0.5, 1.5).BackendErrorKind::CircuitOpenfor fail-fast errors. Only retryable-kind failures count toward opening — a burst of 400s can't cut off healthy traffic (deliberate improvement over ts's count-everything breaker; rs has the classification, ts doesn't).#[cachekit]fails open on backend failure — the wrapped function runs uncached (cachekit-py posture). Thesecurepath stays fail-closed: backend + decryption errors propagate; encrypted workloads never silently degrade. Config errors always propagate (cross-SDK key-divergence contract unchanged).CacheKit::single_flight()(always available, tokio-sync only): per-key in-process async mutex collapses concurrent fills; followers re-check the cache, leaders never do (a leader re-check = a second billable miss under metered-misses). Withreliability+ a lock-capable backend (CachekitIO, Redis — both already implementLockableBackend), a distributed fill lock suppresses fills cross-process; contested processes poll for the remote fill (100 ms cadence, 5 s budget) then compute anyway (fail-open). The macro wires all of this automatically.Feature/preset story
reliabilityfeature, in defaults, native-only (compile_error!withworkers, mirroring thel1guard). Composition:breaker(retry(op))as aBackenddecorator applied atbuild()— zero changes to client call sites, covers plain/secure/interop/macro paths uniformly.production/encrypted/io, off inminimal(ts preset posture). Builder:.reliability(config)/.no_reliability().Backend::as_lockable()default hook (returnsNone; overridden by CachekitIO + Redis) sodyn Backendcan expose its lock capability — trait objects can't cross-cast.Tests
21 new tests (11
reliability_tests.rs, 6 breaker/retry unit tests, 4 macro tests): retry recovery/exhaustion/permanent-passthrough, breaker open/fail-fast/half-open-recovery/probe-slot-release, one-breaker-failure-per-retry-sequence, macro fail-open + secure fail-closed, 5-way concurrent-miss dedup, distributed-lock acquire/release + contested-poll paths.Gates: full suite green (
cachekitio,redis,encryption,l1,macros+ defaults), clippy-D warnings, fmt, MSRV 1.85 check, redis-only and wasm32 feature combos compile (warnings unchanged vs base), cargo-deny licenses.Non-goals
Backpressure (matrix row stays ❌, not in LAB-518 acceptance criteria), SWR (stretch — should split to its own ticket), observability (LAB-101).
Crypto/protocol gate
No encryption, AAD, key-derivation, cache-key, or wire-format code is touched — the decorator passes bytes through untransformed and the macro's key construction is unchanged (
macro_key_delegates_to_interop_keystill green). Expert-panel review not triggered.Companion docs PRs: cachekit-io/protocol (matrix rows), cachekit-io/docs (rust quickstart/builder/macros pages).
Summary by CodeRabbit
New Features
Bug Fixes