mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-08 20:42:43 +00:00
fix(rollups): register read-path warm-ups with the background registry
The mood rollup warm-up on the dashboard summary was started with a bare `void`, so `settleBackgroundTasks()` never saw it. On a stale account the warm-up started a full recompute that could still be writing when the statement counter closed its window, and the hot-route budget read 30 against 28 depending on where it landed. Every read-path start of that shape now goes through `trackBackgroundTask(runDetached(...))`: the measurement and mood rollup warm-ups (seven call sites), the dashboard's today-briefing warm, and the two delegated-access bookkeeping writes in the record guard. A guard refuses a bare `void` on any of them. `trackBackgroundTask` now wraps its handle in `Promise.resolve`: a test double that returns nothing used to leave `undefined` in the set, and settle spun on it at full CPU. The budget fixture mints the mood rollups in its seed, as it already did for the measurement rollups. Eight serial runs of query-budget-hot-routes read the same counts every time (dashboard summary 28, snapshot 99, day 47, health-status 59, correlations 43).
This commit is contained in:
1 parent
3984ceb01c
commit
da2cd62876
14 files changed
+148
-21
No files matched your search
@@ -142,6 +142,14 @@ describe("background-task settle contract", () => {
|
||||
expect(pendingBackgroundTaskCount()).toBe(0);
|
||||
});
|
||||
|
||||
it("drains a handle that is not a promise instead of spinning on it", async () => {
|
||||
// A mocked background starter that returns nothing hands the registry
|
||||
// `undefined`; left in the set, settle would loop on it forever.
|
||||
trackBackgroundTask(undefined as unknown as Promise<unknown>);
|
||||
await settleBackgroundTasks();
|
||||
expect(pendingBackgroundTaskCount()).toBe(0);
|
||||
});
|
||||
|
||||
it("registers what fireAndForget starts", async () => {
|
||||
let release: () => void = () => {};
|
||||
fireAndForget(
|
||||
|
||||
@@ -91,3 +91,56 @@ describe("detached work cannot rename the request's event", () => {
|
||||
expect(startsDetached(detached)).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* Read-path work a request starts and never awaits. Each of these runs real
|
||||
* statements against Postgres after the response is already on its way, so a
|
||||
* bare `void` hides it from `settleBackgroundTasks()`: the statement counter
|
||||
* then closes its window while a stale account's rollup recompute is still
|
||||
* writing, and the next test inherits the tail. The dashboard summary budget
|
||||
* read 30 against 28 that way, depending on which side of the window a full
|
||||
* mood recompute landed.
|
||||
*/
|
||||
const READ_PATH_BACKGROUND_STARTERS = [
|
||||
"ensureUserRollupsFresh",
|
||||
"ensureUserMoodRollupsFresh",
|
||||
"requestTodayBriefingWarm",
|
||||
"touchGrantUsage",
|
||||
"recordDelegatedAccess",
|
||||
] as const;
|
||||
|
||||
describe("read-path background work is registered", () => {
|
||||
const files = readSourceFolder("src", {
|
||||
floor: 3000,
|
||||
include: (rel) =>
|
||||
!rel.startsWith("generated/") &&
|
||||
!rel.includes("__tests__/") &&
|
||||
!/\.test\.tsx?$/.test(rel),
|
||||
});
|
||||
const bare = new RegExp(
|
||||
String.raw`\bvoid\s+(${READ_PATH_BACKGROUND_STARTERS.join("|")})\s*\(`,
|
||||
);
|
||||
|
||||
it("starts none of them with a bare void", () => {
|
||||
const offenders = files.flatMap(([rel, text]) =>
|
||||
stripComments(text)
|
||||
.split("\n")
|
||||
.flatMap((line, i) => (bare.test(line) ? [`src/${rel}:${i + 1}`] : [])),
|
||||
);
|
||||
expect(
|
||||
offenders,
|
||||
"wrap the call in trackBackgroundTask(runDetached(() => ...)) or " +
|
||||
"fireAndForget(...) so the suite can await it",
|
||||
).toEqual([]);
|
||||
});
|
||||
|
||||
it("still finds each starter at a detached call site", () => {
|
||||
const joined = files.map(([, text]) => stripComments(text)).join("\n");
|
||||
for (const name of READ_PATH_BACKGROUND_STARTERS) {
|
||||
expect(
|
||||
new RegExp(String.raw`=>\s*${name}\s*\(`).test(joined),
|
||||
`${name} is no longer started detached anywhere; drop it from the list`,
|
||||
).toBe(true);
|
||||
}
|
||||
});
|
||||
});
|
||||
@@ -31,7 +31,14 @@ vi.mock("@/lib/rollups/mood-rollups", async () => {
|
||||
|
||||
import { buildMoodDailySeries } from "../mood-series";
|
||||
import { prisma } from "@/lib/db";
|
||||
import { readMoodDayRollups } from "@/lib/rollups/mood-rollups";
|
||||
import {
|
||||
ensureUserMoodRollupsFresh,
|
||||
readMoodDayRollups,
|
||||
} from "@/lib/rollups/mood-rollups";
|
||||
import {
|
||||
pendingBackgroundTaskCount,
|
||||
settleBackgroundTasks,
|
||||
} from "@/lib/logging/background-tasks";
|
||||
|
||||
beforeEach(() => {
|
||||
vi.mocked(prisma.moodEntry.findMany).mockResolvedValue([] as never);
|
||||
@@ -104,4 +111,26 @@ describe("buildMoodDailySeries", () => {
|
||||
expect(series.source).toBe("rollup");
|
||||
expect(series.summary.count).toBe(0);
|
||||
});
|
||||
|
||||
it("files the rollup warm-up with the background registry", async () => {
|
||||
// The warm-up is not awaited by the read, so a caller that has to know
|
||||
// when the request's work is over (the statement counter, the suite's
|
||||
// settle hook) can only see it through the registry. Left unregistered,
|
||||
// a stale account's full recompute bled into whatever ran next.
|
||||
let release: () => void = () => {};
|
||||
vi.mocked(ensureUserMoodRollupsFresh).mockImplementationOnce(
|
||||
() =>
|
||||
new Promise((resolve) => {
|
||||
release = () => resolve({ recomputed: true });
|
||||
}),
|
||||
);
|
||||
|
||||
await buildMoodDailySeries("user-1");
|
||||
|
||||
expect(ensureUserMoodRollupsFresh).toHaveBeenCalledWith("user-1");
|
||||
expect(pendingBackgroundTaskCount()).toBe(1);
|
||||
release();
|
||||
await settleBackgroundTasks();
|
||||
expect(pendingBackgroundTaskCount()).toBe(0);
|
||||
});
|
||||
});
|
||||
@@ -12,6 +12,8 @@
|
||||
*/
|
||||
import type { PrismaClient } from "@/generated/prisma/client";
|
||||
import { prisma } from "@/lib/db";
|
||||
import { runDetached } from "@/lib/logging/detached";
|
||||
import { trackBackgroundTask } from "@/lib/logging/background-tasks";
|
||||
import { summarize, type DataPoint } from "@/lib/analytics/trends";
|
||||
import {
|
||||
ensureUserMoodRollupsFresh,
|
||||
@@ -94,7 +96,9 @@ export async function buildMoodDailySeries(
|
||||
): Promise<MoodDailySeries> {
|
||||
// Fire-and-forget warm-up so the next cold mount for this user lands on
|
||||
// the rollup tier even when the boot-time backfill hasn't reached them.
|
||||
void ensureUserMoodRollupsFresh(userId);
|
||||
// Registered so a caller that must know when the request's work is over
|
||||
// (the suite's settle hook, the statement counter) can await it.
|
||||
trackBackgroundTask(runDetached(() => ensureUserMoodRollupsFresh(userId)));
|
||||
|
||||
const since = new Date(Date.now() - MOOD_SERIES_WINDOW_MS);
|
||||
const rollups = await readMoodDayRollups(userId, since);
|
||||
|
||||
@@ -5,6 +5,8 @@
|
||||
* the sleep stages. The stages run in the order the route always ran them.
|
||||
*/
|
||||
import type { SessionUser } from "@/lib/auth/session-user";
|
||||
import { runDetached } from "@/lib/logging/detached";
|
||||
import { trackBackgroundTask } from "@/lib/logging/background-tasks";
|
||||
import { annotate } from "@/lib/logging/context";
|
||||
import type { Locale } from "@/lib/i18n/config";
|
||||
import { DEFAULT_TIMEZONE } from "@/lib/tz/resolver";
|
||||
@@ -42,7 +44,7 @@ export async function buildAnalyticsResponse(
|
||||
// measurement lands, and the next request returns the up-to-date
|
||||
// value once the background refresh completes. `ensureUserRollupsFresh`
|
||||
// wraps its own try/catch, so the void call cannot reject.
|
||||
void ensureUserRollupsFresh(user.id);
|
||||
trackBackgroundTask(runDetached(() => ensureUserRollupsFresh(user.id)));
|
||||
|
||||
// Single per-type coverage probe shared by the
|
||||
// bp_in_target / healthScore / correlations branches below. The
|
||||
|
||||
@@ -59,6 +59,8 @@ import pLimit from "p-limit";
|
||||
import type { MeasurementType } from "@/generated/prisma/client";
|
||||
|
||||
import { prisma } from "@/lib/db";
|
||||
import { runDetached } from "@/lib/logging/detached";
|
||||
import { trackBackgroundTask } from "@/lib/logging/background-tasks";
|
||||
import {
|
||||
HOURLY_MEAN_DAY_TYPES,
|
||||
usesHourlyMeanDay,
|
||||
@@ -284,7 +286,7 @@ export async function computeSummariesSlice(
|
||||
// coverage probe falls back to live SQL for uncovered types, so
|
||||
// correctness is preserved; the read just doesn't block waiting
|
||||
// for the background refresh.
|
||||
void ensureUserRollupsFresh(userId);
|
||||
trackBackgroundTask(runDetached(() => ensureUserRollupsFresh(userId)));
|
||||
|
||||
// v1.4.36 — per-type coverage probe replaces the legacy global
|
||||
// COUNT. The previous gate returned true as soon as ANY type had at
|
||||
|
||||
@@ -1,5 +1,7 @@
|
||||
import type { AccountGrant } from "@/generated/prisma/client";
|
||||
import { prisma } from "@/lib/db";
|
||||
import { runDetached } from "@/lib/logging/detached";
|
||||
import { trackBackgroundTask } from "@/lib/logging/background-tasks";
|
||||
import { SESSION_USER_SELECT, type SessionUser } from "@/lib/auth/session-user";
|
||||
import { annotate, getEvent } from "@/lib/logging/context";
|
||||
import { recordDelegatedAccess } from "@/lib/auth/audit";
|
||||
@@ -372,13 +374,15 @@ async function resolveSwitchedRecord(
|
||||
});
|
||||
// Fire-and-forget, the `ApiToken.lastUsedAt` posture: the read must not wait
|
||||
// on the bookkeeping, and the bookkeeping failing must not fail the read.
|
||||
void touchGrantUsage(grant.id);
|
||||
trackBackgroundTask(runDetached(() => touchGrantUsage(grant.id)));
|
||||
// The owner's copy of the same fact, and the one they can read: a day-
|
||||
// coalesced row on their own record saying who opened it. It lands here
|
||||
// rather than in the handlers because a route cannot forget what it never
|
||||
// had to remember — every delegated request in the product passes through
|
||||
// this line, and only requests that got past the grant check reach it.
|
||||
void recordDelegatedAccess(owner.id, auth.user.id);
|
||||
trackBackgroundTask(
|
||||
runDetached(() => recordDelegatedAccess(owner.id, auth.user.id)),
|
||||
);
|
||||
|
||||
return {
|
||||
session: auth.session,
|
||||
|
||||
@@ -24,6 +24,8 @@
|
||||
import type { SessionUser } from "@/lib/auth/session-user";
|
||||
|
||||
import { prisma } from "@/lib/db";
|
||||
import { runDetached } from "@/lib/logging/detached";
|
||||
import { trackBackgroundTask } from "@/lib/logging/background-tasks";
|
||||
import { annotate } from "@/lib/logging/context";
|
||||
import { cachedSwr, caches, type ServerCache } from "@/lib/cache/server-cache";
|
||||
import { dashboardSnapshotCacheKey } from "@/lib/cache/invalidate";
|
||||
@@ -201,14 +203,18 @@ export async function readDashboardSnapshotCached(
|
||||
// its claim, never thrown.
|
||||
if (briefingAi.available) {
|
||||
const lastSeen = cached.tiles?.lastSeenByType ?? {};
|
||||
void requestTodayBriefingWarm({
|
||||
userId: user.id,
|
||||
locale,
|
||||
timezone: user.timezone,
|
||||
generatedAt,
|
||||
lastSeenAt: (type) => lastSeen[type]?.lastSeenAt ?? null,
|
||||
now,
|
||||
}).catch(() => undefined);
|
||||
trackBackgroundTask(
|
||||
runDetached(() =>
|
||||
requestTodayBriefingWarm({
|
||||
userId: user.id,
|
||||
locale,
|
||||
timezone: user.timezone,
|
||||
generatedAt,
|
||||
lastSeenAt: (type) => lastSeen[type]?.lastSeenAt ?? null,
|
||||
now,
|
||||
}),
|
||||
).catch(() => undefined),
|
||||
);
|
||||
}
|
||||
|
||||
return { body, locale };
|
||||
|
||||
@@ -98,6 +98,8 @@
|
||||
* shape `summarize()` would return.
|
||||
*/
|
||||
import { prisma } from "@/lib/db";
|
||||
import { runDetached } from "@/lib/logging/detached";
|
||||
import { trackBackgroundTask } from "@/lib/logging/background-tasks";
|
||||
import type { DataSummary } from "@/lib/analytics/trends";
|
||||
import {
|
||||
liveTrend,
|
||||
@@ -261,7 +263,7 @@ export async function buildComprehensiveAggregate(
|
||||
// coverage probe falls back to live SQL for uncovered types, so
|
||||
// correctness is preserved; the read just doesn't block waiting
|
||||
// for the background refresh.
|
||||
void ensureUserRollupsFresh(userId);
|
||||
trackBackgroundTask(runDetached(() => ensureUserRollupsFresh(userId)));
|
||||
|
||||
// v1.4.36 — per-type coverage probe. The legacy global COUNT
|
||||
// returned true as soon as any one type had a DAY bucket, which made
|
||||
|
||||
@@ -22,6 +22,8 @@
|
||||
* left on it deliberately (see their call sites in `build.ts`).
|
||||
*/
|
||||
import { prisma } from "@/lib/db";
|
||||
import { runDetached } from "@/lib/logging/detached";
|
||||
import { trackBackgroundTask } from "@/lib/logging/background-tasks";
|
||||
import type { DataPoint } from "@/lib/analytics/trends";
|
||||
import {
|
||||
ensureUserMoodRollupsFresh,
|
||||
@@ -41,7 +43,7 @@ export async function readOverviewMoodSeries(
|
||||
userId: string,
|
||||
since: Date,
|
||||
): Promise<OverviewMoodSeries> {
|
||||
void ensureUserMoodRollupsFresh(userId);
|
||||
trackBackgroundTask(runDetached(() => ensureUserMoodRollupsFresh(userId)));
|
||||
const moodRollupDayRows = await readMoodDayRollups(userId, since);
|
||||
if (moodRollupDayRows.length > 0) {
|
||||
return {
|
||||
|
||||
@@ -4,6 +4,8 @@
|
||||
* Split out of `features.ts`, which builds the block from them.
|
||||
*/
|
||||
import { prisma } from "@/lib/db";
|
||||
import { runDetached } from "@/lib/logging/detached";
|
||||
import { trackBackgroundTask } from "@/lib/logging/background-tasks";
|
||||
import {
|
||||
ensureUserMoodRollupsFresh,
|
||||
readMoodDayRollups,
|
||||
@@ -39,7 +41,7 @@ export async function readMoodDailyPoints(
|
||||
totalEntries: number;
|
||||
latestScore: number | null;
|
||||
}> {
|
||||
void ensureUserMoodRollupsFresh(userId);
|
||||
trackBackgroundTask(runDetached(() => ensureUserMoodRollupsFresh(userId)));
|
||||
// Five-year window mirrors the mood-rollup writer default; covers
|
||||
// every realistic user history span without an unbounded scan.
|
||||
const fiveYearsAgo = new Date(Date.now() - 5 * 365 * 24 * 60 * 60 * 1000);
|
||||
|
||||
@@ -56,9 +56,14 @@ const inFlight = new Set<Promise<unknown>>();
|
||||
export function trackBackgroundTask(settled: Promise<unknown>): void {
|
||||
if (!TRACK_IN_FLIGHT) return;
|
||||
|
||||
inFlight.add(settled);
|
||||
void settled.finally(() => {
|
||||
inFlight.delete(settled);
|
||||
// A test double that stands in for the background work often returns
|
||||
// nothing at all. Filed as is, `undefined` would never leave the set and
|
||||
// `settleBackgroundTasks` would spin on it forever at full CPU. A real
|
||||
// promise passes through `Promise.resolve` as the same object.
|
||||
const handle = Promise.resolve(settled);
|
||||
inFlight.add(handle);
|
||||
void handle.finally(() => {
|
||||
inFlight.delete(handle);
|
||||
});
|
||||
}
|
||||
|
||||
|
||||
@@ -11,6 +11,8 @@
|
||||
* are read in the user's own calendar.
|
||||
*/
|
||||
import { prisma } from "@/lib/db";
|
||||
import { runDetached } from "@/lib/logging/detached";
|
||||
import { trackBackgroundTask } from "@/lib/logging/background-tasks";
|
||||
import { annotate } from "@/lib/logging/context";
|
||||
import {
|
||||
COURSES_COMPLIANCE_SELECT,
|
||||
@@ -102,7 +104,7 @@ export async function buildTargetsResponse(user: AuthedUser) {
|
||||
queryNow.getTime() - 5 * 365 * 24 * 60 * 60 * 1000,
|
||||
);
|
||||
|
||||
void ensureUserMoodRollupsFresh(userId);
|
||||
trackBackgroundTask(runDetached(() => ensureUserMoodRollupsFresh(userId)));
|
||||
|
||||
// All independent reads stay in one bounded fan-out. The medication event
|
||||
// read is the sole dependent query and remains chained to its medication
|
||||
|
||||
@@ -281,9 +281,15 @@ async function seed(): Promise<void> {
|
||||
}),
|
||||
});
|
||||
|
||||
// Both rollup tiers are minted here, the way a running account has them.
|
||||
// Seeded rows without their rollups make the first read start a full
|
||||
// recompute in the background, and that is a warm-up cost, not the read's.
|
||||
const { recomputeUserRollups } =
|
||||
await import("@/lib/rollups/measurement-rollups");
|
||||
await recomputeUserRollups(USER_ID);
|
||||
const { recomputeUserMoodRollups } =
|
||||
await import("@/lib/rollups/mood-rollups");
|
||||
await recomputeUserMoodRollups(USER_ID);
|
||||
|
||||
const session = await db.session.create({
|
||||
data: { userId: USER_ID, expiresAt: new Date(Date.now() + 3_600_000) },
|
||||
|
||||
Reference in new issue
Block a user