-
Notifications
You must be signed in to change notification settings - Fork 55
feat(cache): in-memory tier, size limits, sharding, observability #1122
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 5 commits
e6d5b28
8c4e5d1
9e3c534
7ba9241
0010914
23b5ce1
b3696ff
60d5f50
ad86a9b
7b2e290
9ed8346
464e1f0
093066e
ff09a48
84560bf
2c062bd
ac7edcd
7f6bbac
9f84eee
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,6 +11,18 @@ import { | |
| const FILE_SYSTEM_CACHE_DIRECTORY = | ||
| Deno.env.get("FILE_SYSTEM_CACHE_DIRECTORY") ?? "/tmp/deco_cache"; | ||
|
|
||
| const CACHE_MAX_ENTRY_SIZE = parseInt( | ||
| Deno.env.get("CACHE_MAX_ENTRY_SIZE") ?? "2097152", // 2 MB | ||
| ); | ||
|
cubic-dev-ai[bot] marked this conversation as resolved.
Outdated
cubic-dev-ai[bot] marked this conversation as resolved.
Outdated
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Inconsistent size validation: The Consider validating size in Suggested fix for inline path async function putFile(
key: string,
responseArray: Uint8Array,
) {
+ if (responseArray.length > CACHE_MAX_ENTRY_SIZE) {
+ return; // Skip caching oversized entries
+ }
if (!isCacheInitialized) {
await assertCacheDirectory();
}The worker path in 🤖 Prompt for AI Agents |
||
|
|
||
| const initializedShardDirs = new Set<string>(); | ||
|
|
||
| function shardedPath(cacheDir: string, key: string): string { | ||
| const l1 = key.substring(0, 2); | ||
| const l2 = key.substring(2, 4); | ||
| return `${cacheDir}/${l1}/${l2}/${key}`; | ||
| } | ||
|
|
||
| // Reuse TextEncoder instance to avoid repeated instantiation | ||
| const textEncoder = new TextEncoder(); | ||
|
|
||
|
|
@@ -121,8 +133,16 @@ function createFileSystemCache(): CacheStorage { | |
| if (!isCacheInitialized) { | ||
| await assertCacheDirectory(); | ||
| } | ||
| const filePath = `${FILE_SYSTEM_CACHE_DIRECTORY}/${key}`; | ||
|
|
||
| const filePath = shardedPath(FILE_SYSTEM_CACHE_DIRECTORY, key); | ||
| const dir = filePath.substring(0, filePath.lastIndexOf("/")); | ||
| if (!initializedShardDirs.has(dir)) { | ||
| try { | ||
| await Deno.mkdir(dir, { recursive: true }); | ||
| initializedShardDirs.add(dir); | ||
| } catch { | ||
| // transient failure — don't mark initialized so next write retries mkdir | ||
| } | ||
| } | ||
| await Deno.writeFile(filePath, responseArray); | ||
| return; | ||
| } | ||
|
|
@@ -132,8 +152,12 @@ function createFileSystemCache(): CacheStorage { | |
| await assertCacheDirectory(); | ||
| } | ||
| try { | ||
| const filePath = `${FILE_SYSTEM_CACHE_DIRECTORY}/${key}`; | ||
| const filePath = shardedPath(FILE_SYSTEM_CACHE_DIRECTORY, key); | ||
| const fileContent = await Deno.readFile(filePath); | ||
| if (fileContent.length > CACHE_MAX_ENTRY_SIZE) { | ||
| Deno.remove(filePath).catch(() => {}); | ||
| return null; | ||
| } | ||
| return fileContent; | ||
| } catch (_err) { | ||
| const err = _err as { code?: string }; | ||
|
|
@@ -151,7 +175,7 @@ function createFileSystemCache(): CacheStorage { | |
| await assertCacheDirectory(); | ||
| } | ||
| try { | ||
| const filePath = `${FILE_SYSTEM_CACHE_DIRECTORY}/${key}`; | ||
| const filePath = shardedPath(FILE_SYSTEM_CACHE_DIRECTORY, key); | ||
| await Deno.remove(filePath); | ||
| return true; | ||
| } catch (err) { | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,85 @@ | ||||||||||||||||||||||||||
| import { LRUCache } from "npm:lru-cache@10.2.0"; | ||||||||||||||||||||||||||
| import { | ||||||||||||||||||||||||||
| assertCanBeCached, | ||||||||||||||||||||||||||
| assertNoOptions, | ||||||||||||||||||||||||||
| baseCache, | ||||||||||||||||||||||||||
| withCacheNamespace, | ||||||||||||||||||||||||||
| } from "./utils.ts"; | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| const MEMORY_CACHE_MAX_SIZE = parseInt( | ||||||||||||||||||||||||||
| Deno.env.get("MEMORY_CACHE_MAX_SIZE") ?? "268435456", // 256 MB | ||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||
|
cubic-dev-ai[bot] marked this conversation as resolved.
Outdated
|
||||||||||||||||||||||||||
| const MEMORY_CACHE_MAX_ITEMS = parseInt( | ||||||||||||||||||||||||||
| Deno.env.get("MEMORY_CACHE_MAX_ITEMS") ?? "2048", | ||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||
|
coderabbitai[bot] marked this conversation as resolved.
Outdated
|
||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| interface CacheEntry { | ||||||||||||||||||||||||||
| body: Uint8Array; | ||||||||||||||||||||||||||
| headers: [string, string][]; | ||||||||||||||||||||||||||
| status: number; | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
coderabbitai[bot] marked this conversation as resolved.
|
||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| function createInMemoryCache(): CacheStorage { | ||||||||||||||||||||||||||
| const store = new LRUCache<string, CacheEntry>({ | ||||||||||||||||||||||||||
| max: MEMORY_CACHE_MAX_ITEMS, | ||||||||||||||||||||||||||
| maxSize: MEMORY_CACHE_MAX_SIZE, | ||||||||||||||||||||||||||
| sizeCalculation: (entry) => entry.body.length, | ||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| const caches: CacheStorage = { | ||||||||||||||||||||||||||
| delete: () => { | ||||||||||||||||||||||||||
| throw new Error("Not Implemented"); | ||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||
| has: () => { | ||||||||||||||||||||||||||
| throw new Error("Not Implemented"); | ||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||
| keys: () => { | ||||||||||||||||||||||||||
| throw new Error("Not Implemented"); | ||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||
| match: () => { | ||||||||||||||||||||||||||
| throw new Error("Not Implemented"); | ||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||
| open: (cacheName: string): Promise<Cache> => { | ||||||||||||||||||||||||||
| const requestURLSHA1 = withCacheNamespace(cacheName); | ||||||||||||||||||||||||||
| return Promise.resolve({ | ||||||||||||||||||||||||||
| ...baseCache, | ||||||||||||||||||||||||||
| delete: async ( | ||||||||||||||||||||||||||
| request: RequestInfo | URL, | ||||||||||||||||||||||||||
| _options?: CacheQueryOptions, | ||||||||||||||||||||||||||
| ): Promise<boolean> => { | ||||||||||||||||||||||||||
|
Comment on lines
+91
to
+94
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🌐 Web query:
💡 Result: The Cache.delete(request, options) method uses the provided CacheQueryOptions (such as ignoreSearch and ignoreMethod) to control how the matching is performed when identifying the cache entry to delete. It finds the Cache entry whose key matches the request according to the matching rules modified by the options, deletes it if found (resolving to true), or resolves to false if no match is found. Specifically: - ignoreSearch: When true, ignores the query string in the URL during matching (defaults to false). - ignoreMethod: When true, does not validate the Request's HTTP method (normally only GET/HEAD allowed; defaults to false). This behavior is identical to how options work in Cache.match and Cache.matchAll. Citations:
🏁 Script executed: # First, let's find and examine the inMemoryCache.ts file
find . -name "inMemoryCache.ts" -type fRepository: deco-cx/deco Length of output: 89 🏁 Script executed: # Once found, let's examine the full delete() and match() implementations
cat -n runtime/caches/inMemoryCache.ts | head -100Repository: deco-cx/deco Length of output: 3189 🏁 Script executed: # Search for assertNoOptions usage in the codebase to understand the pattern
rg "assertNoOptions" --type typescriptRepository: deco-cx/deco Length of output: 90 🏁 Script executed: # Let's check the utils.ts file to see what assertNoOptions does
cat -n runtime/caches/utils.tsRepository: deco-cx/deco Length of output: 4551 🏁 Script executed: # Search for assertNoOptions usage with context
rg "assertNoOptions" -A 2 -B 2 --type tsRepository: deco-cx/deco Length of output: 4070 Add assertion for unsupported The Suggested fix delete: async (
request: RequestInfo | URL,
- _options?: CacheQueryOptions,
+ options?: CacheQueryOptions,
): Promise<boolean> => {
+ assertNoOptions(options);
const cacheKey = await requestURLSHA1(request);
return store.delete(cacheKey);
},📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||
| const cacheKey = await requestURLSHA1(request); | ||||||||||||||||||||||||||
| return store.delete(cacheKey); | ||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||
| match: async ( | ||||||||||||||||||||||||||
| request: RequestInfo | URL, | ||||||||||||||||||||||||||
| options?: CacheQueryOptions, | ||||||||||||||||||||||||||
| ): Promise<Response | undefined> => { | ||||||||||||||||||||||||||
| assertNoOptions(options); | ||||||||||||||||||||||||||
| const cacheKey = await requestURLSHA1(request); | ||||||||||||||||||||||||||
| const entry = store.get(cacheKey); | ||||||||||||||||||||||||||
| if (!entry) return undefined; | ||||||||||||||||||||||||||
| return new Response(entry.body as unknown as BodyInit, { | ||||||||||||||||||||||||||
|
cubic-dev-ai[bot] marked this conversation as resolved.
|
||||||||||||||||||||||||||
| headers: new Headers(entry.headers), | ||||||||||||||||||||||||||
| status: entry.status, | ||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||
| put: async ( | ||||||||||||||||||||||||||
| request: RequestInfo | URL, | ||||||||||||||||||||||||||
| response: Response, | ||||||||||||||||||||||||||
| ): Promise<void> => { | ||||||||||||||||||||||||||
| const req = new Request(request); | ||||||||||||||||||||||||||
| assertCanBeCached(req, response); | ||||||||||||||||||||||||||
| if (!response.body) return; | ||||||||||||||||||||||||||
| const cacheKey = await requestURLSHA1(request); | ||||||||||||||||||||||||||
| const body = new Uint8Array(await response.arrayBuffer()); | ||||||||||||||||||||||||||
| const headers: [string, string][] = [...response.headers.entries()]; | ||||||||||||||||||||||||||
|
Comment on lines
+118
to
+136
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🌐 Web query:
💡 Result: Yes, Response.arrayBuffer returns an empty ArrayBuffer (with byteLength 0) when the response body is null. Caching such responses is valid behavior. In the Fetch API, Response.body is null for responses constructed with null body (e.g., new Response(null)) or per HTTP semantics for no-body statuses like 204 No Content, 205 Reset Content, and 304 Not Modified. However, browsers do not implement the spec's null body exactly; they provide an empty ReadableStream instead. The arrayBuffer method reads the body stream to completion, resolving to an empty ArrayBuffer for null or empty bodies. This is confirmed by MDN documentation stating arrayBuffer resolves with an ArrayBuffer after reading the stream, and practical observations in issues where null bodies become empty ArrayBuffers (e.g., Safari workaround turning null to empty ArrayBuffer). The WHATWG Fetch spec links to body methods like arrayBuffer, and implementations copy the buffer source, yielding empty for no content. Caching Responses with null/empty bodies is valid and common, e.g., in Service Workers via caches.match returning cached empty-body responses (e.g., 304), as shown in WHATWG discussions and examples. Citations:
🏁 Script executed: # Find the file and check its structure
fd -t f "inMemoryCache.ts" runtime/caches/Repository: deco-cx/deco Length of output: 87 🏁 Script executed: # View the specific section around lines 72-75
cat -n runtime/caches/inMemoryCache.ts | sed -n '60,90p'Repository: deco-cx/deco Length of output: 1077 🏁 Script executed: # Find where assertCanBeCached is defined and what it does
rg "assertCanBeCached" -A 5Repository: deco-cx/deco Length of output: 3531 🏁 Script executed: # Check if there are any tests related to empty body caching
rg -i "empty.*body|body.*empty|arrayBuffer.*null" --type ts --type js -A 3Repository: deco-cx/deco Length of output: 224 🏁 Script executed: # Look for any usage of the cache to understand if empty responses are expected
rg "store\.get|store\.set" runtime/caches/ -B 2 -A 2Repository: deco-cx/deco Length of output: 868 🏁 Script executed: # Look for tests related to caching empty bodies or null bodies
find . -type f \( -name "*test*" -o -name "*.test.*" -o -name "*.spec.*" \) | xargs rg -i "cache.*empty|empty.*cache|null.*body|body.*null" 2>/dev/null | head -30Repository: deco-cx/deco Length of output: 243 🏁 Script executed: # Check if there are any comments explaining why the guard exists
rg -B 3 "if.*!response\.body" runtime/caches/Repository: deco-cx/deco Length of output: 1267 🏁 Script executed: # Look for any documentation or comments in the utils file about caching rules
cat -n runtime/caches/utils.ts | head -50Repository: deco-cx/deco Length of output: 1591 Remove the early return guard that prevents caching of responses with null/empty bodies. Line 72 returns early when Suggested fix const req = new Request(request);
assertCanBeCached(req, response);
- if (!response.body) return;
const cacheKey = await requestURLSHA1(request);
const body = new Uint8Array(await response.arrayBuffer());
const headers: [string, string][] = [...response.headers.entries()];
store.set(cacheKey, { body, headers, status: response.status });📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||
| store.set(cacheKey, { body, headers, status: response.status }); | ||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| return caches; | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| export const caches = createInMemoryCache(); | ||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,11 +1,18 @@ | ||
| import { LRUCache } from "npm:lru-cache@10.2.0"; | ||
| import { ValueType } from "../../deps.ts"; | ||
| import { meter } from "../../observability/otel/metrics.ts"; | ||
| import { | ||
| assertCanBeCached, | ||
| assertNoOptions, | ||
| baseCache, | ||
| createBaseCacheStorage, | ||
| } from "./utils.ts"; | ||
|
|
||
| const lruEvictionCounter = meter.createCounter("lru_cache_eviction", { | ||
| unit: "1", | ||
| valueType: ValueType.DOUBLE, | ||
| }); | ||
|
|
||
| // keep compatible with old variable name | ||
| const CACHE_MAX_SIZE = parseInt( | ||
| Deno.env.get("CACHE_MAX_SIZE") ?? Deno.env.get("MAX_CACHE_SIZE") ?? | ||
|
|
@@ -30,17 +37,46 @@ const cacheOptions = (cache: Cache) => ( | |
| maxSize: CACHE_MAX_SIZE, | ||
| ttlAutopurge: CACHE_TTL_AUTOPURGE, | ||
| ttlResolution: CACHE_TTL_RESOLUTION, | ||
| dispose: async (_value: boolean, key: string) => { | ||
| dispose: async (_value: boolean, key: string, reason: string) => { | ||
| lruEvictionCounter.add(1, { reason }); | ||
| await cache.delete(key); | ||
| }, | ||
|
Comment on lines
+42
to
45
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: find . -type f -name "lrucache.ts" | head -5Repository: deco-cx/deco Length of output: 84 🏁 Script executed: cat -n runtime/caches/lrucache.tsRepository: deco-cx/deco Length of output: 7197 🏁 Script executed: npm info lru-cache@10.2.0 descriptionRepository: deco-cx/deco Length of output: 114 🏁 Script executed: # Search for lru-cache documentation or examples in the codebase
rg -i "dispose|noDisposeOnSet" --type ts --type jsRepository: deco-cx/deco Length of output: 3348 🌐 Web query:
💡 Result: In lru-cache version 10.2.0, the dispose callback is provided as an option when creating an LRUCache instance: dispose: (value, key, reason) => {}. It is called whenever an item is removed from the cache, including on eviction (when the cache is full and the least recently used item is dropped), overwrites (when cache.set is called on an existing key), explicit deletes, TTL expiration, or fetch failures. The 'reason' parameter is a string from the DisposeReason enum with these values: - 'evict': Item evicted due to cache being full (LRU eviction). - 'set': Item overwritten by a new value via cache.set (this covers "set overwrites"). - 'delete': Explicit deletion via cache.delete, cache.clear, or cache.set(key, undefined). - 'expire': Item removed due to TTL expiration. - 'fetch': Item deleted due to fetchMethod returning undefined or aborting. There is no 'overwrite' reason; overwrites use 'set'. The feature has been stable since v7.2 when reasons were added to dispose (v10.2.0 inherits this). Use noDisposeOnSet: true to suppress dispose calls specifically on overwrites. The callback runs before the item is fully removed from the cache. Citations:
Filter dispose callback to only delete on true evictions, not overwrites. In Lines 40-43, the dispose callback unconditionally deletes the backing store for all disposal reasons. The 🤖 Prompt for AI Agents |
||
| } | ||
| ); | ||
|
|
||
| const lruSizeGauge = meter.createObservableGauge("lru_cache_keys", { | ||
| description: "number of keys in the LRU cache", | ||
| unit: "1", | ||
| valueType: ValueType.DOUBLE, | ||
| }); | ||
|
|
||
| const lruBytesGauge = meter.createObservableGauge("lru_cache_bytes", { | ||
| description: "total bytes tracked by the LRU cache", | ||
| unit: "bytes", | ||
| valueType: ValueType.DOUBLE, | ||
| }); | ||
|
|
||
| // deno-lint-ignore no-explicit-any | ||
| const activeCaches = new Map<string, LRUCache<string, any>>(); | ||
|
|
||
| lruSizeGauge.addCallback((observer) => { | ||
| for (const [name, lru] of activeCaches) { | ||
| observer.observe(lru.size, { cache: name }); | ||
| } | ||
| }); | ||
|
|
||
| lruBytesGauge.addCallback((observer) => { | ||
| for (const [name, lru] of activeCaches) { | ||
| observer.observe(lru.calculatedSize, { cache: name }); | ||
| } | ||
| }); | ||
|
|
||
| function createLruCacheStorage(cacheStorageInner: CacheStorage): CacheStorage { | ||
| const caches = createBaseCacheStorage( | ||
| cacheStorageInner, | ||
| (_cacheName, cacheInner, requestURLSHA1) => { | ||
| const fileCache = new LRUCache(cacheOptions(cacheInner)); | ||
| activeCaches.set(_cacheName, fileCache); | ||
| return Promise.resolve({ | ||
| ...baseCache, | ||
| delete: async ( | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.