feat(l1): LAB-728 stale-while-revalidate — serve stale + single-flight background refresh - #47
Conversation
…ound refresh (LAB-728) An L1 hit past swr_threshold_ratio × entry TTL (default 0.5, ±10% jitter, mirroring py/ts) but before hard expiry is returned immediately while one background task re-executes the #[cachekit] function and rewrites both layers. Refresh dedup rides the existing cold-miss single_flight() — no SWR-specific dedup path — so N concurrent stale readers cost exactly one origin execution, in-process and (on lock-capable backends) per fleet. Hard-expired entries take the normal blocking miss path. - builder: swr_enabled (default on, sibling parity) + swr_threshold_ratio (validated (0.0, 1.0]); native-only surface — no silent no-op elsewhere - CacheKit is now Clone (all-Arc; clones share L1 + single-flight state) so the 'static refresh task can own the client - interop_get_swr on CacheKit + SecureCache (secure judges staleness on the L1 ciphertext entry — zero-knowledge preserved) - macro: sync fns rejected with a clear decoration-time error; refresh task re-materialises reference args via ToOwned (&str stays &str via as_str) - 30 s backfill cap reconciled: freshness window derives from each entry's own TTL, so backfilled entries background-refresh at ~ratio × 30 s and the refresh restores the full write-path TTL (documented, never silently clamped) - refresh needs a tokio runtime (Handle::try_current); other executors skip the refresh and keep serving until hard expiry — documented
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 2 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 (5)
WalkthroughAdds native L1 stale-while-revalidate support with configurable freshness thresholds, background single-flight refreshes, secure and non-secure client reads, async macro enforcement, runtime integration, documentation, and integration tests. ChangesStale-while-revalidate
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant CacheKitMacro
participant CacheKit
participant L1Cache
participant Origin
Caller->>CacheKitMacro: invoke async cached function
CacheKitMacro->>CacheKit: interop_get_swr
CacheKit->>L1Cache: classify entry
L1Cache-->>CacheKit: stale value
CacheKit-->>Caller: return stale value immediately
CacheKitMacro->>Origin: single-flight background refresh
Origin-->>CacheKitMacro: refreshed value
CacheKitMacro->>CacheKit: rewrite cache layers
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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-macros/src/lib.rs`:
- Around line 358-390: Reject mutable reference parameters in the SWR capture
logic before processing `non_client_params`, using the `Type::Reference` branch
and its `Reference::mutability` indicator to emit a clear macro error. Leave
immutable reference and owned-argument capture behavior unchanged.
In `@crates/cachekit/src/client.rs`:
- Around line 91-134: Add Debug, Clone, and PartialEq derives to SwrRead<T> in
crates/cachekit/src/client.rs lines 91-134, with the standard generic bounds,
and add the same derives to L1SwrRead in crates/cachekit/src/l1/mod.rs lines
25-35. No other changes are needed.
In `@crates/cachekit/src/lib.rs`:
- Line 69: Add SwrRead to the public prelude module exports, reusing the
existing SwrRead symbol already re-exported from client alongside CacheKit and
related types. Ensure users importing cachekit::prelude::* can reference the
return type of CacheKit::interop_get_swr and SecureCache::interop_get_swr
without an additional import.
- Around line 110-119: Update __swr_spawn to retain and observe the spawned
task’s JoinHandle result instead of dropping it silently. Add a
production-visible tracing warning or existing metrics hook when the refresh
task returns an error or panics, while preserving the fail-soft behavior by not
propagating the failure to callers.
In `@crates/cachekit/tests/swr_tests.rs`:
- Around line 179-198: Increase the timing margin in
disabled_swr_serves_until_hard_expiry_without_refreshing so the second probe
remains safely before the 2-second hard expiry, matching the wider real-clock
slack used elsewhere in the tests; adjust the TTL or trailing sleep without
changing the assertion that no background refresh occurs.
- Around line 6-12: Update the file-level cfg gate in the SWR tests to also
require a non-wasm32 target, matching the native-only constraint documented in
the header and the target-specific availability of client().swr_threshold_ratio.
Preserve the existing feature and unsync conditions.
🪄 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: 3e6fd1af-8fc9-4d86-9c9b-f37d9518a9b7
📒 Files selected for processing (8)
README.mdcrates/cachekit-macros/src/lib.rscrates/cachekit/Cargo.tomlcrates/cachekit/src/client.rscrates/cachekit/src/l1/mod.rscrates/cachekit/src/lib.rscrates/cachekit/src/reliability.rscrates/cachekit/tests/swr_tests.rs
…, test cfg + timing CodeRabbit-Resolved: cachekit-macros/src/lib.rs:390:Reject &mut parameters CodeRabbit-Resolved: client.rs:134:SWR enums lack common derives CodeRabbit-Resolved: lib.rs:69:SwrRead prelude export CodeRabbit-Resolved: swr_tests.rs:12:cfg gate missing not(wasm32) CodeRabbit-Resolved: swr_tests.rs:198:timing margin hard expiry
|
@coderabbitai review |
✅ Action performedReview finished.
|
1 participant
Closes LAB-728 — the stretch item deferred from LAB-518.
What
L1 stale-while-revalidate for
#[cachekit]: a hit past the freshness threshold (but before hard expiry) is served immediately while exactly one background task re-executes the function and rewrites both cache layers. No caller ever blocks on a merely-stale entry; concurrent stale readers never stampede the origin (misses are billable).Semantics (sibling parity)
swr_threshold_ratio× the L1 entry's own TTL, ±10% jitter — cachekit-py's elapsed-lifetime semantics (L1Cache.get_with_swr), same default 0.5 as py and ts. Note: py measures elapsed > ratio×TTL, ts measures remaining < ratio×TTL; identical at 0.5, and rs deliberately mirrors py (the elder SDK) — documented onswr_threshold_ratio.L1CacheConfig.swr_enabled=True, tsswrEnabled: true). Off switch:.swr_enabled(false)restores the exact pre-SWR expire-or-serve contract (test-verified).single_flight()— no SWR-specific dedup path. The spawned refresh task acquires the same per-key flight the cold-miss path uses: first task leads and re-executes; followers see the still-present entry and stand down. On lock-capable backends this also suppresses refreshes across processes for free.Acceptance criteria → evidence (all live call paths, no LAB-388s)
CacheKitBuilder::swr_enabled/swr_threshold_ratio(validated(0.0, 1.0]), default on/0.5;swr_tests::threshold_ratio_is_validated_at_buildswr_tests::stale_reads_serve_immediately_and_refresh_exactly_once— 8 concurrent stale reads complete in <300 ms against a 400 ms originsingle_flight()swr_tests::hard_expired_entry_takes_the_blocking_miss_path— asserts the call blocks ≥ origin latency and recomputesasync fnat decoration time with an actionable message; unit-tested incachekit-macros(sync_fn_is_a_clear_decoration_time_error)L1_BACKFILL_TTL_SECS,get_with_swr, READMEinterop_vector_tests,encryption_interop_tests,aad_testsall pass unchangedNotable mechanics
CacheKitis nowClone(all-Arc, cheap): the'staticrefresh task must own the client. Clones share L1, single-flight map, backend, encryption — required so clones dedup against each other (test:clones_share_l1_state).'staticcapture: the macro re-materialises&Targs viaToOwnedand rebinds them under the original name/shape inside the task (&strstays&strvia.as_str()); owned args are cloned (already a macro requirement). Test:str_argument_refreshes_in_the_background.workers+l1were already mutually exclusive; underunsync/wasm the SWR builder surface doesn't exist (compile error, not silent no-op) andSwrRead::Staleis unreachable.random_unit()(uuid-v4-backed jitter) moved fromreliability.rsto the crate root, shared with L1.l1feature now pullstokio/rt(forHandle::try_current+spawn); compile-time only when unused.Gates run
cargo fmt --check✓ ·clippy --all-targets(CI feature set)-D warnings✓ · full test suite (CI feature set) ✓ incl. 7 new SWR integration tests + 2 macro unit tests · doc-tests ✓ · MSRV 1.85check --all-targets✓ · wasm32check(workers,encryption) ✓ (pre-existing warnings only).Crypto/protocol review gate: not triggered — no encryption, AAD, key-derivation, cache-key, or wire-format change (SWR is an L1 serving policy; the secure path reuses the existing ciphertext read + decrypt unchanged).
Summary by CodeRabbit
New Features
Bug Fixes
Documentation