From c50007ef616547946548de5596043473c876c78e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Tue, 20 Oct 2026 10:55:09 +0200 Subject: [PATCH] fix(day): read the notable history through the fenced cache path The comparison history blocks were read with cache.get and stored with cache.set, outside the single-flight slot that an invalidation fences. A read that spanned a write cached the days from before it for the block's whole TTL, and two concurrent reads of a block both queried. Each block now goes through cache.wrap; the blocks missing at the time still share one read of their span. --- .../__tests__/notable-history-cache.test.ts | 36 ++++++++++++ src/lib/day/notable.ts | 55 ++++++++++++------- 2 files changed, 72 insertions(+), 19 deletions(-) diff --git a/src/lib/day/__tests__/notable-history-cache.test.ts b/src/lib/day/__tests__/notable-history-cache.test.ts index 256ded2e3..7fb0e9843 100644 --- a/src/lib/day/__tests__/notable-history-cache.test.ts +++ b/src/lib/day/__tests__/notable-history-cache.test.ts @@ -132,6 +132,42 @@ describe("notable history cache", () => { }); }); +describe("a write while the history is being read", () => { + it("is not hidden by the read that started before it", async () => { + // The first read is held until a write lands and invalidates the + // account's analytics entries, as `invalidateUserMeasurements` does. + const real = readLocalDailyStats.getMockImplementation()!; + let release!: () => void; + const held = new Promise((r) => (release = r)); + readLocalDailyStats.mockImplementationOnce(async (args) => { + const before = await real(args); + await held; + return before; + }); + const stale = load(SPIKE); + await new Promise((r) => setTimeout(r, 0)); + // The write lowers the spike, then evicts. + const original = real; + readLocalDailyStats.mockImplementation(async (args) => { + const stats = (await original(args)) as Map>; + stats.get("BLOOD_PRESSURE_SYS")?.set(SPIKE, 120); + return stats; + }); + caches.analytics.deleteByPrefix("user-1|"); + release(); + // The read already under way answers with what it saw. + expect((await stale).map((n) => n.kind)).toEqual(["extremeHigh"]); + // The next one reads again and sees the write. + expect(await load(SPIKE)).toEqual([]); + }); + + it("joins a read of the same block already under way", async () => { + await Promise.all([load(SPIKE), load(SPIKE)]); + // One read of the missing span, shared by both. + expect(readLocalDailyStats).toHaveBeenCalledTimes(1); + }); +}); + describe("notable history cache size", () => { const cached: number[] = []; beforeEach(() => { diff --git a/src/lib/day/notable.ts b/src/lib/day/notable.ts index 32ab02261..2fb50b3af 100644 --- a/src/lib/day/notable.ts +++ b/src/lib/day/notable.ts @@ -274,26 +274,43 @@ async function readExtremeHistory(args: { }); } - const found = new Map(); - const missing = blocks.filter((block) => { - const hit = cache.get(block.key); - if (hit) found.set(block.key, hit); - return !hit; - }); - if (missing.length > 0) { - const first = missing[0]!; - const last = missing[missing.length - 1]!; - const stats = await read( - startOfLocalDayKey(first.start, args.tz), - last.end, + // Every block goes through `wrap`, so a build is coalesced with a + // concurrent read of the same block and, above all, is fenced: a write + // that invalidates the account's analytics entries while the read is in + // flight stops the result from being cached, instead of the pre-write + // days being served for the block's whole TTL. The blocks missing right + // now still share one read of their contiguous span. + const missing = blocks.filter((block) => cache.get(block.key) === null); + let span: Promise | null = null; + const readSpan = (): Promise => { + span ??= read( + startOfLocalDayKey(missing[0]!.start, args.tz), + missing[missing.length - 1]!.end, ); - for (const block of blocks) { - if (block.start < first.start || block.end > last.end) continue; - const cell = sliceDays(stats, block.start, block.end); - cache.set(block.key, cell, HISTORY_TTL_MS); - found.set(block.key, cell); - } - } + return span; + }; + const inSpan = (block: { start: string; end: string }) => + missing.length > 0 && + block.start >= missing[0]!.start && + block.end <= missing[missing.length - 1]!.end; + const cells = await Promise.all( + blocks.map(async (block) => { + const { value } = await cache.wrap( + block.key, + async () => + sliceDays( + inSpan(block) + ? await readSpan() + : await read(startOfLocalDayKey(block.start, args.tz), block.end), + block.start, + block.end, + ), + HISTORY_TTL_MS, + ); + return [block.key, value] as const; + }), + ); + const found = new Map(cells); const out: DailyStats = new Map(); for (const block of blocks) {