docs: expand CACHING.md (non-Redis L2, sizing/cost, SRE coordination)#7736
Conversation
PR #6682 added support for any IDistributedCache (Cosmos, SQL, EF) as the L2 store under ExtendedCache, but the docs continued to read as Redis-only. Update CACHING.md to document the new modes, show wiring for pairing with Cosmos, and explain the backplane / L1-staleness tradeoff.
withinfocus
changed the title
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7736 +/- ##
==========================================
- Coverage 65.01% 60.58% -4.43%
==========================================
Files 2145 2147 +2
Lines 95028 95086 +58
Branches 8504 8507 +3
==========================================
- Hits 61781 57607 -4174
- Misses 31126 35453 +4327
+ Partials 2121 2026 -95 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
🤖 Bitwarden Claude Code ReviewOverall Assessment: APPROVE Reviewed the docs-only follow-up to #6682 that fills in non-Redis L2 coverage for Code Review DetailsNo findings. All seven prior review threads from the previous round are resolved with substantive follow-up commits, including the most recent addition of an explicit SRE / infrastructure coordination section. |
JaredSnider-Bitwarden
left a comment
JaredSnider-Bitwarden
left a comment
There was a problem hiding this comment.
Thank you for adding these doc changes! Claude and I found a few things below (included suggestions for fixes). Happy to discuss!
- Drop L1/L2 shorthand from decision tree; define vocabulary once in the ExtendedCache section. - Fix Option 5 inline comment about the "persistent" keyed cache — Cosmos in cloud, aliased to the unnamed IDistributedCache in self-hosted. - Note that ExtendedCache prefixes every key with cacheName: so aliased consumers cannot collide with the raw "persistent" namespace. - Pairing-with-Cosmos: clarify that FusionCache writes per-document ttl (not container TTL) and call out the RU / eager-refresh tradeoff. - Add a worked Specific Example for ExtendedCache + Cosmos (Long-Lived Per-Tenant Computed Aggregates). - Footnote the Configuration Priority table to point at the opt-in Cosmos pairing path.
Volume, write rate, and cache size are first-order constraints that gate which backend can hold a workload — but the prior decision tree silently assumed the answer fit. Address that: - New "Sizing, Throughput, and Cost" section after the overview table, covering working-set size (L1 bounded by heap, L2 ceilings per backend), read rate (when L1 absorbs vs. becomes overhead), write rate (backplane saturation, eager-refresh cost), and per-backend cost shape. - Callout above the decision tree pointing at the new section. - Decision-tree branches now include sizing/write-rate gates that can override the API recommendation. - SSO grants and Org/Provider Abilities "Why" lists now include the size and rate profile that makes ExtendedCache appropriate there. - ExtendedCache Cons and "When NOT to Use" expanded with the two main outs: working sets too large for L1, and sustained-high write rates that saturate the Redis backplane.
Picking a caching option in code is only half of standing up a new cache. The runtime — Redis sizing, Cosmos provisioning, RU budgets, eviction policies, monitoring — is owned by SRE and is not stood up by the AddExtendedCache / AddDistributedCache registrations. - New "Coordinating with Infrastructure" section after "Sizing, Throughput, and Cost", covering: loop SRE in early, defaults are not production configuration, shared infrastructure has unseen tenants, new backends need provisioning beyond code, and self-hosted environments differ from cloud. - Decision-tree preamble now points at both Sizing and Coordinating with Infrastructure as prerequisites before using the tree.
|
JaredSnider-Bitwarden
left a comment
JaredSnider-Bitwarden
left a comment
There was a problem hiding this comment.
These are really helpful improvements. Thank you very much!
withinfocus
changed the title
2 participants
🎟️ Tracking
Follow-up to #6682, expanded in review.
📔 Objective
#6682 added support for using any
IDistributedCache(Cosmos DB, SQL Server, EF) as the L2 store underExtendedCache, but the docs insrc/Core/Utilities/CACHING.mdonly got a one-line setting rename. This PR closes that gap and, in the course of review, expands the doc to cover two adjacent areas that were also missing or hand-wavy: sizing/throughput/cost, and coordination with the infrastructure team.Non-Redis L2 documentation (original scope)
ExtendedCachewith a non-Redis L2, including the keyed-alias pattern for pairing with the"persistent"Cosmos cache.ExtendedCachemay not fit long-lived data (no backplane → cross-instance L1 staleness) instead of the prior blanket "days/weeks → persistent cache."ExtendedCache+ Cosmos pairing, exercising every FusionCache feature honestly.Review feedback applied
L1/L2vocabulary defined once in the ExtendedCache intro and dropped from the decision tree, which was using it before introduction.Duration→AbsoluteExpirationRelativeToNow→ per-documentttl; containerDefaultTimeToLivemust be enabled for it to apply.ExtendedCache+ Cosmos pairing.ExtendedCacheprefixes every key withcacheName:, so aliased consumers cannot collide with raw"persistent"namespace.AddDistributedCacheregisters Cosmos under"persistent"in self-hosted (it aliases to the unnamedIDistributedCachethere).Sizing, throughput, and cost (new)
A new section after the overview table acknowledges that the decision tree silently assumed the dataset fit in the backends it pointed to. Covers:
Decision tree adds gates for working-set size and sustained-high write rate. ExtendedCache Cons and "When NOT to Use" expanded with the two main outs: working sets too large for L1, and sustained-high write rates that saturate the Redis backplane. SSO grants and Org/Provider Abilities "Why" lists now include the size/rate profile that makes
ExtendedCacheappropriate there.Coordinating with Infrastructure (new)
A new section calling out that
services.AddExtendedCache(...)does not stand up the runtime. Covers:CreateIfNotExists = false+ container-levelDefaultTimeToLiveas the worked example.No code changes —
src/Core/Utilities/CACHING.mdonly. Net diff is ~+230 / -45.📸 Screenshots
N/A — docs-only.