From 56a9d5601b571b4dc213947e2f1b12fe4475ce7a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Samy=20Pess=C3=A9?= Date: Mon, 11 Mar 2024 12:30:26 +0000 Subject: [PATCH] Fix cache revalidation in KV and output stats on revalidation (#231) * Store setAt on cache meta and output stats on revalidation * Delete from KV cache * test more * Improve * Format * Store the key in metadata * Avoid storing too much in KV * Don't put tag on resolve of url --- src/app/(global)/~gitbook/revalidate/route.ts | 3 +- src/lib/api.ts | 13 +--- src/lib/cache/cache.ts | 21 ++++-- src/lib/cache/cloudflare-kv.ts | 17 ++++- src/lib/cache/redis.ts | 26 ++++---- src/lib/cache/revalidateTags.ts | 66 +++++++++---------- src/lib/cache/types.ts | 12 ++-- 7 files changed, 89 insertions(+), 69 deletions(-) diff --git a/src/app/(global)/~gitbook/revalidate/route.ts b/src/app/(global)/~gitbook/revalidate/route.ts index 3413bc7f4..eeb61d4fa 100644 --- a/src/app/(global)/~gitbook/revalidate/route.ts +++ b/src/app/(global)/~gitbook/revalidate/route.ts @@ -25,10 +25,11 @@ export async function POST(req: NextRequest) { ); } - const result = await revalidateTags(json.tags, !!json.purge); + const result = await revalidateTags(json.tags); return NextResponse.json({ success: true, keys: result.keys, + stats: result.stats, }); } diff --git a/src/lib/api.ts b/src/lib/api.ts index e48d62166..8ec3aaa61 100644 --- a/src/lib/api.ts +++ b/src/lib/api.ts @@ -44,7 +44,7 @@ export interface ContentTarget { /** * Parameter to cache an entry as an immutable one (ex: revisions, documents). * It'll cache it for 1 week and revalidate it 24h before expiration. - * + * * We don't cache for more than this to ensure we don't use too much storage and keep the cache small. */ const immutableCacheTtl = { @@ -202,20 +202,13 @@ export const getPublishedContentByUrl = cache( const parsed = parseCacheResponse(response); - const tags = [ - ...parsed.tags, - ...('space' in response.data - ? [getAPICacheTag({ tag: 'space', space: response.data.space })] - : []), - ]; - const data: PublishedContentWithCache = { ...response.data, cacheMaxAge: parsed.ttl, - cacheTags: tags, + cacheTags: parsed.tags, }; return { - tags, + tags: parsed.tags, ttl: parsed.ttl, data, }; diff --git a/src/lib/cache/cache.ts b/src/lib/cache/cache.ts index 580f59b0c..f15df5ef7 100644 --- a/src/lib/cache/cache.ts +++ b/src/lib/cache/cache.ts @@ -71,14 +71,17 @@ export function cache( const result = await fn(...args, { signal }); signal?.throwIfAborted(); + const setAt = Date.now(); const expiresAt = - Date.now() + (result.ttl ?? options.defaultTtl ?? 60 * 60 * 24) * 1000; + setAt + (result.ttl ?? options.defaultTtl ?? 60 * 60 * 24) * 1000; const cacheEntry: CacheEntry = { data: result.data, meta: { + key, cache: cacheName, tags: result.tags ?? [], + setAt, expiresAt, revalidatesAt: result.revalidateBefore ? expiresAt - result.revalidateBefore * 1000 @@ -242,9 +245,7 @@ export function getCache(name: string): CacheFunction | null { * Get a cache key for a function and its arguments. */ export function getCacheKey(fnName: string, args: any[]) { - let innerKey = args - .map((arg) => (typeof arg === 'object' && !!arg ? hash(arg) : JSON.stringify(arg))) - .join(','); + let innerKey = args.map((arg) => hashValue(arg)).join(','); // Avoid crazy long keys, by fallbacking to a hash if (innerKey.length > 400) { @@ -254,6 +255,18 @@ export function getCacheKey(fnName: string, args: any[]) { return `${fnName}(${innerKey})`; } +function hashValue(arg: any): string { + if (arg === undefined) { + return ''; + } + + if (typeof arg === 'object' && !!arg) { + return hash(arg); + } + + return JSON.stringify(arg); +} + async function setCacheEntry(key: string, entry: CacheEntry) { return await trace( { diff --git a/src/lib/cache/cloudflare-kv.ts b/src/lib/cache/cloudflare-kv.ts index 710ccbb5e..8a04f6753 100644 --- a/src/lib/cache/cloudflare-kv.ts +++ b/src/lib/cache/cloudflare-kv.ts @@ -34,7 +34,7 @@ export const cloudflareKVCache: CacheBackend = { const entry = await kv.get(kvKey, { type: 'json', - cacheTtl: 2 * 60, + cacheTtl: 60, }); span.setAttribute('hit', !!entry); @@ -69,7 +69,7 @@ export const cloudflareKVCache: CacheBackend = { if (entry.meta.tags.length > 0) { const metadata: KVTagMetadata = { - key: key, + key, // TODO: Remove this key from the metadata in 1 day meta: entry.meta, }; const jsonMetadata = JSON.stringify(metadata); @@ -113,6 +113,8 @@ export const cloudflareKVCache: CacheBackend = { return result; } + const pendingDeletions: Array> = []; + await Promise.all( tags.map(async (tag) => { const entries = await kv.list({ @@ -124,13 +126,22 @@ export const cloudflareKVCache: CacheBackend = { for (const entry of entries.keys) { if (entry.metadata) { const metadata = entry.metadata as KVTagMetadata; + + const key = metadata.meta.key ?? metadata.key; + result.metas.push(metadata.meta); - result.keys.push(metadata.key); + result.keys.push(key); + + // Delete the tag key and the value key + pendingDeletions.push(kv.delete(getValueKey(key))); + pendingDeletions.push(kv.delete(entry.name)); } } }), ); + await Promise.all(pendingDeletions); + return result; }, }; diff --git a/src/lib/cache/redis.ts b/src/lib/cache/redis.ts index c8eb87522..e42b25f8c 100644 --- a/src/lib/cache/redis.ts +++ b/src/lib/cache/redis.ts @@ -94,7 +94,7 @@ export const redisCache: CacheBackend = { await multi.exec(); }, - async revalidateTags(tags, purge) { + async revalidateTags(tags) { const redis = getRedis(); if (!redis) { return { keys: [], metas: [] }; @@ -109,20 +109,18 @@ export const redisCache: CacheBackend = { if (keys.size > 0) { // Read the meta - if (!purge) { - metas = ( - await redis.mget>( - // Hard limit to avoid fetching a massive list of data - // Starts with the smallest keys. - Array.from(keys) - .sort((a, b) => a.length - b.length) - .slice(0, 50) - .map((key) => getCacheEntryKey(key, 'meta')), - ) + metas = ( + await redis.mget>( + // Hard limit to avoid fetching a massive list of data + // Starts with the smallest keys. + Array.from(keys) + .sort((a, b) => a.length - b.length) + .slice(0, 50) + .map((key) => getCacheEntryKey(key, 'meta')), ) - .flat() - .filter(filterOutNullable); - } + ) + .flat() + .filter(filterOutNullable); // Delete all keys keys.forEach((key) => { diff --git a/src/lib/cache/revalidateTags.ts b/src/lib/cache/revalidateTags.ts index b2d400b87..13c7453b7 100644 --- a/src/lib/cache/revalidateTags.ts +++ b/src/lib/cache/revalidateTags.ts @@ -1,24 +1,30 @@ -import pMap from 'p-map'; - import { cacheBackends } from './backends'; -import { getCache, getCacheKey } from './cache'; +import { getCacheKey } from './cache'; import { CacheEntryMeta } from './types'; -import { waitUntil } from '../waitUntil'; + +interface RevalidateTagsStats { + [key: string]: { + /** + * Backends that have the key. + */ + [backend: string]: { set: boolean; setAt?: number; expiresAt?: number }; + }; +} /** * Revalidate all values associated with tags. * It clears the values from the caches, but also start a background task to revalidate them. */ -export async function revalidateTags( - tags: string[], - purge: boolean, -): Promise<{ +export async function revalidateTags(tags: string[]): Promise<{ keys: string[]; + stats: RevalidateTagsStats; }> { if (tags.length === 0) { - return { keys: [] }; + return { keys: [], stats: {} }; } + const stats: RevalidateTagsStats = {}; + const processed = new Set(); const keysByBackend = new Map(); @@ -27,16 +33,24 @@ export async function revalidateTags( await Promise.all( cacheBackends.map(async (backend, backendIndex) => { - const { keys: addedKeys, metas: addedMetas } = await backend.revalidateTags( - tags, - purge, - ); + const { keys: addedKeys, metas: addedMetas } = await backend.revalidateTags(tags); + + console.log('revalidateTags', backend.name, addedKeys); + addedKeys.forEach((key) => { + stats[key] = stats[key] ?? {}; + stats[key][backend.name] = { set: true }; + keys.add(key); keysByBackend.set(backendIndex, [...(keysByBackend.get(backendIndex) ?? []), key]); }); addedMetas.forEach((meta) => { - const key = getCacheKey(meta.cache, meta.args); + const key = meta.key ?? getCacheKey(meta.cache, meta.args); + stats[key] = stats[key] ?? {}; + stats[key][backend.name] = { set: true }; + stats[key][backend.name].setAt = meta.setAt; + stats[key][backend.name].expiresAt = meta.expiresAt; + if (!processed.has(key)) { metas.push(meta); processed.add(key); @@ -53,31 +67,17 @@ export async function revalidateTags( ); if (unclearedKeys.length > 0) { + unclearedKeys.forEach((key) => { + stats[key][backend.name] = { set: false }; + }); + await backend.del(unclearedKeys); } }), ); - // Refresh the values in the cache in the background - if (metas && !purge) { - await waitUntil( - pMap( - metas, - async (meta) => { - console.log(`revalidating ${meta.cache} with args`, meta.args); - const cache = getCache(meta.cache); - if (cache) { - await cache.revalidate(...meta.args); - } - }, - { - concurrency: 5, - }, - ), - ); - } - return { keys: Array.from(keys.keys()), + stats, }; } diff --git a/src/lib/cache/types.ts b/src/lib/cache/types.ts index 23717ebc9..aaad436d3 100644 --- a/src/lib/cache/types.ts +++ b/src/lib/cache/types.ts @@ -1,4 +1,11 @@ export interface CacheEntryMeta { + key?: string; + + /** + * Timestamp when the entry was created. + */ + setAt: number; + /** * Name of the function that was cached. */ @@ -59,8 +66,5 @@ export interface CacheBackend { * Revalidate all keys associated with tags. * It should return the meta of all entries that were revalidated. */ - revalidateTags( - tags: string[], - purge: boolean, - ): Promise<{ keys: string[]; metas: CacheEntryMeta[] }>; + revalidateTags(tags: string[]): Promise<{ keys: string[]; metas: CacheEntryMeta[] }>; }