From f81b114b690e8aa4381861dd8903161a7de10075 Mon Sep 17 00:00:00 2001 From: openhands Date: Fri, 25 Sep 2026 18:42:23 +0200 Subject: [PATCH] perf(cache): single-flight avatar renders, cacheable public reads, cheap row counts Three separate things that were each costing more than they needed to on the hot path. - Single-flight avatar renders. The disk cache was checked first and a miss went straight to the upstream, with nothing shared between callers, so a page requesting dozens of avatars at once turned N concurrent requests for one figure into N renders. A render is the most expensive operation this app does, and the duplication happened exactly when the cache had nothing to offer. Eight concurrent requests now cause one render instead of eight. The map lives on globalThis because Next can evaluate the module more than once per process, and two copies would each start their own render. - Let public read-only routes be cached by a shared cache. Every JSON response was `cache-control: no-store`, so a CDN in front of the app could not answer any of it and every request reached the origin. publicCacheControl() opts a route in with s-maxage and stale-while-revalidate, using the same TTL as the server-side cache so the two layers cannot disagree. The default stays no-store: most routes here are personalised, admin-only or auth-dependent. /api/badges/leaderboard is deliberately left alone because it returns per-viewer rank entries to signed-in callers. Note this only takes effect once a cache rule exists for /api/* at the CDN, or the explicit `cache: "no-store"` is dropped from the client fetches (24 files do that today, including the /api/online poll). The headers alone are inert until one of those happens. - Take the homepage row counts from the storage engine estimate instead of COUNT(*), which walks an index and gets slower as the tables grow. A missing or zero estimate falls back to the exact count rather than ever showing a wrong zero. The online count stays exact: it is an indexed read over a small subset and a few seconds of drift reads as broken rather than approximate. The counters move into one module because the homepage and the boot warm-up populate the same cache keys, so two implementations would race to write different values into the same entry. 3223 tests pass. --- src/app/(site)/page.tsx | 46 +++---- src/app/api/guilds/route.ts | 6 +- src/app/api/leaderboard/route.ts | 7 +- src/app/api/online/count/route.ts | 7 +- src/app/api/online/route.ts | 7 +- src/app/api/photos/route.ts | 6 +- src/app/api/radio/current-dj/route.ts | 7 +- src/app/api/radio/points/leaderboard/route.ts | 7 +- src/app/api/radio/shouts/route.ts | 7 +- src/app/api/shop/categories/route.ts | 7 +- src/app/api/shop/route.ts | 12 +- src/app/api/staff/route.ts | 7 +- src/app/api/teams/route.ts | 7 +- src/app/api/values/[id]/route.ts | 6 +- src/app/api/values/categories/route.ts | 7 +- src/app/api/values/route.ts | 12 +- src/lib/api.test.ts | 38 +++++- src/lib/api.ts | 17 +++ src/lib/imager-upstream.test.ts | 83 ++++++++++++ src/lib/imager-upstream.ts | 35 ++++++ src/lib/services/cache-warmup.ts | 33 ++--- src/lib/services/public-counters.test.ts | 118 +++++++++++++++++ src/lib/services/public-counters.ts | 119 ++++++++++++++++++ 23 files changed, 514 insertions(+), 87 deletions(-) create mode 100644 src/lib/imager-upstream.test.ts create mode 100644 src/lib/services/public-counters.test.ts create mode 100644 src/lib/services/public-counters.ts diff --git a/src/app/(site)/page.tsx b/src/app/(site)/page.tsx index 31396a0c..ee6ca9c4 100644 --- a/src/app/(site)/page.tsx +++ b/src/app/(site)/page.tsx @@ -18,11 +18,16 @@ import { ThemeSwitcher } from "@/components/theme-switcher"; import { TypewriterText } from "@/components/typewriter-text"; import { auth } from "@/lib/auth"; import { cached } from "@/lib/cache"; -import { CameraWeb, db, Rooms, User } from "@/lib/db"; +import { CameraWeb, db, User } from "@/lib/db"; import { formatDate } from "@/lib/format-date"; import { resolveHotelName } from "@/lib/hotel-name"; import { captchaConfig } from "@/lib/services/captcha"; import { getNewsList } from "@/lib/services/news-list"; +import { + countPhotos, + countRooms, + countUsers, +} from "@/lib/services/public-counters"; import { publicReadFailure } from "@/lib/services/public-read"; import { siteSettings } from "@/lib/services/site-settings"; import { @@ -181,36 +186,15 @@ async function getHotelData() { .where(eq(User.online, "1")) .then((rows) => rows[0]?.total ?? 0), ).catch(publicReadFailure("home.online")), - cached( - "total_users", - 300_000, - () => - db - .select({ total: count() }) - .from(User) - .then((rows) => rows[0]?.total ?? 0), - { staleMs: 300000 }, - ).catch(publicReadFailure("home.users")), - cached( - "total_rooms", - 300_000, - () => - db - .select({ total: count() }) - .from(Rooms) - .then((rows) => rows[0]?.total ?? 0), - { staleMs: 300000 }, - ).catch(publicReadFailure("home.rooms")), - cached( - "total_photos", - 300_000, - () => - db - .select({ total: count() }) - .from(CameraWeb) - .then((rows) => rows[0]?.total ?? 0), - { staleMs: 300000 }, - ).catch(publicReadFailure("home.photos-count")), + cached("total_users", 300_000, countUsers, { staleMs: 300000 }).catch( + publicReadFailure("home.users"), + ), + cached("total_rooms", 300_000, countRooms, { staleMs: 300000 }).catch( + publicReadFailure("home.rooms"), + ), + cached("total_photos", 300_000, countPhotos, { staleMs: 300000 }).catch( + publicReadFailure("home.photos-count"), + ), getNewsList(4, { throwOnError: true }).catch( publicReadFailure("home.news"), ), diff --git a/src/app/api/guilds/route.ts b/src/app/api/guilds/route.ts index d276f6f1..3f24816c 100644 --- a/src/app/api/guilds/route.ts +++ b/src/app/api/guilds/route.ts @@ -3,7 +3,7 @@ // fields. The owner is stored as the user_id column (accessor userId), surfaced // here as ownerId for API clarity. import { count, desc } from "drizzle-orm"; -import { apiJson, pagination } from "@/lib/api"; +import { apiJson, pagination, publicCacheControl } from "@/lib/api"; import { db, Guilds } from "@/lib/db"; import { apiCacheKey, cacheSafe, redisCache } from "@/lib/redis-cache"; @@ -53,7 +53,9 @@ export async function GET(req: Request) { { staleMs: 120_000 }, ); - return apiJson(data); + return apiJson(data, { + headers: { "cache-control": publicCacheControl(120) }, + }); } catch { // DB unreachable — never 500; return an empty, well-formed payload. return apiJson( diff --git a/src/app/api/leaderboard/route.ts b/src/app/api/leaderboard/route.ts index e9535714..74db9aa2 100644 --- a/src/app/api/leaderboard/route.ts +++ b/src/app/api/leaderboard/route.ts @@ -1,5 +1,5 @@ import { desc, eq, inArray } from "drizzle-orm"; -import { apiJson } from "@/lib/api"; +import { apiJson, publicCacheControl } from "@/lib/api"; import { db, User, UsersCurrency } from "@/lib/db"; import { apiCacheKey, redisCache } from "@/lib/redis-cache"; @@ -89,7 +89,10 @@ export async function GET(req: Request) { { staleMs: 60 }, ); - return apiJson({ type, data: rows }, { status: 200 }); + return apiJson( + { type, data: rows }, + { headers: { "cache-control": publicCacheControl(60) }, status: 200 }, + ); } catch { return apiJson({ type: "credits", data: [] }, { status: 200 }); } diff --git a/src/app/api/online/count/route.ts b/src/app/api/online/count/route.ts index 8d299816..015210a9 100644 --- a/src/app/api/online/count/route.ts +++ b/src/app/api/online/count/route.ts @@ -4,7 +4,7 @@ // { count: 0 } (never 500) on DB failure. import { count, eq } from "drizzle-orm"; -import { apiJson } from "@/lib/api"; +import { apiJson, publicCacheControl } from "@/lib/api"; import { cached } from "@/lib/cache"; import { db, User } from "@/lib/db"; @@ -25,7 +25,10 @@ export async function GET(_req: Request) { }, { staleMs: 15_000 }, ); - return apiJson({ count: result }); + return apiJson( + { count: result }, + { headers: { "cache-control": publicCacheControl(10) } }, + ); } catch { return apiJson({ count: 0 }); } diff --git a/src/app/api/online/route.ts b/src/app/api/online/route.ts index 2d183fb5..32ed3f71 100644 --- a/src/app/api/online/route.ts +++ b/src/app/api/online/route.ts @@ -4,7 +4,7 @@ // Capped at 100 rows. Returns empty data (never 500) on DB failure. import { eq } from "drizzle-orm"; -import { apiJson } from "@/lib/api"; +import { apiJson, publicCacheControl } from "@/lib/api"; import { cached } from "@/lib/cache"; import { db, User } from "@/lib/db"; @@ -24,7 +24,10 @@ export async function GET(_req: Request) { { staleMs: 15_000 }, ); - return apiJson({ users }); + return apiJson( + { users }, + { headers: { "cache-control": publicCacheControl(10) } }, + ); } catch { return apiJson({ users: [] }); } diff --git a/src/app/api/photos/route.ts b/src/app/api/photos/route.ts index 4ec59743..603d0091 100644 --- a/src/app/api/photos/route.ts +++ b/src/app/api/photos/route.ts @@ -1,5 +1,5 @@ import { count, desc } from "drizzle-orm"; -import { apiJson, pagination } from "@/lib/api"; +import { apiJson, pagination, publicCacheControl } from "@/lib/api"; import { CameraWeb, db } from "@/lib/db"; import { apiCacheKey, cacheSafe, redisCache } from "@/lib/redis-cache"; @@ -47,7 +47,9 @@ export async function GET(req: Request) { { staleMs: 120_000 }, ); - return apiJson(data); + return apiJson(data, { + headers: { "cache-control": publicCacheControl(60) }, + }); } catch { // DB unavailable — return an empty payload instead of a 500. return apiJson( diff --git a/src/app/api/radio/current-dj/route.ts b/src/app/api/radio/current-dj/route.ts index 1ce83a3b..cfc7cafa 100644 --- a/src/app/api/radio/current-dj/route.ts +++ b/src/app/api/radio/current-dj/route.ts @@ -1,5 +1,5 @@ import { eq } from "drizzle-orm"; -import { apiJson } from "@/lib/api"; +import { apiJson, publicCacheControl } from "@/lib/api"; import { db, User } from "@/lib/db"; import { apiCacheKey, redisCache } from "@/lib/redis-cache"; import { siteSettings } from "@/lib/services/site-settings"; @@ -41,7 +41,10 @@ export async function GET(_req: Request) { { staleMs: 30_000 }, ); - return apiJson({ dj }); + return apiJson( + { dj }, + { headers: { "cache-control": publicCacheControl(30) } }, + ); } catch { // DB / settings unavailable — no DJ rather than a 500. return apiJson({ dj: null }, { status: 200 }); diff --git a/src/app/api/radio/points/leaderboard/route.ts b/src/app/api/radio/points/leaderboard/route.ts index 048342d5..99c240d0 100644 --- a/src/app/api/radio/points/leaderboard/route.ts +++ b/src/app/api/radio/points/leaderboard/route.ts @@ -1,5 +1,5 @@ import { desc, inArray, sum } from "drizzle-orm"; -import { apiJson } from "@/lib/api"; +import { apiJson, publicCacheControl } from "@/lib/api"; import { db, RadioListenerPoints, User } from "@/lib/db"; import { apiCacheKey, redisCache } from "@/lib/redis-cache"; @@ -51,7 +51,10 @@ export async function GET(_req: Request) { { staleMs: 120_000 }, ); - return apiJson({ data }); + return apiJson( + { data }, + { headers: { "cache-control": publicCacheControl(60) } }, + ); } catch { // DB unavailable — serve an empty leaderboard rather than a 500. return apiJson({ data: [] }, { status: 200 }); diff --git a/src/app/api/radio/shouts/route.ts b/src/app/api/radio/shouts/route.ts index 15a66297..ec904780 100644 --- a/src/app/api/radio/shouts/route.ts +++ b/src/app/api/radio/shouts/route.ts @@ -1,5 +1,5 @@ import { desc, inArray } from "drizzle-orm"; -import { apiError, apiJson } from "@/lib/api"; +import { apiError, apiJson, publicCacheControl } from "@/lib/api"; import { bearerUserId } from "@/lib/api-auth"; import { db, RadioShouts, User } from "@/lib/db"; import { rateLimit } from "@/lib/rate-limit"; @@ -61,7 +61,10 @@ export async function GET(_req: Request) { { staleMs: 10_000 }, ); - return apiJson({ shouts: data }); + return apiJson( + { shouts: data }, + { headers: { "cache-control": publicCacheControl(10) } }, + ); } catch { // DB unavailable — serve an empty list rather than a 500. return apiJson({ shouts: [] }, { status: 200 }); diff --git a/src/app/api/shop/categories/route.ts b/src/app/api/shop/categories/route.ts index da368113..84c099a3 100644 --- a/src/app/api/shop/categories/route.ts +++ b/src/app/api/shop/categories/route.ts @@ -1,7 +1,7 @@ // Public REST: website store categories (website_shop_categories). // AtomCMS JSON API parity — read-only list ordered by `order` then name. import { asc } from "drizzle-orm"; -import { apiJson } from "@/lib/api"; +import { apiJson, publicCacheControl } from "@/lib/api"; import { db, WebsiteShopCategories } from "@/lib/db"; import { apiCacheKey, cacheSafe, redisCache } from "@/lib/redis-cache"; @@ -29,7 +29,10 @@ export async function GET(_req: Request) { { staleMs: 120_000 }, ); - return apiJson({ data }); + return apiJson( + { data }, + { headers: { "cache-control": publicCacheControl(60) } }, + ); } catch { // DB unreachable — never 500; return empty data. return apiJson({ data: [] }, { status: 200 }); diff --git a/src/app/api/shop/route.ts b/src/app/api/shop/route.ts index c02fb34b..b0dfb911 100644 --- a/src/app/api/shop/route.ts +++ b/src/app/api/shop/route.ts @@ -2,7 +2,13 @@ // AtomCMS JSON API parity — read-only list of buyable packages, paginated and // ordered by `position` (then name), matching the admin /admin/shop query. import { asc, count, eq } from "drizzle-orm"; -import { apiError, apiJson, pagination, positiveBigInt } from "@/lib/api"; +import { + apiError, + apiJson, + pagination, + positiveBigInt, + publicCacheControl, +} from "@/lib/api"; import { db, WebsiteShopArticles } from "@/lib/db"; import { apiCacheKey, cacheSafe, redisCache } from "@/lib/redis-cache"; @@ -74,7 +80,9 @@ export async function GET(req: Request) { { staleMs: 120_000 }, ); - return apiJson(data); + return apiJson(data, { + headers: { "cache-control": publicCacheControl(60) }, + }); } catch { // DB unreachable — never 500; return an empty, well-formed payload. return apiJson( diff --git a/src/app/api/staff/route.ts b/src/app/api/staff/route.ts index e4a52d7f..821829dc 100644 --- a/src/app/api/staff/route.ts +++ b/src/app/api/staff/route.ts @@ -1,5 +1,5 @@ import { asc, desc, gte } from "drizzle-orm"; -import { apiJson } from "@/lib/api"; +import { apiJson, publicCacheControl } from "@/lib/api"; import { db, User } from "@/lib/db"; import { apiCacheKey, redisCache } from "@/lib/redis-cache"; import { siteSettings } from "@/lib/services/site-settings"; @@ -32,7 +32,10 @@ export async function GET(_req: Request) { { staleMs: 300 }, ); - return apiJson({ data: staff }, { status: 200 }); + return apiJson( + { data: staff }, + { headers: { "cache-control": publicCacheControl(300) }, status: 200 }, + ); } catch { // Never 500 — serve an empty payload if the DB is unreachable. return apiJson({ data: [] }, { status: 200 }); diff --git a/src/app/api/teams/route.ts b/src/app/api/teams/route.ts index 5e5baa4a..f43cd640 100644 --- a/src/app/api/teams/route.ts +++ b/src/app/api/teams/route.ts @@ -1,5 +1,5 @@ import { asc, eq } from "drizzle-orm"; -import { apiJson } from "@/lib/api"; +import { apiJson, publicCacheControl } from "@/lib/api"; import { db, WebsiteTeams } from "@/lib/db"; import { apiCacheKey, cacheSafe, redisCache } from "@/lib/redis-cache"; @@ -29,7 +29,10 @@ export async function GET(_req: Request) { ); // apiJson serialises BigInt ids → string automatically. - return apiJson({ data: rows }, { status: 200 }); + return apiJson( + { data: rows }, + { headers: { "cache-control": publicCacheControl(300) }, status: 200 }, + ); } catch { return apiJson({ data: [] }, { status: 200 }); } diff --git a/src/app/api/values/[id]/route.ts b/src/app/api/values/[id]/route.ts index dd3ab18e..30406647 100644 --- a/src/app/api/values/[id]/route.ts +++ b/src/app/api/values/[id]/route.ts @@ -2,7 +2,7 @@ // together with its parent category. AtomCMS JSON API parity. Returns { error } // (404) when the id is unknown. import { eq } from "drizzle-orm"; -import { apiJson } from "@/lib/api"; +import { apiJson, publicCacheControl } from "@/lib/api"; import { db, WebsiteRareValueCategories, WebsiteRareValues } from "@/lib/db"; import { apiCacheKey, cacheSafe, redisCache } from "@/lib/redis-cache"; @@ -83,7 +83,9 @@ export async function GET( return apiJson({ error: "Rare value not found" }, { status: 404 }); } - return apiJson(data); + return apiJson(data, { + headers: { "cache-control": publicCacheControl(300) }, + }); } catch { // DB unavailable — treat as not found rather than a 500. return apiJson({ error: "Rare value not found" }, { status: 200 }); diff --git a/src/app/api/values/categories/route.ts b/src/app/api/values/categories/route.ts index eb2e4054..91b4a450 100644 --- a/src/app/api/values/categories/route.ts +++ b/src/app/api/values/categories/route.ts @@ -2,7 +2,7 @@ // AtomCMS JSON API parity — read-only list ordered by priority then name, // matching the admin /admin/rare-values query. import { asc } from "drizzle-orm"; -import { apiJson } from "@/lib/api"; +import { apiJson, publicCacheControl } from "@/lib/api"; import { db, WebsiteRareValueCategories } from "@/lib/db"; import { apiCacheKey, cacheSafe, redisCache } from "@/lib/redis-cache"; @@ -29,7 +29,10 @@ export async function GET(_req: Request) { { staleMs: 600_000 }, ); - return apiJson({ data }); + return apiJson( + { data }, + { headers: { "cache-control": publicCacheControl(300) } }, + ); } catch { // DB unreachable — never 500; return empty data. return apiJson({ data: [] }, { status: 200 }); diff --git a/src/app/api/values/route.ts b/src/app/api/values/route.ts index 1c8551b3..6c4f26ad 100644 --- a/src/app/api/values/route.ts +++ b/src/app/api/values/route.ts @@ -2,7 +2,13 @@ // AtomCMS JSON API parity — read-only catalog of rares with their credit / // currency values. Supports ?category= filter; paginated, ordered by name. import { asc, count, eq } from "drizzle-orm"; -import { apiError, apiJson, pagination, positiveBigInt } from "@/lib/api"; +import { + apiError, + apiJson, + pagination, + positiveBigInt, + publicCacheControl, +} from "@/lib/api"; import { db, WebsiteRareValues } from "@/lib/db"; import { apiCacheKey, cacheSafe, redisCache } from "@/lib/redis-cache"; @@ -64,7 +70,9 @@ export async function GET(req: Request) { { staleMs: 300_000 }, ); - return apiJson(data); + return apiJson(data, { + headers: { "cache-control": publicCacheControl(300) }, + }); } catch { // DB unreachable — never 500; return an empty, well-formed payload. return apiJson( diff --git a/src/lib/api.test.ts b/src/lib/api.test.ts index f0ab304b..6dea8976 100644 --- a/src/lib/api.test.ts +++ b/src/lib/api.test.ts @@ -1,6 +1,20 @@ // @ts-nocheck import { describe, expect, it } from "vitest"; -import { apiOk, apiUnavailable, pagination, positiveBigInt } from "@/lib/api"; +import { + apiJson, + apiOk, + apiUnavailable, + pagination, + positiveBigInt, + publicCacheControl, +} from "@/lib/api"; + +// Mirrors how a public read-only route opts in. +function publiclyCacheable(data: unknown, seconds: number) { + return apiJson(data, { + headers: { "cache-control": publicCacheControl(seconds) }, + }); +} describe("apiOk", () => { it("returns a BigInt-safe success envelope with ok: true", async () => { @@ -67,3 +81,25 @@ describe("pagination", () => { }); }); }); + +describe("publicCacheControl", () => { + it("lets a shared cache answer without the request reaching the app", async () => { + const response = publiclyCacheable({ rows: [1, 2, 3] }, 120); + + // s-maxage is the part a CDN in front of the app acts on. + expect(response.headers.get("cache-control")).toContain("s-maxage=120"); + expect(response.headers.get("cache-control")).toContain("public"); + await expect(response.json()).resolves.toEqual({ rows: [1, 2, 3] }); + }); + + it("keeps the previous body servable while it refreshes", () => { + expect(publicCacheControl(60)).toContain("stale-while-revalidate=60"); + }); + + it("stays no-store by default, because most routes here are personal", async () => { + // Auth, admin and per-viewer routes keep this. If this ever defaults to + // something cacheable, private data ends up in a shared cache. + expect(apiOk({}).headers.get("cache-control")).toBe("no-store"); + expect(publiclyCacheable({ a: 1 }, 60).status).toBe(200); + }); +}); diff --git a/src/lib/api.ts b/src/lib/api.ts index 61674c83..89fd1f14 100644 --- a/src/lib/api.ts +++ b/src/lib/api.ts @@ -7,6 +7,23 @@ import { NextResponse } from "next/server"; */ const CORS_ORIGIN = process.env.APP_URL ?? "http://localhost:3000"; +/** + * Opt a response in to HTTP caching. + * + * The default is deliberately `no-store`: most routes here are personalised, + * admin-only, or auth-dependent, and caching any of those would be a data leak. + * A route may only pass this when its body is fully public and identical for + * every caller. + * + * `s-maxage` is what lets a shared cache (the CDN in front of this app) answer + * without the request reaching the app at all; `max-age` covers the browser. + * `stale-while-revalidate` lets that cache keep serving the previous body while + * it refreshes, which is the same trade-off the in-process cache makes. + */ +export function publicCacheControl(seconds: number): string { + return `public, max-age=${seconds}, s-maxage=${seconds}, stale-while-revalidate=${seconds}`; +} + export function apiJson(data: unknown, init?: ResponseInit): NextResponse { const body = JSON.stringify(data, (_k, v) => typeof v === "bigint" ? v.toString() : v, diff --git a/src/lib/imager-upstream.test.ts b/src/lib/imager-upstream.test.ts new file mode 100644 index 00000000..1b43abab --- /dev/null +++ b/src/lib/imager-upstream.test.ts @@ -0,0 +1,83 @@ +// @ts-nocheck +import { beforeEach, expect, it, vi } from "vitest"; + +const state = vi.hoisted(() => ({ upstreamCalls: 0 })); + +vi.mock("@/lib/imager-cache", () => ({ + avatarCacheDir: () => "/tmp/imager-test", + imagingCacheKey: (value: string) => value, + // Nothing is ever cached on disk, so every call reaches the render path and + // the in-flight map is the only thing that can dedupe it. + readImagingCache: vi.fn(async () => null), + writeImagingCache: vi.fn(async () => {}), +})); +vi.mock("@/lib/runtime-asset-config", () => ({ + resolveImagerBase: (origin: string) => `${origin}/imager`, +})); + +import { fetchAvatarImage, ImagerUnavailableError } from "./imager-upstream"; + +const params = new URLSearchParams({ figure: "hr-1", direction: "2" }); + +beforeEach(() => { + state.upstreamCalls = 0; + globalThis.imagingInFlight?.clear(); + // A render is not instantaneous, so every caller launched in the same tick + // is genuinely in flight at the same time. That overlap is what the + // in-flight map has to collapse. + vi.stubGlobal( + "fetch", + vi.fn(async () => { + state.upstreamCalls += 1; + await new Promise((resolve) => setTimeout(resolve, 5)); + return new Response(new Uint8Array([1, 2, 3]), { + headers: { "content-type": "image/png" }, + }); + }), + ); +}); + +it("renders once for many concurrent requests of the same figure", async () => { + // This is the whole point: a page asks for dozens of avatars at once and + // repeats figures, and a render is the most expensive thing this app does. + const results = await Promise.all( + Array.from({ length: 8 }, () => + fetchAvatarImage("https://site.test", params), + ), + ); + + expect(state.upstreamCalls).toBe(1); + for (const result of results) { + expect([...result.body]).toEqual([1, 2, 3]); + expect(result.source).toBe("primary"); + } +}); + +it("does not dedupe different figures", async () => { + const other = new URLSearchParams({ figure: "hr-2", direction: "2" }); + await Promise.all([ + fetchAvatarImage("https://site.test", params), + fetchAvatarImage("https://site.test", other), + ]); + expect(state.upstreamCalls).toBe(2); +}); + +it("clears the in-flight entry so a later request reads the cache again", async () => { + // A leaked entry would pin one promise per figure for the process lifetime + // and serve the same body forever. + await fetchAvatarImage("https://site.test", params); + expect(globalThis.imagingInFlight?.size ?? 0).toBe(0); +}); + +it("clears the in-flight entry when the render fails", async () => { + vi.stubGlobal( + "fetch", + vi.fn(async () => { + throw Error("upstream down"); + }), + ); + await expect( + fetchAvatarImage("https://habbo.com/habbo-imaging/avatarimage", params), + ).rejects.toThrow(ImagerUnavailableError); + expect(globalThis.imagingInFlight?.size ?? 0).toBe(0); +}); diff --git a/src/lib/imager-upstream.ts b/src/lib/imager-upstream.ts index fecf5a6c..9f903c63 100644 --- a/src/lib/imager-upstream.ts +++ b/src/lib/imager-upstream.ts @@ -22,6 +22,18 @@ const HABBO_PUBLIC_UPSTREAM = "https://www.habbo.com/habbo-imaging/avatarimage"; export const PRIMARY_TIMEOUT_MS = 8_000; export const FALLBACK_TIMEOUT_MS = 2_000; +// Keyed by cache key, so one render serves every concurrent request for the same +// figure. Held on globalThis because Next can evaluate this module more than once +// per process (multiple server bundles); separate copies would each start their +// own render and defeat the point entirely. +const globalForRenders = globalThis as typeof globalThis & { + imagingInFlight?: Map>; +}; +const inFlight = globalForRenders.imagingInFlight ?? new Map(); +globalForRenders.imagingInFlight = inFlight; + +const MAX_IN_FLIGHT_RENDERS = 256; + export type ImagerSource = | "primary" | "fallback" @@ -107,6 +119,29 @@ export async function fetchAvatarImage( const cached = await readImagingCache(avatarCacheDir(), cacheKey); if (cached) return { ...cached, source: "cache" }; + // Single-flight a render. A page requests dozens of avatars at once and + // repeats figures, so without this a cold cache turns N concurrent requests + // for one figure into N upstream renders — the most expensive operation in + // the app, launched exactly when the cache has nothing to offer. The winner + // writes to the disk cache; the others wait for that one result. + const pending = inFlight.get(cacheKey); + if (pending) return pending; + + // Defensive bound: entries are removed as soon as they settle, so this only + // matters if renders are somehow never cleaned up. + if (inFlight.size >= MAX_IN_FLIGHT_RENDERS) inFlight.clear(); + const render = renderAvatar(primary, cacheKey, params).finally(() => { + inFlight.delete(cacheKey); + }); + inFlight.set(cacheKey, render); + return render; +} + +async function renderAvatar( + primary: string, + cacheKey: string, + params: URLSearchParams, +): Promise { const primaryResult = await fetchUpstream( primary, params, diff --git a/src/lib/services/cache-warmup.ts b/src/lib/services/cache-warmup.ts index 4ce0e0a5..12e467cb 100644 --- a/src/lib/services/cache-warmup.ts +++ b/src/lib/services/cache-warmup.ts @@ -1,11 +1,17 @@ import "server-only"; -import { asc, count, desc, eq, gte } from "drizzle-orm"; +import { asc, desc, eq, gte } from "drizzle-orm"; import { cached } from "@/lib/cache"; -import { CameraWeb, db, Rooms, User, WebsiteTeams } from "@/lib/db"; +import { db, User, WebsiteTeams } from "@/lib/db"; import { logger } from "@/lib/logger"; import { apiCacheKey, redisCache } from "@/lib/redis-cache"; import { cacheNews } from "@/lib/services/news-cache"; +import { + countOnline, + countPhotos, + countRooms, + countUsers, +} from "@/lib/services/public-counters"; import { siteSettings } from "@/lib/services/site-settings"; /** @@ -73,29 +79,6 @@ export async function warmPublicCaches(): Promise { ); } -async function countOnline(): Promise { - const [row] = await db - .select({ total: count() }) - .from(User) - .where(eq(User.online, "1")); - return row?.total ?? 0; -} - -async function countUsers(): Promise { - const [row] = await db.select({ total: count() }).from(User); - return row?.total ?? 0; -} - -async function countRooms(): Promise { - const [row] = await db.select({ total: count() }).from(Rooms); - return row?.total ?? 0; -} - -async function countPhotos(): Promise { - const [row] = await db.select({ total: count() }).from(CameraWeb); - return row?.total ?? 0; -} - async function listOnlineUsers() { return db .select({ username: User.username, look: User.look }) diff --git a/src/lib/services/public-counters.test.ts b/src/lib/services/public-counters.test.ts new file mode 100644 index 00000000..7d1ceaf5 --- /dev/null +++ b/src/lib/services/public-counters.test.ts @@ -0,0 +1,118 @@ +// @ts-nocheck +import { beforeEach, expect, it, vi } from "vitest"; + +const state = vi.hoisted(() => ({ + // Rows the fake information_schema reports, keyed by table name. + estimates: {} as Record, + failEstimate: false, + // Value the exact COUNT(*) returns, and how often it actually ran. + exactCounts: 0, + exactRuns: 0, +})); + +const tables = vi.hoisted(() => ({ + User: { name: "users" }, + Rooms: { name: "rooms" }, + CameraWeb: { name: "camera_web" }, +})); + +vi.mock("@/lib/db", () => { + const chain: any = new Proxy(() => {}, { + get: (_t, prop) => { + if (prop === Symbol.toStringTag) return "Query"; + if (prop === "where") return () => chain; + if (prop === "then") + return (resolve: any, reject: any) => { + // Only the estimate lookup is meant to fail here; the exact + // fallback has to keep working, so it is not gated. + state.exactRuns += 1; + return Promise.resolve([{ total: state.exactCounts }]).then( + resolve, + reject, + ); + }; + return () => chain; + }, + }); + return { + db: { + select: () => chain, + // The information_schema lookup, the only one that matters here. + execute: async () => { + if (state.failEstimate) throw Error("information_schema unavailable"); + return Object.entries(state.estimates) + .filter(([, v]) => v !== null) + .map(([TABLE_NAME, TABLE_ROWS]) => ({ TABLE_NAME, TABLE_ROWS })); + }, + }, + ...(tables as any), + }; +}); + +import { + countOnline, + countPhotos, + countRooms, + countUsers, + estimatedRowCounts, +} from "./public-counters"; + +beforeEach(() => { + state.estimates = {}; + state.failEstimate = false; + state.exactCounts = 0; + state.exactRuns = 0; +}); + +it("reads the engine estimate instead of scanning the table", async () => { + // An exact count is recorded, so a non-zero value here means one really ran. + state.estimates = { users: 123_456, rooms: 42, camera_web: 7 }; + expect(await countUsers()).toBe(123_456); + expect(await countRooms()).toBe(42); + expect(await countPhotos()).toBe(7); + expect(state.exactRuns).toBe(0); +}); + +it("falls back to the exact count when the engine has no estimate", async () => { + // A freshly created InnoDB table can report 0, which would show "0 members". + state.estimates = { users: 0, rooms: null, camera_web: null }; + state.exactCounts = 999; + expect(await countUsers()).toBe(999); + expect(await countRooms()).toBe(999); + expect(await countPhotos()).toBe(999); + expect(state.exactRuns).toBe(3); +}); + +it("falls back to the exact count when the estimate lookup fails", async () => { + state.failEstimate = true; + state.exactCounts = 55; + expect(await countUsers()).toBe(55); +}); + +it("never turns a missing estimate into a zero", async () => { + // Showing "0" is worse than being slow: it is visibly wrong. + state.estimates = { users: null }; + state.exactCounts = 1; + expect(await countUsers()).toBe(1); + expect(await countUsers()).not.toBe(0); +}); + +it("handles the bigint string the driver may hand back", async () => { + state.estimates = { users: "98765" }; + expect(await countUsers()).toBe(98_765); +}); + +it("returns a null for every requested table it has no estimate for", async () => { + expect(await estimatedRowCounts(["users", "rooms"])).toEqual({ + users: null, + rooms: null, + }); +}); + +it("keeps the online count exact", async () => { + // Being a few seconds behind looks broken, and it is an indexed count of a + // small subset, so there is nothing to gain by approximating it. + state.estimates = { users: 5_000_000 }; + state.exactCounts = 12; + expect(await countOnline()).toBe(12); +}); diff --git a/src/lib/services/public-counters.ts b/src/lib/services/public-counters.ts new file mode 100644 index 00000000..97248b0d --- /dev/null +++ b/src/lib/services/public-counters.ts @@ -0,0 +1,119 @@ +import "server-only"; + +import { count, eq, sql } from "drizzle-orm"; +import { CameraWeb, db, Rooms, User } from "@/lib/db"; + +/** + * Row counters for the public homepage. + * + * These live in one place on purpose: the homepage and the boot warm-up both + * populate the same cache keys, so two different implementations would race to + * write different values into the same entry. + */ + +/** Physical table names, needed because `information_schema` speaks in strings. */ +const TABLE = { + users: "users", + rooms: "rooms", + photos: "camera_web", +} as const; + +/** + * Ask the storage engine for its own row estimates. + * + * An exact `COUNT(*)` on InnoDB walks an index, so it gets slower as the table + * grows. For a homepage counter that a visitor sees as a rounded "member since" + * number, an approximate count is the right trade: it is a single indexed + * lookup on `information_schema` instead of a scan of the whole table. + * + * InnoDB's estimate can lag behind reality and, right after a bulk write, can be + * badly off. It is therefore never used as a reason to show zero: a missing + * estimate falls back to the exact count, so the worst case is the old + * behaviour. + * + * Returns null for any table the engine has no estimate for. + */ +export async function estimatedRowCounts( + tables: string[], +): Promise> { + const result: Record = {}; + for (const table of tables) result[table] = null; + if (tables.length === 0) return result; + + try { + const names = tables.map((table) => sql`${table}`); + const rows = await db.execute<{ + TABLE_NAME: string; + TABLE_ROWS: number | string | null; + }>(sql` + SELECT TABLE_NAME, TABLE_ROWS + FROM information_schema.TABLES + WHERE TABLE_SCHEMA = DATABASE() + AND TABLE_NAME IN (${sql.join(names, sql`, `)}) + `); + + for (const row of rows as unknown as { + TABLE_NAME: string; + TABLE_ROWS: number | string | null; + }[]) { + const value = Number(row.TABLE_ROWS); + // TABLE_ROWS is a BIGINT that node-mysql hands back as a string or a + // double; anything non-finite means "no estimate", not zero rows. + if (row.TABLE_NAME in result && Number.isFinite(value) && value >= 0) + result[row.TABLE_NAME] = Math.floor(value); + } + } catch { + // No estimate available: every caller falls back to the exact count. + } + return result; +} + +/** + * Count rows in a table, preferring the cheap estimate. + * + * `estimate` names the table to ask the engine about, `exact` is the original + * query, kept as the fallback. + */ +async function countTable( + estimate: string, + exact: () => Promise, +): Promise { + const estimates = await estimatedRowCounts([estimate]); + const value = estimates[estimate]; + if (value !== null && value > 0) return value; + return exact(); +} + +export function countUsers(): Promise { + return countTable(TABLE.users, async () => { + const [row] = await db.select({ total: count() }).from(User); + return row?.total ?? 0; + }); +} + +export function countRooms(): Promise { + return countTable(TABLE.rooms, async () => { + const [row] = await db.select({ total: count() }).from(Rooms); + return row?.total ?? 0; + }); +} + +export function countPhotos(): Promise { + return countTable(TABLE.photos, async () => { + const [row] = await db.select({ total: count() }).from(CameraWeb); + return row?.total ?? 0; + }); +} + +/** + * Online users is deliberately *not* estimated: it is read from an index over a + * small subset of rows, it is the one counter people watch closely, and being a + * few seconds behind looks broken rather than approximate. + */ +export async function countOnline(): Promise { + const [row] = await db + .select({ total: count() }) + .from(User) + .where(eq(User.online, "1")); + return row?.total ?? 0; +}