From a05a54afeb88f46cb41d88663c171f327a1b9583 Mon Sep 17 00:00:00 2001 From: Anthony Ettinger Date: Fri, 21 Aug 2026 15:46:57 +0000 Subject: [PATCH] Compute the slow breakdown where nothing is waiting for it /api/crawlstats came down from 118s to 20.2s, and then sat there exactly, on every request. The remaining 20 seconds were `categoryStats` hitting the timeout, failing, caching nothing, and being asked again by the next reader. Decomposed against production, the reason it cannot be made cheap: feeds per category 6,340ms index-only growth by day+category 1,046ms index-only status x category 23,942ms sum(item_count) per category 35,440ms crawled-in-last-day 40,020ms The slow three read columns no index covers, so they fetch every one of 476,715 rows. An index covering them would have to carry `status` and `last_success_at`, both rewritten on every crawl -- buying a fast chart with a slower crawler, when writes are the one thing this system has none of to spare. The other half is that the 30-second ceiling was ours. It is `TURSO_REQUEST_TIMEOUT_MS`, not a limit of the database, and given a longer deadline the whole statement completes in 58.9 seconds. So the query is unchanged and simply stops being on the request path. The poller recomputes it every five minutes on a connection with a patient deadline and primes the same Redis key the web service reads, so a reader finds it already there. It is a read, and reads do not queue behind the single writer, so this costs the crawler nothing. `primeCache` goes through the same envelope `remember` writes, so the warmer and the reader cannot drift on the format, and it is a no-op without REDIS_URL -- which is the local and test case, where the poller simply does not warm. Co-Authored-By: Claude Opus 5 (1M context) --- apps/poller/src/index.js | 54 +++++++++++++++++++- packages/db/index.js | 3 +- packages/db/src/cache.js | 23 +++++++++ packages/db/src/client.js | 12 ++++- packages/db/src/statsWarmer.js | 90 ++++++++++++++++++++++++++++++++++ packages/db/test/cache.test.js | 25 +++++++++- 6 files changed, 202 insertions(+), 5 deletions(-) create mode 100644 packages/db/src/statsWarmer.js diff --git a/apps/poller/src/index.js b/apps/poller/src/index.js index 9d51d3b..988b60f 100644 --- a/apps/poller/src/index.js +++ b/apps/poller/src/index.js @@ -1,4 +1,13 @@ -import { connect, createWriteWorker, migrate, q, accounts, alerts, takeWriteTally } from '@rssamplifier/db'; +import { + connect, + createWriteWorker, + migrate, + q, + accounts, + alerts, + takeWriteTally, + warmStatsCache, +} from '@rssamplifier/db'; import { crawlDue, enrichDue, @@ -135,6 +144,16 @@ const authorIntervalMs = (Number(env['AUTHOR_INTERVAL_SECONDS']) || 60) * 1000; // minutes stale on a page that refreshes every fifteen seconds. const queueSampleMs = (Number(env['QUEUE_SAMPLE_SECONDS']) || 600) * 1000; +// How often the category breakdown is recomputed into Redis for /crawlstats. +// +// It is a ~59-second read of every feed, which is why it cannot live on a +// request: with a 30-second deadline in front of it, it could only ever fail, +// so the cache it was supposed to fill stayed empty and every visitor paid the +// full timeout. Five minutes is far more often than the number moves -- it is +// the shape of a directory of half a million feeds -- and the point is only +// that Redis is never empty, not that it is current to the second. +const statsWarmMs = (Number(env['STATS_WARM_SECONDS']) || 300) * 1000; + // Where the write queue lives. Absent, `connect()` falls back to the // in-process serialiser and this daemon starts no worker — the system as it // shipped before the queue existed, which is the right thing to degrade to. @@ -831,6 +850,34 @@ async function queueTick() { } } +/** Guards against a warm that outruns its interval starting a second one. */ +let warming = false; + +/** + * Recompute the category breakdown into Redis, so no page has to. + * + * This read takes about a minute against half a million feeds and cannot be + * made cheap: the columns it groups by are rewritten on every crawl, so an + * index covering them would be paid for on the write path, which is the one + * thing this system has none of to spare. Doing it here instead costs the + * crawler nothing -- it is a read, and reads do not queue behind the single + * writer everything else contends for. + * + * `warmStatsCache` never throws; the result is logged rather than acted on, + * because there is nothing to do about a warm that did not happen except serve + * the previous answer, which is what the cache already does. + */ +async function statsTick() { + if (warming) return; + warming = true; + try { + const result = await warmStatsCache({ log }); + if (!result.ok) return; + } finally { + warming = false; + } +} + /** * The one process that actually writes to Turso. * @@ -881,6 +928,7 @@ const cardTimer = setInterval(cardTick, cardIntervalMs); const alertTimer = setInterval(alertTick, alertIntervalMs); const enrichTimer = setInterval(enrichTick, authorIntervalMs); const queueTimer = setInterval(queueTick, queueSampleMs); +const statsTimer = setInterval(statsTick, statsWarmMs); const searchTimer = setInterval(searchTick, searchIntervalMs); // Same cadence as the queue sample: long enough that the line is a summary // rather than a stream, short enough to bracket an experiment against. @@ -891,6 +939,9 @@ void cardTick(); void alertTick(); void enrichTick(); void queueTick(); +// Run once at boot, so a deploy does not leave the page slow until the first +// interval comes round. +void statsTick(); log('started', { intervalSeconds: intervalMs / 1000, @@ -922,6 +973,7 @@ function shutdown(signal) { clearInterval(enrichTimer); clearInterval(alertTimer); clearInterval(queueTimer); + clearInterval(statsTimer); clearInterval(searchTimer); clearInterval(tallyTimer); diff --git a/packages/db/index.js b/packages/db/index.js index 408ccde..fc62a95 100644 --- a/packages/db/index.js +++ b/packages/db/index.js @@ -1,6 +1,7 @@ export { connect, newId, nowIso } from './src/client.js'; export { createWriteWorker, WRITE_QUEUE, takeWriteTally } from './src/writeQueue.js'; -export { remember, redisClient, resetCacheState } from './src/cache.js'; +export { remember, redisClient, primeCache, resetCacheState } from './src/cache.js'; +export { warmStatsCache } from './src/statsWarmer.js'; export { migrate } from './src/migrate.js'; export * as q from './src/queries.js'; export * as accounts from './src/accounts.js'; diff --git a/packages/db/src/cache.js b/packages/db/src/cache.js index 554126a..97df9a9 100644 --- a/packages/db/src/cache.js +++ b/packages/db/src/cache.js @@ -268,6 +268,29 @@ function cacheKey(key) { return `rsa:stats:${key}`; } +/** + * Store a value under `key` as though a reader had just computed it. + * + * For work that cannot be done on a request: the category breakdown takes ~59 + * seconds, so no page can wait for it and a cache that only fills from readers + * never fills at all. A background job computes it on a patient connection and + * primes the same key, and every reader is then served from Redis. + * + * Goes through `writeEntry` rather than reimplementing the envelope, so the + * warmer and the reader cannot drift apart on the format. + * + * @param {string} key + * @param {unknown} value + * @param {{ client?: any, maxStaleMs?: number }} [opts] + * @returns {Promise} whether it was stored + */ +export async function primeCache(key, value, opts = {}) { + const client = opts.client !== undefined ? opts.client : await redisClient(); + if (!client) return false; + await writeEntry(client, key, value, opts.maxStaleMs ?? 24 * 60 * 60 * 1000); + return true; +} + /** Test seam: forget the in-flight refreshes between cases. */ export function resetCacheState() { inFlight.clear(); diff --git a/packages/db/src/client.js b/packages/db/src/client.js index 584ec00..01fb46a 100644 --- a/packages/db/src/client.js +++ b/packages/db/src/client.js @@ -15,7 +15,13 @@ import { queueWrites } from './writeQueue.js'; * A `file:` URL needs no auth token, which is what makes local development and * the test suite work without a Turso account. * - * @param {{ url?: string, authToken?: string, redisUrl?: string, queue?: boolean }} [opts] + * `timeoutMs` overrides the per-request deadline for this connection alone. + * The default is right for anything serving a page, and wrong for the one + * background job that recomputes the category breakdown: that read takes ~59 + * seconds against half a million feeds, so on the default it can only ever + * fail. See `warmStatsCache` in ./statsWarmer.js. + * + * @param {{ url?: string, authToken?: string, redisUrl?: string, queue?: boolean, timeoutMs?: number }} [opts] * @returns {import('@libsql/client').Client} */ export function connect(opts = {}) { @@ -26,7 +32,9 @@ export function connect(opts = {}) { if (!url) throw new Error('TURSO_DATABASE_URL must be set'); const client = createClient( - url.startsWith('file:') ? { url } : { url, authToken, fetch: withTimeout(requestTimeoutMs()) }, + url.startsWith('file:') + ? { url } + : { url, authToken, fetch: withTimeout(opts.timeoutMs ?? requestTimeoutMs()) }, ); // Redis moves the write queue out of the process, which is the only way to diff --git a/packages/db/src/statsWarmer.js b/packages/db/src/statsWarmer.js new file mode 100644 index 0000000..597bc12 --- /dev/null +++ b/packages/db/src/statsWarmer.js @@ -0,0 +1,90 @@ +import { connect } from './client.js'; +import { primeCache } from './cache.js'; +import * as q from './queries.js'; + +/** + * Recompute the numbers no page can afford to wait for, and put them in Redis. + * + * ## Why a background job and not a faster query + * + * `categoryStats` groups half a million feeds by category while reading their + * status, item count and last-success time. Measured against production on + * 2026-08-21, decomposed piece by piece: + * + * feeds per category 6,340ms (index-only, uses the partial index) + * growth by day+category 1,046ms (index-only) + * status x category 23,942ms + * sum(item_count) per category 35,440ms + * crawled-in-last-day 40,020ms + * + * The three slow ones read columns no index covers, so they fetch every row. + * An index that covered them would have to carry `status` and `last_success_at`, + * both rewritten on every crawl — and writes, not reads, are this database's + * binding constraint. Buying a fast breakdown with a slower crawler is the + * wrong trade. + * + * So the query stays as it is and stops being on the request path. The whole + * statement completes in **58.9 seconds** given a deadline long enough to allow + * it, which is the other half of this: the 30s ceiling it kept hitting is this + * codebase's own `TURSO_REQUEST_TIMEOUT_MS`, not a limit of the database. A + * connection with a patient deadline can finish what a page never could. + * + * ## Why this is safe to run beside the crawler + * + * It is a read, and reads bypass the write serialization entirely — they do not + * take the single writer that everything else queues behind. One long read every + * few minutes costs the crawler nothing. + * + * ## Why it primes rather than returns + * + * Nothing here consumes the value. The point is that `remember('categoryStats')` + * on the web side finds it already there, so the reader that would have waited + * 20 seconds and then given up is served from Redis instead. + */ + +/** Long enough for the ~59s read, with room for a bad day. */ +const WARM_TIMEOUT_MS = 150_000; + +/** How much growth history the breakdown carries; matches the web's GROWTH_DAYS. */ +const GROWTH_DAYS = 30; + +/** + * Compute the category breakdown on a patient connection and cache it. + * + * Never throws. A warmer that takes the poller down with it would trade a slow + * chart for a stopped crawler, and the cache simply keeps serving whatever it + * last had. + * + * @param {{ log?: (event: string, fields?: object) => void, client?: any }} [opts] + * @returns {Promise<{ ok: boolean, ms: number, cached: boolean, error?: string }>} + */ +export async function warmStatsCache(opts = {}) { + const started = Date.now(); + const log = opts.log ?? (() => {}); + + /** @type {import('@libsql/client').Client|null} */ + let db = null; + try { + // Its own connection, deliberately. Sharing the poller's would either impose + // this job's 150-second deadline on every crawl write or leave this job with + // the 30 seconds it cannot finish in. + db = connect({ timeoutMs: WARM_TIMEOUT_MS, queue: false }); + + const value = await q.categoryStats(db, GROWTH_DAYS); + const cached = await primeCache('categoryStats', value, { client: opts.client }); + + const ms = Date.now() - started; + log('stats-warmed', { ms, categories: value?.categories?.length ?? 0, cached }); + return { ok: true, ms, cached }; + } catch (error) { + const ms = Date.now() - started; + // Logged as `stats-warm-skipped`, not `…-error`: the page has a cached + // answer and is fine, and the operational-error panel treats any event + // whose name ends in "error" as an alarm. A missed warm is not one. + const reason = error instanceof Error ? error.message : String(error); + log('stats-warm-skipped', { ms, reason }); + return { ok: false, ms, cached: false, error: reason }; + } finally { + try { db?.close(); } catch { /* closing a broken client is not a failure */ } + } +} diff --git a/packages/db/test/cache.test.js b/packages/db/test/cache.test.js index cfd900a..1dec893 100644 --- a/packages/db/test/cache.test.js +++ b/packages/db/test/cache.test.js @@ -1,7 +1,7 @@ import assert from 'node:assert/strict'; import { test, beforeEach } from 'node:test'; -import { remember, resetCacheState } from '../src/cache.js'; +import { remember, primeCache, resetCacheState } from '../src/cache.js'; /** * A Redis stand-in. @@ -196,6 +196,29 @@ test('a failed computation with nothing cached returns the fallback, not a throw assert.deepEqual(got, {}, 'callers treat this as "unavailable", so it must not throw'); }); +test('a primed value is served to readers without them computing anything', async () => { + // The warmer's whole contract. `categoryStats` takes ~59s, so no reader can + // fill this key; a background job fills it and readers must find it there. + const client = fakeRedis(); + + const stored = await primeCache('categoryStats', { total: 476_715 }, { client }); + assert.equal(stored, true); + + let ran = 0; + const got = await remember('categoryStats', { ttlMs: 5 * 60_000, client }, async () => { + ran += 1; + throw new Error('a reader must never have to run this'); + }); + + assert.deepEqual(got, { total: 476_715 }); + assert.equal(ran, 0, 'the reader did not touch the database'); +}); + +test('priming without a client is a no-op rather than a crash', async () => { + // The poller runs with no REDIS_URL locally and in the test suite. + assert.equal(await primeCache('k', { a: 1 }, { client: null }), false); +}); + test('bigint counts survive the round trip', async () => { // libSQL hands back BigInt for some aggregates and JSON.stringify throws on // it, which would silently disable the cache for exactly the count-heavy