From 6e8815fc6f01322e0f484ff5a193873f8ef01611 Mon Sep 17 00:00:00 2001 From: Birfy Date: Tue, 10 Nov 2026 11:34:07 +0000 Subject: [PATCH] fix(project-metrics): drop the five-minute buckets the cache can never answer again MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The cache key carries the five-minute window, so every window is a new key and nothing ever removed an old one: a board that polled kept 288 dead buckets per day for each repository and zone, for the life of the process (#2806). A write now sweeps the entries whose expiresAt has passed, and CACHE_MAX bounds a table whose writes outpace its reads because nothing sweeps in between. A hit inside the same window, METRICS_TTL_MS and the shape of the answer are unchanged, and a caller-supplied store that answers only get/set is left as it was. Tests: core/test/project-metrics-cache-bound.test.js — red before the fix (bucket 0 answered `hit` after 200 later windows; 201 entries retained) and green after it, with the two existing project-metrics suites green. Fixes #2806 Docs-not-needed: core-host.md's project-metrics paragraph still describes the same key, the same five-minute lifetime, the same hit and the same answer; this change only bounds how long an entry that can never be answered is retained. Written-by: Dev --- changelog.d/2806.fixed.md | 1 + core/src/host/project-metrics.js | 17 ++++++++++++++ core/test/project-metrics-cache-bound.test.js | 22 +++++++++++++++++++ .../ds-bot/files/src/host/project-metrics.js | 17 ++++++++++++++ .../test/project-metrics-cache-bound.test.js | 22 +++++++++++++++++++ 5 files changed, 79 insertions(+) create mode 100644 changelog.d/2806.fixed.md create mode 100644 core/test/project-metrics-cache-bound.test.js create mode 100644 patches/ds-bot/files/test/project-metrics-cache-bound.test.js diff --git a/changelog.d/2806.fixed.md b/changelog.d/2806.fixed.md new file mode 100644 index 000000000..73648bebf --- /dev/null +++ b/changelog.d/2806.fixed.md @@ -0,0 +1 @@ +The board's metrics cache no longer holds every five-minute window it has ever answered: a window that is over is dropped on the next write, and the table is capped, so a host that runs for weeks keeps a handful of entries instead of one per window per repository and zone. A hit inside the same window, the five-minute lifetime and every number on the board are unchanged. See [#2806](https://github.com/ThinkFlowLab/FlyBot/issues/2806). diff --git a/core/src/host/project-metrics.js b/core/src/host/project-metrics.js index 5926af4f8..1d7b1b5e9 100644 --- a/core/src/host/project-metrics.js +++ b/core/src/host/project-metrics.js @@ -140,6 +140,22 @@ export function runningOf(task, now) { // with it. A hit answers the same object and spends no source call, which is what the issue's // "cache hit reads the source once" case is about. const cache = new Map() +// #2806: the key carries the five-minute window, so a bucket that is over can never be answered +// again — a new key arrives every five minutes for each repository and zone, and nothing ever +// removed the old ones. Every write now drops what it can no longer answer, and the cap catches a +// table whose writes outpace its reads, because nothing sweeps in between. +const CACHE_MAX = 512 +const pruneCache = (cacheStore, now) => { + // The default store is the Map above. An injected store that keeps only `get`/`set` (a test seam) + // is left as it is rather than given a `delete` it never promised. + if (typeof cacheStore.delete !== 'function') return + for (const [key, entry] of cacheStore) if (entry.expiresAt <= now) cacheStore.delete(key) + while (cacheStore.size >= CACHE_MAX) { + const oldest = cacheStore.keys().next() + if (oldest.done) break + cacheStore.delete(oldest.value) + } +} const emptyMetrics = () => Object.fromEntries(METRIC_KEYS.map(key => [key, null])) @@ -243,6 +259,7 @@ export async function readMetrics({ repo, token, fetchImpl, timeZone = 'UTC', no generatedAt: value.generatedAt, metrics: { prsOpenedToday, prsMergedToday, issuesOpenedToday, issuesClosedToday, prMergeMedianMs, issueCloseMedianMs }, } + pruneCache(cacheStore, now) cacheStore.set(key, { expiresAt: now + ttlMs, value: repositoryValue }) return { ...withLive(value), cache: 'miss', ...(source.truncated ? { truncated: true } : {}) } } diff --git a/core/test/project-metrics-cache-bound.test.js b/core/test/project-metrics-cache-bound.test.js new file mode 100644 index 000000000..02b033c7b --- /dev/null +++ b/core/test/project-metrics-cache-bound.test.js @@ -0,0 +1,22 @@ +// #2806: the shared metrics cache is keyed by a five-minute window, so an entry can never be +// answered again once its window is over — a bucket that is done is dead weight, and the host kept +// every one of them for the life of the process (288 a day per repository and zone). +import test from 'node:test' +import assert from 'node:assert/strict' +import { METRICS_TTL_MS, readMetrics } from '../src/host/project-metrics.js' + +const NOW = Date.parse('2026-10-30T12:00:00Z') +const fetcher = async () => ({ ok: true, json: async () => [] }) + +test('a five-minute bucket that is over is dropped, not kept for the life of the host', async () => { + const store = new Map() + const read = now => readMetrics({ repo: 'o/r', token: 'tok', fetchImpl: fetcher, now, cacheStore: store }) + // Bucket 0 fills the cache, then 200 more windows pass: each one is a new key, and the read that + // fills it is the only chance this table has to drop the windows it can no longer answer. + assert.equal((await read(NOW)).cache, 'miss') + for (let bucket = 1; bucket <= 200; bucket += 1) await read(NOW + bucket * METRICS_TTL_MS) + // A bucket that is over is a miss, not a hit served out of a dead entry. Before #2806 this read + // answered `hit` from bucket 0's entry, which had been carried across 200 later windows. + assert.equal((await read(NOW)).cache, 'miss') + assert.ok(store.size <= 2, `the store keeps the live window, not 201 dead ones (kept ${store.size})`) +}) diff --git a/patches/ds-bot/files/src/host/project-metrics.js b/patches/ds-bot/files/src/host/project-metrics.js index 5926af4f8..1d7b1b5e9 100644 --- a/patches/ds-bot/files/src/host/project-metrics.js +++ b/patches/ds-bot/files/src/host/project-metrics.js @@ -140,6 +140,22 @@ export function runningOf(task, now) { // with it. A hit answers the same object and spends no source call, which is what the issue's // "cache hit reads the source once" case is about. const cache = new Map() +// #2806: the key carries the five-minute window, so a bucket that is over can never be answered +// again — a new key arrives every five minutes for each repository and zone, and nothing ever +// removed the old ones. Every write now drops what it can no longer answer, and the cap catches a +// table whose writes outpace its reads, because nothing sweeps in between. +const CACHE_MAX = 512 +const pruneCache = (cacheStore, now) => { + // The default store is the Map above. An injected store that keeps only `get`/`set` (a test seam) + // is left as it is rather than given a `delete` it never promised. + if (typeof cacheStore.delete !== 'function') return + for (const [key, entry] of cacheStore) if (entry.expiresAt <= now) cacheStore.delete(key) + while (cacheStore.size >= CACHE_MAX) { + const oldest = cacheStore.keys().next() + if (oldest.done) break + cacheStore.delete(oldest.value) + } +} const emptyMetrics = () => Object.fromEntries(METRIC_KEYS.map(key => [key, null])) @@ -243,6 +259,7 @@ export async function readMetrics({ repo, token, fetchImpl, timeZone = 'UTC', no generatedAt: value.generatedAt, metrics: { prsOpenedToday, prsMergedToday, issuesOpenedToday, issuesClosedToday, prMergeMedianMs, issueCloseMedianMs }, } + pruneCache(cacheStore, now) cacheStore.set(key, { expiresAt: now + ttlMs, value: repositoryValue }) return { ...withLive(value), cache: 'miss', ...(source.truncated ? { truncated: true } : {}) } } diff --git a/patches/ds-bot/files/test/project-metrics-cache-bound.test.js b/patches/ds-bot/files/test/project-metrics-cache-bound.test.js new file mode 100644 index 000000000..02b033c7b --- /dev/null +++ b/patches/ds-bot/files/test/project-metrics-cache-bound.test.js @@ -0,0 +1,22 @@ +// #2806: the shared metrics cache is keyed by a five-minute window, so an entry can never be +// answered again once its window is over — a bucket that is done is dead weight, and the host kept +// every one of them for the life of the process (288 a day per repository and zone). +import test from 'node:test' +import assert from 'node:assert/strict' +import { METRICS_TTL_MS, readMetrics } from '../src/host/project-metrics.js' + +const NOW = Date.parse('2026-10-30T12:00:00Z') +const fetcher = async () => ({ ok: true, json: async () => [] }) + +test('a five-minute bucket that is over is dropped, not kept for the life of the host', async () => { + const store = new Map() + const read = now => readMetrics({ repo: 'o/r', token: 'tok', fetchImpl: fetcher, now, cacheStore: store }) + // Bucket 0 fills the cache, then 200 more windows pass: each one is a new key, and the read that + // fills it is the only chance this table has to drop the windows it can no longer answer. + assert.equal((await read(NOW)).cache, 'miss') + for (let bucket = 1; bucket <= 200; bucket += 1) await read(NOW + bucket * METRICS_TTL_MS) + // A bucket that is over is a miss, not a hit served out of a dead entry. Before #2806 this read + // answered `hit` from bucket 0's entry, which had been carried across 200 later windows. + assert.equal((await read(NOW)).cache, 'miss') + assert.ok(store.size <= 2, `the store keeps the live window, not 201 dead ones (kept ${store.size})`) +})