From cfc19a211cacc4fabd8a46ccb24458b596d486cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Thu, 10 Sep 2026 10:09:59 +0200 Subject: [PATCH] fix(backups): give the off-host verdicts slack for a Berlin night that is 25 hours long The threshold was the schedule period with no budget on top of it. Two things spend that budget on a host where nothing is wrong. The cron runs 02:30 Europe/Berlin, so on the DST fall-back Sunday the gap between two runs is 25 hours and every account on every host would cross one period on the same morning. And the ledger instant is when that account's object landed, by design, so on a cohort walked one account at a time an account reached an hour later tonight than last night drifts past 24 with two clean runs behind it. The schedule period and the verdict threshold are now separate numbers. The period is still what the card quotes; the threshold is the period plus six hours, applied to both steps, so due still means one run produced nothing and stale still means two. The fall-back night is pinned in the unit file by its real dates rather than by an arithmetic 25, and a genuinely missed night at 31 hours still reads due. The runbook says why the slack is there. --- docs/ops/backup-restore.md | 8 +++ .../api/admin/backups/__tests__/route.test.ts | 2 +- .../offhost-backup-freshness.test.ts | 62 ++++++++++++++++--- src/lib/jobs/offhost-backup-freshness.ts | 37 +++++++++-- .../integration/offhost-backup-ledger.test.ts | 5 +- 5 files changed, 97 insertions(+), 17 deletions(-) diff --git a/docs/ops/backup-restore.md b/docs/ops/backup-restore.md index 12b834de3..4abb38e46 100644 --- a/docs/ops/backup-restore.md +++ b/docs/ops/backup-restore.md @@ -228,6 +228,14 @@ how that compares with the nightly schedule. **Fresh** is inside one run, did not, and **never** means no object has ever landed. On a host without `BACKUP_S3_*` the card says off-host backup is not configured and stops there. +Each verdict carries six hours of slack on top of the schedule's own 24, and +that slack is load-bearing on two nights. The cron runs at 02:30 Europe/Berlin, +so on the DST fall-back Sunday two consecutive runs are 25 hours apart; and the +timestamp is when _that account's_ object landed, so on a cohort walked one +account at a time an account reached later tonight than last night drifts by +hours. Without the slack a healthy host would paint its whole cohort **due** +once a year. + The verdicts come from a ledger the worker writes when the object lands (`offhost_backup_state`), not from a listing of the bucket. Keep the worker's grant as documented — PutObject, GetObject, AbortMultipartUpload — and the card diff --git a/src/app/api/admin/backups/__tests__/route.test.ts b/src/app/api/admin/backups/__tests__/route.test.ts index adbdd45e4..c1dcd9a11 100644 --- a/src/app/api/admin/backups/__tests__/route.test.ts +++ b/src/app/api/admin/backups/__tests__/route.test.ts @@ -73,7 +73,7 @@ describe("GET /api/admin/backups — off-host freshness", () => { id: "u2", username: "account-two", offhostBackupState: { - lastSuccessAt: new Date(now - 30 * HOUR), + lastSuccessAt: new Date(now - 31 * HOUR), sizeBytes: BigInt(8192), }, }, diff --git a/src/lib/jobs/__tests__/offhost-backup-freshness.test.ts b/src/lib/jobs/__tests__/offhost-backup-freshness.test.ts index f364d9e69..77be636b1 100644 --- a/src/lib/jobs/__tests__/offhost-backup-freshness.test.ts +++ b/src/lib/jobs/__tests__/offhost-backup-freshness.test.ts @@ -1,12 +1,14 @@ import { describe, expect, it } from "vitest"; import { classifyOffhostBackup, + OFFHOST_BACKUP_GRACE_HOURS, OFFHOST_BACKUP_PERIOD_HOURS, } from "@/lib/jobs/offhost-backup-freshness"; const NOW = new Date("2026-09-10T09:00:00.000Z"); const HOUR = 3_600_000; const PERIOD = OFFHOST_BACKUP_PERIOD_HOURS * HOUR; +const GRACE = OFFHOST_BACKUP_GRACE_HOURS * HOUR; /** `age` hours before NOW. */ function ago(ms: number): Date { @@ -21,27 +23,64 @@ describe("classifyOffhostBackup", () => { }); }); - it("is fresh right up to the one-period boundary and due one millisecond past it", () => { + it("is fresh right up to one period plus the grace and due one millisecond past it", () => { expect( - classifyOffhostBackup({ lastSuccessAt: ago(PERIOD), now: NOW }).freshness, + classifyOffhostBackup({ lastSuccessAt: ago(PERIOD + GRACE), now: NOW }) + .freshness, ).toBe("fresh"); expect( - classifyOffhostBackup({ lastSuccessAt: ago(PERIOD + 1), now: NOW }) - .freshness, + classifyOffhostBackup({ + lastSuccessAt: ago(PERIOD + GRACE + 1), + now: NOW, + }).freshness, ).toBe("due"); }); - it("is due right up to the two-period boundary and stale one millisecond past it", () => { + it("is due right up to two periods plus the grace and stale one millisecond past it", () => { expect( - classifyOffhostBackup({ lastSuccessAt: ago(PERIOD * 2), now: NOW }) - .freshness, + classifyOffhostBackup({ + lastSuccessAt: ago(PERIOD * 2 + GRACE), + now: NOW, + }).freshness, ).toBe("due"); expect( - classifyOffhostBackup({ lastSuccessAt: ago(PERIOD * 2 + 1), now: NOW }) - .freshness, + classifyOffhostBackup({ + lastSuccessAt: ago(PERIOD * 2 + GRACE + 1), + now: NOW, + }).freshness, ).toBe("stale"); }); + it("keeps the whole cohort fresh across the Berlin DST fall-back night", () => { + // 2026-10-25 is the fall-back Sunday in Europe/Berlin: the clocks go back + // at 03:00 CEST, so 02:30 local happens 25 hours after the previous + // 02:30 local. Both instants are pinned in UTC because that is what the + // ledger stores, and the test must not depend on the runner's own zone. + const beforeTheChange = new Date("2026-10-24T00:30:00.000Z"); // 02:30 CEST + const afterTheChange = new Date("2026-10-25T01:30:00.000Z"); // 02:30 CET + const gapHours = + (afterTheChange.getTime() - beforeTheChange.getTime()) / HOUR; + expect(gapHours).toBe(25); + + // Without slack the whole cohort would read `due` on that one morning, + // on every host, with nothing wrong anywhere. + expect( + classifyOffhostBackup({ + lastSuccessAt: beforeTheChange, + now: afterTheChange, + }).freshness, + ).toBe("fresh"); + }); + + it("still calls a genuinely missed night due", () => { + // 31 hours: past one period even with six hours of slack, so a run that + // produced nothing for this account still shows. + expect( + classifyOffhostBackup({ lastSuccessAt: ago(31 * HOUR), now: NOW }) + .freshness, + ).toBe("due"); + }); + it("reports whole hours of age", () => { expect( classifyOffhostBackup({ lastSuccessAt: ago(90 * 60_000), now: NOW }), @@ -50,7 +89,8 @@ describe("classifyOffhostBackup", () => { it("honours a caller's own period rather than the nightly default", () => { // A six-hour schedule: the same 12-hour-old object that is fresh under the - // nightly cron is two periods behind under this one. + // nightly cron is two periods behind under this one. The grace is passed + // as zero so the thresholds under test are the caller's period alone. const twelveHours = ago(12 * HOUR); expect( classifyOffhostBackup({ lastSuccessAt: twelveHours, now: NOW }).freshness, @@ -60,6 +100,7 @@ describe("classifyOffhostBackup", () => { lastSuccessAt: twelveHours, now: NOW, periodHours: 6, + graceHours: 0, }).freshness, ).toBe("due"); expect( @@ -67,6 +108,7 @@ describe("classifyOffhostBackup", () => { lastSuccessAt: twelveHours, now: NOW, periodHours: 5, + graceHours: 0, }).freshness, ).toBe("stale"); }); diff --git a/src/lib/jobs/offhost-backup-freshness.ts b/src/lib/jobs/offhost-backup-freshness.ts index fe1fee86c..869b5c1b0 100644 --- a/src/lib/jobs/offhost-backup-freshness.ts +++ b/src/lib/jobs/offhost-backup-freshness.ts @@ -18,8 +18,9 @@ * event as four: * * never — the worker has never put an object there for this account. - * fresh — inside one schedule period. This is what every account looks like - * on a host where the cron is doing its job. + * fresh — inside one schedule period plus the grace below. This is what + * every account looks like on a host where the cron is doing its + * job. * due — past one period, inside two. One night produced nothing: a retry, a * long run, a restart during the window. Worth showing, not worth * alarming about. @@ -29,10 +30,30 @@ /** * Hours between two scheduled off-host runs. Tracks `OFFHOST_BACKUP_CRON` - * (`30 2 * * *`) in `src/lib/jobs/reminder/register-maintenance.ts` — nightly. + * (`30 2 * * *`) in `src/lib/jobs/reminder/register-maintenance.ts`, which + * every schedule runs in `Europe/Berlin` (`tz` in `registrar-shared.ts`). + * This is the number the card quotes — the schedule's own period, not the + * threshold a verdict is measured against. */ export const OFFHOST_BACKUP_PERIOD_HOURS = 24; +/** + * Slack on top of one period, before a verdict moves. + * + * Two things make a healthy host miss a bare 24 hours. The schedule is + * Berlin-local, so on the DST fall-back night the gap between two 02:30 runs + * is 25 hours and every account on every host would cross one period on the + * same morning. And the ledger instant is when THAT account's object landed, + * deliberately: on a cohort walked one account at a time, an account reached + * at 05:30 one night and 06:30 the next is 25 hours old with two entirely + * successful runs behind it. + * + * Six hours is the budget for both. A card whose argument is that a count + * trains an operator to stop reading it cannot afford an annual cohort-wide + * false `due`. + */ +export const OFFHOST_BACKUP_GRACE_HOURS = 6; + export type OffhostBackupFreshness = "never" | "fresh" | "due" | "stale"; export interface OffhostBackupVerdict { @@ -46,12 +67,15 @@ export function classifyOffhostBackup(args: { now: Date; /** Defaults to the nightly schedule. Tests pass their own. */ periodHours?: number; + /** Defaults to `OFFHOST_BACKUP_GRACE_HOURS`. Tests pass their own. */ + graceHours?: number; }): OffhostBackupVerdict { const { lastSuccessAt, now } = args; if (lastSuccessAt === null) return { freshness: "never", ageHours: null }; const periodMs = (args.periodHours ?? OFFHOST_BACKUP_PERIOD_HOURS) * 3_600_000; + const graceMs = (args.graceHours ?? OFFHOST_BACKUP_GRACE_HOURS) * 3_600_000; const ageMs = now.getTime() - lastSuccessAt.getTime(); // A clock that ran backwards between the upload and this read reports a // negative age. That is a host-clock problem, not a backup problem, and @@ -59,7 +83,10 @@ export function classifyOffhostBackup(args: { // thing. const ageHours = Math.max(0, Math.floor(ageMs / 3_600_000)); - if (ageMs <= periodMs) return { freshness: "fresh", ageHours }; - if (ageMs <= periodMs * 2) return { freshness: "due", ageHours }; + // The grace rides on each threshold rather than on the period, so `due` + // still means "one scheduled run produced nothing" and `stale` still means + // two — the slack only keeps a long night from being read as a missed one. + if (ageMs <= periodMs + graceMs) return { freshness: "fresh", ageHours }; + if (ageMs <= periodMs * 2 + graceMs) return { freshness: "due", ageHours }; return { freshness: "stale", ageHours }; } diff --git a/tests/integration/offhost-backup-ledger.test.ts b/tests/integration/offhost-backup-ledger.test.ts index d588eaf89..27f70d178 100644 --- a/tests/integration/offhost-backup-ledger.test.ts +++ b/tests/integration/offhost-backup-ledger.test.ts @@ -14,6 +14,7 @@ import type { Readable } from "node:stream"; import { classifyOffhostBackup, + OFFHOST_BACKUP_GRACE_HOURS, OFFHOST_BACKUP_PERIOD_HOURS, } from "@/lib/jobs/offhost-backup-freshness"; import { @@ -129,7 +130,9 @@ describe("off-host backup ledger (real Postgres)", () => { // because the run that skipped this account still reported a success for // everybody else. const aged = new Date( - Date.now() - (OFFHOST_BACKUP_PERIOD_HOURS * 2 + 1) * HOUR, + Date.now() - + (OFFHOST_BACKUP_PERIOD_HOURS * 2 + OFFHOST_BACKUP_GRACE_HOURS + 1) * + HOUR, ); await prisma.offhostBackupState.update({ where: { userId: "account-one" },