feat(cache): add L1 in-memory cache tier with admission filter#1127
Open
vibegui wants to merge 5 commits into
Open
feat(cache): add L1 in-memory cache tier with admission filter#1127vibegui wants to merge 5 commits into
vibegui wants to merge 5 commits into
Conversation
- Add inMemoryCache (256MB LRU) as L1 tier ahead of filesystem - Wire as: headersCache → LRU → tiered(inMemory, fileSystem) - L1 admission filter (MEMORY_CACHE_MIN_HITS=2) prevents one-hit wonders - L1 eviction logging with periodic warnings when cache is full - Fast path: skip body read on L1 hit with no backfill needed - X-Cache-Tier header for observability (stripped before response) - Import shared inFuture from utils (eliminates 2x Date allocation) - Add bgRevalidation and cacheEntrySize histograms - Comprehensive tests for tiered cache and in-memory cache Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
Tagging OptionsShould a new tag be published when this PR is merged?
|
📝 WalkthroughWalkthroughThis PR introduces a two-tier caching architecture combining an in-memory L1 cache with existing file-system storage. It implements LRU-based in-memory cache with admission filtering and eviction tracking, updates the cache stack to use both tiers, and adds tracing enrichment to capture content length and cache tier information in spans. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant TieredCache as Tiered Cache<br/>(orchestrator)
participant L1 as L1: InMemory<br/>Cache
participant L2 as L2: FileSystem<br/>Cache
Client->>TieredCache: cache.match(request)
TieredCache->>L1: match(request)
alt L1 Hit (Fresh)
L1-->>TieredCache: Response + tier:0
TieredCache-->>Client: Response + tier:0<br/>(fast path)
else L1 Miss or Stale
TieredCache->>L2: match(request)
alt L2 Hit
L2-->>TieredCache: Response + tier:1
TieredCache->>TieredCache: Read body once
TieredCache->>L1: put(request, response)<br/>(backfill)
L1-->>TieredCache: ✓
TieredCache-->>Client: Response + tier:1<br/>(slow path)
else L2 Miss
TieredCache-->>Client: undefined
end
end
Client->>TieredCache: cache.put(request, response)
TieredCache->>L1: put(request, response)
L1-->>TieredCache: ✓
TieredCache->>L2: put(request, response)
L2-->>TieredCache: ✓
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~40 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
3 issues found across 7 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="runtime/caches/inMemoryCache.ts">
<violation number="1" location="runtime/caches/inMemoryCache.ts:49">
P2: `l1_cache_eviction` is incremented for non-eviction disposals (`set`/`delete`), so the metric overreports cache pressure.</violation>
</file>
<file name="runtime/caches/tiered.test.ts">
<violation number="1" location="runtime/caches/tiered.test.ts:117">
P2: This assertion is ineffective because L1 was already populated before the call, so it cannot validate that backfill replaced stale data.</violation>
</file>
<file name="blocks/loader.ts">
<violation number="1" location="blocks/loader.ts:381">
P2: Do not record JSON parse duration into `resolver_latency`; use a dedicated histogram to avoid mixing two different latency semantics in one metric.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
…rse metric, min hits default - Only count actual evictions (reason=evict) in l1_cache_eviction counter - Fix stale backfill test to assert fresh body content, not just presence - Remove json_parse timing from resolver_latency (mixing semantics) - Bump MEMORY_CACHE_MIN_HITS default from 2 to 3 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
runtime/caches/tiered.ts (1)
88-115:⚠️ Potential issue | 🟠 MajorPreserve the last stale hit instead of overwriting it with a later miss.
matchedis assigned before the miss check, so a stale response from an upper tier is lost if a lower tier misses afterwards. In thestale L1 / miss L2path this turns a stale fallback into a full miss, and the fallback tier index is wrong as well.Suggested fix
let matched: Response | undefined; let matchedTierIndex: number | undefined; const indexOfCachesToUpdate: number[] = []; for (const [index, cache] of openedCaches.entries()) { - matched = await cache.match(request, options).catch(() => + const candidate = await cache.match(request, options).catch(() => undefined ); - if (!matched) { + if (!candidate) { indexOfCachesToUpdate.push(index); continue; } + matched = candidate; + matchedTierIndex = index; - const expires = matched.headers.get("expires"); + const expires = candidate.headers.get("expires"); const isStale = expires ? !inFuture(expires) : false; if (!isStale) { // found a match that is not stale, no need to check the other caches - matchedTierIndex = index; break; } indexOfCachesToUpdate.push(index); } if (!matched) return undefined; - const tierIndex = matchedTierIndex ?? (openedCaches.length - 1); + const tierIndex = matchedTierIndex!;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@runtime/caches/tiered.ts` around lines 88 - 115, The loop currently assigns matched before checking for a miss, so a previous stale matched response can be overwritten by a later miss; change the match assignment pattern in the loop to first await cache.match into a temporary variable (e.g., let candidate = await cache.match(...).catch(() => undefined)) and only assign matched = candidate and matchedTierIndex = index when candidate is non-undefined; when candidate is stale keep it (do not clear matched) and continue scanning for a non-stale hit, still pushing misses to indexOfCachesToUpdate as before; this preserves the last stale hit from an upper tier and keeps matchedTierIndex accurate while leaving indexOfCachesToUpdate logic unchanged (refer to variables matched, matchedTierIndex, indexOfCachesToUpdate, openedCaches, and helper inFuture).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@runtime/caches/inMemoryCache.ts`:
- Around line 123-137: The admission gate currently resets a key's counter on
every put causing the first refresh to be skipped; fix this in the put path
(after computing cacheKey via requestURLSHA1) by checking whether the key
already exists in the L1 store (store.has(cacheKey) or equivalent) and, if so,
bypass the admissionCounts increment/return logic and immediately perform
store.set(cacheKey, { body, headers, status }) (also ensure any admissionCounts
entry for that key is cleared). This keeps MEMORY_CACHE_MIN_HITS enforcement for
cold keys but allows updates/refreshes to cached entries to write through
immediately.
In `@runtime/caches/tiered.test.ts`:
- Around line 1-180: This file fails formatting checks; run the code formatter
and commit the changes: run `deno fmt` (or your project's formatter) on
runtime/caches/tiered.test.ts to normalize spacing/indentation and ensure
imports/exports and test blocks (e.g., functions mapCacheStorage, freshResponse,
staleResponse, and the Deno.test blocks) conform to the project's style, then
stage and push the formatted file.
---
Outside diff comments:
In `@runtime/caches/tiered.ts`:
- Around line 88-115: The loop currently assigns matched before checking for a
miss, so a previous stale matched response can be overwritten by a later miss;
change the match assignment pattern in the loop to first await cache.match into
a temporary variable (e.g., let candidate = await cache.match(...).catch(() =>
undefined)) and only assign matched = candidate and matchedTierIndex = index
when candidate is non-undefined; when candidate is stale keep it (do not clear
matched) and continue scanning for a non-stale hit, still pushing misses to
indexOfCachesToUpdate as before; this preserves the last stale hit from an upper
tier and keeps matchedTierIndex accurate while leaving indexOfCachesToUpdate
logic unchanged (refer to variables matched, matchedTierIndex,
indexOfCachesToUpdate, openedCaches, and helper inFuture).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a148d999-92a9-4c56-9451-ec829df26153
📒 Files selected for processing (7)
blocks/loader.tsruntime/caches/common.tsruntime/caches/inMemoryCache.test.tsruntime/caches/inMemoryCache.tsruntime/caches/mod.tsruntime/caches/tiered.test.tsruntime/caches/tiered.ts
Comment on lines
+1
to
+180
| import { assertEquals, assertNotEquals } from "@std/assert"; | ||
| import { createTieredCache } from "./tiered.ts"; | ||
|
|
||
| const NOT_IMPL = (): never => { | ||
| throw new Error("Not Implemented"); | ||
| }; | ||
|
|
||
| // Simple map-backed CacheStorage. Keys are raw URL strings. | ||
| function mapCacheStorage(map = new Map<string, Response>()): CacheStorage { | ||
| const url = (r: RequestInfo | URL) => | ||
| typeof r === "string" ? r : (r as Request).url ?? String(r); | ||
| return { | ||
| delete: NOT_IMPL, | ||
| has: NOT_IMPL, | ||
| keys: NOT_IMPL, | ||
| match: NOT_IMPL, | ||
| open: (_name: string) => | ||
| Promise.resolve({ | ||
| add: NOT_IMPL, | ||
| addAll: NOT_IMPL, | ||
| keys: NOT_IMPL, | ||
| matchAll: NOT_IMPL, | ||
| delete: (r: RequestInfo | URL) => | ||
| Promise.resolve(map.delete(url(r))), | ||
| match: (r: RequestInfo | URL) => | ||
| Promise.resolve(map.get(url(r))), | ||
| put: (r: RequestInfo | URL, res: Response) => { | ||
| map.set(url(r), res); | ||
| return Promise.resolve(); | ||
| }, | ||
| } as Cache), | ||
| }; | ||
| } | ||
|
|
||
| const REQ = new Request("https://example.com/item"); | ||
| const CACHE = "test"; | ||
|
|
||
| function freshResponse(body = "data", status = 200): Response { | ||
| return new Response(body, { | ||
| status, | ||
| headers: { | ||
| expires: new Date(Date.now() + 60_000).toUTCString(), | ||
| "content-length": String(new TextEncoder().encode(body).length), | ||
| "content-type": "application/json", | ||
| }, | ||
| }); | ||
| } | ||
|
|
||
| function staleResponse(body = "stale"): Response { | ||
| return new Response(body, { | ||
| headers: { | ||
| // expired far enough back that STALE_TTL_PERIOD (30s) doesn't save it | ||
| expires: new Date(Date.now() - 120_000).toUTCString(), | ||
| "content-length": String(new TextEncoder().encode(body).length), | ||
| }, | ||
| }); | ||
| } | ||
|
|
||
| // flush fire-and-forget microtasks (backfill is not awaited in tiered.ts) | ||
| const flush = () => new Promise<void>((r) => setTimeout(r, 0)); | ||
|
|
||
| Deno.test("tiered / single tier: hit sets X-Cache-Tier: 0", async () => { | ||
| const tiered = createTieredCache(mapCacheStorage()); | ||
| const cache = await tiered.open(CACHE); | ||
| await cache.put(REQ, freshResponse()); | ||
|
|
||
| const result = await cache.match(REQ); | ||
| assertNotEquals(result, undefined); | ||
| assertEquals(result!.headers.get("x-cache-tier"), "0"); | ||
| }); | ||
|
|
||
| Deno.test("tiered / single tier: miss returns undefined", async () => { | ||
| const tiered = createTieredCache(mapCacheStorage()); | ||
| const cache = await tiered.open(CACHE); | ||
|
|
||
| assertEquals(await cache.match(REQ), undefined); | ||
| }); | ||
|
|
||
| Deno.test("tiered / single tier: preserves non-200 status", async () => { | ||
| const tiered = createTieredCache(mapCacheStorage()); | ||
| const cache = await tiered.open(CACHE); | ||
| await cache.put(REQ, freshResponse("not found", 404)); | ||
|
|
||
| const result = await cache.match(REQ); | ||
| assertEquals(result?.status, 404); | ||
| }); | ||
|
|
||
| Deno.test("tiered / two tiers: L1 hit returns X-Cache-Tier: 0, no backfill", async () => { | ||
| const l1 = new Map<string, Response>(); | ||
| const l2 = new Map<string, Response>(); | ||
| const tiered = createTieredCache(mapCacheStorage(l1), mapCacheStorage(l2)); | ||
| const cache = await tiered.open(CACHE); | ||
|
|
||
| // put distributes to both tiers | ||
| await cache.put(REQ, freshResponse()); | ||
|
|
||
| const result = await cache.match(REQ); | ||
| assertEquals(result?.headers.get("x-cache-tier"), "0"); | ||
| // L1 hit means L2 was never needed — both had the entry from put, L1 was checked first | ||
| }); | ||
|
|
||
| Deno.test("tiered / two tiers: L1 miss, L2 hit — X-Cache-Tier: 1, backfills L1", async () => { | ||
| const l1 = new Map<string, Response>(); | ||
| const l2 = new Map<string, Response>(); | ||
| // Pre-populate only L2 directly | ||
| l2.set(REQ.url, freshResponse()); | ||
|
|
||
| const tiered = createTieredCache(mapCacheStorage(l1), mapCacheStorage(l2)); | ||
| const cache = await tiered.open(CACHE); | ||
|
|
||
| const result = await cache.match(REQ); | ||
| assertNotEquals(result, undefined); | ||
| assertEquals(result!.headers.get("x-cache-tier"), "1"); | ||
|
|
||
| await flush(); | ||
| // L1 should now be backfilled | ||
| assertNotEquals(l1.get(REQ.url), undefined); | ||
| }); | ||
|
|
||
| Deno.test("tiered / two tiers: all miss returns undefined", async () => { | ||
| const tiered = createTieredCache(mapCacheStorage(), mapCacheStorage()); | ||
| const cache = await tiered.open(CACHE); | ||
|
|
||
| assertEquals(await cache.match(REQ), undefined); | ||
| }); | ||
|
|
||
| Deno.test("tiered / two tiers: stale L1, fresh L2 — serves L2, backfills L1", async () => { | ||
| const l1 = new Map<string, Response>(); | ||
| const l2 = new Map<string, Response>(); | ||
| l1.set(REQ.url, staleResponse("old")); | ||
| l2.set(REQ.url, freshResponse("new")); | ||
|
|
||
| const tiered = createTieredCache(mapCacheStorage(l1), mapCacheStorage(l2)); | ||
| const cache = await tiered.open(CACHE); | ||
|
|
||
| const result = await cache.match(REQ); | ||
| assertNotEquals(result, undefined); | ||
| // Served from L2 (stale L1 was skipped) | ||
| assertEquals(result!.headers.get("x-cache-tier"), "1"); | ||
| assertEquals(await result!.text(), "new"); | ||
|
|
||
| await flush(); | ||
| // L1 gets backfilled with fresh data | ||
| assertNotEquals(l1.get(REQ.url), undefined); | ||
| }); | ||
|
|
||
| Deno.test("tiered / put: distributes to all tiers", async () => { | ||
| const l1 = new Map<string, Response>(); | ||
| const l2 = new Map<string, Response>(); | ||
| const tiered = createTieredCache(mapCacheStorage(l1), mapCacheStorage(l2)); | ||
| const cache = await tiered.open(CACHE); | ||
|
|
||
| await cache.put(REQ, freshResponse()); | ||
| assertNotEquals(l1.get(REQ.url), undefined); | ||
| assertNotEquals(l2.get(REQ.url), undefined); | ||
| }); | ||
|
|
||
| Deno.test("tiered / put: single tier does not read body unnecessarily", async () => { | ||
| // Verify single-tier put path works (no double-read) | ||
| const map = new Map<string, Response>(); | ||
| const tiered = createTieredCache(mapCacheStorage(map)); | ||
| const cache = await tiered.open(CACHE); | ||
|
|
||
| await cache.put(REQ, freshResponse("body")); | ||
| const result = await cache.match(REQ); | ||
| assertEquals(await result!.text(), "body"); | ||
| }); | ||
|
|
||
| Deno.test("tiered / status: non-200 response preserved through multi-tier", async () => { | ||
| const l1 = new Map<string, Response>(); | ||
| const l2 = new Map<string, Response>(); | ||
| l2.set(REQ.url, freshResponse("gone", 410)); | ||
|
|
||
| const tiered = createTieredCache(mapCacheStorage(l1), mapCacheStorage(l2)); | ||
| const cache = await tiered.open(CACHE); | ||
|
|
||
| const result = await cache.match(REQ); | ||
| assertEquals(result?.status, 410); | ||
| assertEquals(result?.headers.get("x-cache-tier"), "1"); | ||
| }); |
There was a problem hiding this comment.
Run deno fmt on this file before merge.
CI is already failing on deno fmt --check here.
🧰 Tools
🪛 GitHub Actions: ci
[error] 23-26: deno fmt --check found 2 not formatted files (including this region in tiered.test.ts).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@runtime/caches/tiered.test.ts` around lines 1 - 180, This file fails
formatting checks; run the code formatter and commit the changes: run `deno fmt`
(or your project's formatter) on runtime/caches/tiered.test.ts to normalize
spacing/indentation and ensure imports/exports and test blocks (e.g., functions
mapCacheStorage, freshResponse, staleResponse, and the Deno.test blocks) conform
to the project's style, then stage and push the formatted file.
Contributor
There was a problem hiding this comment.
1 issue found across 4 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="runtime/caches/inMemoryCache.ts">
<violation number="1" location="runtime/caches/inMemoryCache.ts:27">
P1: The admission filter applies unconditionally, even when the key is already resident in L1. On a refresh, `put()` resets the counter to 1 and returns early — the new value is silently dropped and the stale entry stays in memory until `MEMORY_CACHE_MIN_HITS` (3) additional writes arrive. Wrap the admission gate in a `!store.has(cacheKey)` check so that already-admitted keys are updated immediately.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
runtime/caches/inMemoryCache.ts (1)
29-33: Consider usingValueType.INTfor eviction counter.Eviction events are discrete occurrences.
ValueType.INTis the more semantically appropriate choice for counters that increment by whole numbers, thoughDOUBLEwill work correctly.🔧 Suggested change
const l1EvictionCounter = meter.createCounter("l1_cache_eviction", { description: "number of entries evicted from the L1 in-memory cache", unit: "1", - valueType: ValueType.DOUBLE, + valueType: ValueType.INT, });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@runtime/caches/inMemoryCache.ts` around lines 29 - 33, The eviction counter uses ValueType.DOUBLE but eviction events are discrete; change the counter creation at meter.createCounter for l1EvictionCounter to use ValueType.INT instead of ValueType.DOUBLE so the metric semantically represents integer increments.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@runtime/caches/inMemoryCache.ts`:
- Around line 29-33: The eviction counter uses ValueType.DOUBLE but eviction
events are discrete; change the counter creation at meter.createCounter for
l1EvictionCounter to use ValueType.INT instead of ValueType.DOUBLE so the metric
semantically represents integer increments.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 64d38eeb-65f8-4683-882c-f7fc816a5fdc
📒 Files selected for processing (4)
blocks/loader.tsruntime/caches/inMemoryCache.test.tsruntime/caches/inMemoryCache.tsruntime/caches/tiered.test.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- runtime/caches/tiered.test.ts
- blocks/loader.ts
- runtime/caches/inMemoryCache.test.ts
Histograms, bgRevalidation timer, and cache entry size recording now ship in the correctness+observability PR for earlier visibility. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
When a key is already in L1 and gets refreshed (e.g. background revalidation), the admission filter was blocking the update — the stale value stayed in RAM until 3 more writes. Now keys already in the store bypass admission and update immediately. Also runs deno fmt on test files. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (2)
runtime/caches/inMemoryCache.test.ts (1)
87-93: Consider adding a test for admission bypass on existing keys.The implementation allows immediate refresh for keys already in L1 (bypassing the admission filter). A test verifying that a 4th put with updated content is immediately visible would strengthen coverage of this behavior.
🧪 Suggested test case
Deno.test("inMemoryCache: existing key bypasses admission on refresh", async () => { const cache = await caches.open(nextCache()); await putUntilAdmitted(cache, REQ, () => new Response("original")); assertEquals(await (await cache.match(REQ))!.text(), "original"); // A single put should update the existing entry (no re-admission needed) await cache.put(REQ, new Response("updated")); assertEquals(await (await cache.match(REQ))!.text(), "updated"); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@runtime/caches/inMemoryCache.test.ts` around lines 87 - 93, Add a test that verifies existing L1 keys bypass the admission filter by putting a new Response for an admitted key and confirming the updated content is immediately visible: open a cache with nextCache(), use putUntilAdmitted(cache, REQ, ...) to create the initial "original" entry, assert cache.match(REQ) returns "original", then call cache.put(REQ, new Response("updated")) and assert cache.match(REQ) now returns "updated". Reference putUntilAdmitted, cache.put, cache.match, nextCache, and REQ in the test.runtime/caches/inMemoryCache.ts (1)
29-33: Consider usingValueType.INTfor the counter.The eviction counter only increments by whole numbers.
ValueType.INTwould be more semantically appropriate thanDOUBLE.♻️ Proposed fix
const l1EvictionCounter = meter.createCounter("l1_cache_eviction", { description: "number of entries evicted from the L1 in-memory cache", unit: "1", - valueType: ValueType.DOUBLE, + valueType: ValueType.INT, });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@runtime/caches/inMemoryCache.ts` around lines 29 - 33, The l1 cache eviction counter uses ValueType.DOUBLE but only ever increments by whole numbers; update the meter.createCounter call that assigns l1EvictionCounter to use ValueType.INT instead of ValueType.DOUBLE (ensure the reference to ValueType in the file stays correct/imported) so the metric is semantically an integer counter.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@runtime/caches/inMemoryCache.test.ts`:
- Around line 87-93: Add a test that verifies existing L1 keys bypass the
admission filter by putting a new Response for an admitted key and confirming
the updated content is immediately visible: open a cache with nextCache(), use
putUntilAdmitted(cache, REQ, ...) to create the initial "original" entry, assert
cache.match(REQ) returns "original", then call cache.put(REQ, new
Response("updated")) and assert cache.match(REQ) now returns "updated".
Reference putUntilAdmitted, cache.put, cache.match, nextCache, and REQ in the
test.
In `@runtime/caches/inMemoryCache.ts`:
- Around line 29-33: The l1 cache eviction counter uses ValueType.DOUBLE but
only ever increments by whole numbers; update the meter.createCounter call that
assigns l1EvictionCounter to use ValueType.INT instead of ValueType.DOUBLE
(ensure the reference to ValueType in the file stays correct/imported) so the
metric is semantically an integer counter.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3821f62a-84d5-4069-b906-a80161936bf3
📒 Files selected for processing (3)
runtime/caches/inMemoryCache.test.tsruntime/caches/inMemoryCache.tsruntime/caches/tiered.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- runtime/caches/tiered.test.ts
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.
Summary
inMemoryCache(256MB LRU) as L1 tier ahead of filesystemheadersCache → LRU → tiered(inMemory, fileSystem)MEMORY_CACHE_MIN_HITS=2) prevents one-hit wonders from displacing hot keysarrayBuffer()read on L1 hit with no backfill neededX-Cache-Tierheader for observability (stripped before response reaches caller)inFuturefrom utils (eliminates 2× Date allocation per call)bgRevalidationandcacheEntrySizehistogramsEnv vars
MEMORY_CACHE_MAX_SIZEMEMORY_CACHE_MAX_ITEMSMEMORY_CACHE_MIN_HITSCACHE_MAX_ENTRY_SIZETest plan
cache_tier=0(L1 hit) in traces for hot keyscache_tier=1(L2/filesystem) and get backfilled to L1l1_cache_evictioncounter — should be low relative to trafficdeno test runtime/caches/tiered.test.ts runtime/caches/inMemoryCache.test.ts🤖 Generated with Claude Code
PR 4 of 5 — split from #1122. Merge order: 1 → 2 → 3 → 4 → 5
Summary by cubic
Adds a 256MB in-memory LRU cache as an L1 tier in front of the filesystem cache to cut latency and reduce disk I/O. Includes an admission filter with smarter refresh handling, tier tagging, and an eviction counter for visibility.
New Features
inMemoryCache(256MB, up to 2048 items) wired asheadersCache → LRU → tiered(inMemory, fileSystem).MEMORY_CACHE_MIN_HITS=3) and max entry size (CACHE_MAX_ENTRY_SIZE=2MB); keys already in L1 bypass the gate so refreshes update immediately.X-Cache-Tierheader (stripped before response), span attrscache_tierandcontent_length.l1_cache_evictioncounter (actual evictions only). Tests cover in-memory behavior and tiered backfill.Dependencies
npm:lru-cache@10.2.0for the in-memory LRU implementation.Written for commit 460a9b4. Summary will update on new commits.
Summary by CodeRabbit
New Features
Tests