mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-08 12:32:43 +00:00
fix: give the weekly backup a run window of its own
The `data-backup` queue was registered without send options, so every job it minted took pg-boss's queue defaults: a fifteen-minute expiration and two redeliveries. A snapshot that serialises every account's whole record does not fit in fifteen minutes on a large one, so the pass was killed part-way through, redelivered twice against the same record, and reached `failed` with "job timed out" about three quarters of an hour after it started — having never once completed. Two hours, on the schedule rather than on the queue: `create_queue()` ends in ON CONFLICT DO NOTHING, so a queue-level default would apply to a fresh database and be inert on every instance that already has the queue. The admin run-now route sends with the same options, or an ad-hoc snapshot would silently keep the default the cron was rescued from. The default two retries stay. The old redeliveries were pointless because the first attempt could not succeed either; a transient failure genuinely benefits from a second go, and the alternative is losing the week.
This commit is contained in:
1 parent
214b268b41
commit
b5dd9372a7
5 files changed
+127
-8
No files matched your search
@@ -15,11 +15,13 @@ import { auditLog } from "@/lib/auth/audit";
|
||||
import { annotate } from "@/lib/logging/context";
|
||||
import { getGlobalBoss } from "@/lib/jobs/boss-instance";
|
||||
import { checkRateLimit } from "@/lib/rate-limit";
|
||||
import {
|
||||
DATA_BACKUP_QUEUE,
|
||||
DATA_BACKUP_SEND_OPTIONS,
|
||||
} from "@/lib/jobs/data-backup-policy";
|
||||
|
||||
export const dynamic = "force-dynamic";
|
||||
|
||||
const DATA_BACKUP_QUEUE = "data-backup";
|
||||
|
||||
export const POST = apiHandler(async (request: NextRequest) => {
|
||||
const { user: admin } = await requireAdmin();
|
||||
annotate({ action: { name: "admin.backups.run" } });
|
||||
@@ -57,9 +59,14 @@ export const POST = apiHandler(async (request: NextRequest) => {
|
||||
throw new HttpError(503, "Background worker is not running");
|
||||
}
|
||||
|
||||
const jobId = await boss.send(DATA_BACKUP_QUEUE, {
|
||||
triggeredAt: new Date().toISOString(),
|
||||
});
|
||||
// Same expiration window as the weekly cron. An ad-hoc snapshot runs the
|
||||
// identical pass over the identical record, so it must not silently keep the
|
||||
// 15-minute queue default the cron was rescued from.
|
||||
const jobId = await boss.send(
|
||||
DATA_BACKUP_QUEUE,
|
||||
{ triggeredAt: new Date().toISOString() },
|
||||
DATA_BACKUP_SEND_OPTIONS,
|
||||
);
|
||||
|
||||
annotate({ meta: { job_id: jobId ?? null } });
|
||||
|
||||
|
||||
@@ -0,0 +1,55 @@
|
||||
/**
|
||||
* Send-policy pins for the weekly `data-backup` queue.
|
||||
*
|
||||
* The queue was registered with no send options at all, so every job it minted
|
||||
* inherited pg-boss's queue defaults — `expireInSeconds: 900`, `retryLimit: 2`.
|
||||
* A full-record snapshot of a large account does not fit in fifteen minutes, so
|
||||
* the pass was killed mid-run, redelivered twice against the same record, and
|
||||
* gave up roughly 46 minutes after it started. Weekly, silently, for as long as
|
||||
* the record stayed large.
|
||||
*
|
||||
* Three facts have to hold together or the window is not real:
|
||||
*
|
||||
* 1. the policy names a window well past the 15-minute default,
|
||||
* 2. the CRON SCHEDULE carries it — send options ride the schedule because
|
||||
* pg-boss's `create_queue()` ends in ON CONFLICT DO NOTHING, so a
|
||||
* queue-level default would be inert on every already-running instance,
|
||||
* 3. the admin "run now" route sends with the same options, or an ad-hoc
|
||||
* snapshot silently keeps the default that broke the cron one.
|
||||
*/
|
||||
import { readFileSync } from "node:fs";
|
||||
import { join } from "node:path";
|
||||
import { describe, expect, it } from "vitest";
|
||||
|
||||
import {
|
||||
DATA_BACKUP_QUEUE,
|
||||
DATA_BACKUP_SEND_OPTIONS,
|
||||
} from "../data-backup-policy";
|
||||
|
||||
const read = (file: string) => readFileSync(join(process.cwd(), file), "utf8");
|
||||
|
||||
describe("data-backup — pg-boss send policy", () => {
|
||||
it("widens the expiration far past the 15-minute queue default", () => {
|
||||
expect(DATA_BACKUP_QUEUE).toBe("data-backup");
|
||||
// 900 s is the pg-boss 12.26 queue default (`QUEUE_DEFAULTS.expire_seconds`
|
||||
// in `dist/plans.js`); anything at or below it re-opens the bug.
|
||||
expect(DATA_BACKUP_SEND_OPTIONS.expireInSeconds).toBeGreaterThan(900);
|
||||
expect(DATA_BACKUP_SEND_OPTIONS.expireInSeconds).toBeGreaterThanOrEqual(
|
||||
60 * 60,
|
||||
);
|
||||
});
|
||||
|
||||
it("the weekly schedule carries the policy", () => {
|
||||
const source = read("src/lib/jobs/reminder/register-maintenance.ts");
|
||||
expect(source).toMatch(
|
||||
/\[\s*DATA_BACKUP_QUEUE,\s*DATA_BACKUP_CRON,\s*DATA_BACKUP_SEND_OPTIONS,?\s*\]/,
|
||||
);
|
||||
});
|
||||
|
||||
it("the admin run-now route sends with the same policy", () => {
|
||||
const source = read("src/app/api/admin/backups/run/route.ts");
|
||||
expect(source).toMatch(
|
||||
/boss\.send\(\s*DATA_BACKUP_QUEUE,[\s\S]{0,200}DATA_BACKUP_SEND_OPTIONS,?\s*\)/,
|
||||
);
|
||||
});
|
||||
});
|
||||
@@ -26,4 +26,16 @@ describe("pg-boss failed-row ledger retention", () => {
|
||||
PG_BOSS_FAILED_ROW_AVAILABILITY_HOURS,
|
||||
);
|
||||
});
|
||||
|
||||
it("pins the queue defaults the data-backup window had to escape", () => {
|
||||
const plans = readFileSync(join(PG_BOSS_ROOT, "dist/plans.js"), "utf8");
|
||||
|
||||
// These two lines are the whole story behind the weekly backup's "job
|
||||
// timed out": a pass with no send options gets fifteen minutes and two
|
||||
// redeliveries, so a snapshot that cannot finish in fifteen minutes gives
|
||||
// up 45 minutes later having never once completed. If a pg-boss upgrade
|
||||
// changes either default, `DATA_BACKUP_SEND_OPTIONS` deserves a re-read.
|
||||
expect(plans).toContain("expire_seconds: FIFTEEN_MINUTES");
|
||||
expect(plans).toContain("retry_limit: 2");
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,39 @@
|
||||
/**
|
||||
* Queue name and pg-boss send policy for the weekly on-host data backup.
|
||||
*
|
||||
* A leaf module on purpose: the cron registrar, the admin "run now" route and
|
||||
* the policy test all need these two facts, and none of them should have to
|
||||
* pull in the worker's handler graph to get them.
|
||||
*
|
||||
* The queue was registered with no send options, so every job it minted took
|
||||
* pg-boss 12.26's queue defaults — `expireInSeconds: 900`, `retryLimit: 2`
|
||||
* (`QUEUE_DEFAULTS` in `pg-boss/dist/plans.js`). Fifteen minutes is a fine
|
||||
* default for a cleanup tick and far too short for a pass that serialises every
|
||||
* account's whole record: the job was killed mid-run, redelivered twice against
|
||||
* the same record, and reached `failed` with `job timed out` about 46 minutes
|
||||
* after it started. Nothing about that says "too slow" — the pass never got to
|
||||
* finish once.
|
||||
*
|
||||
* Two hours is deliberately generous rather than tuned. The handler now costs a
|
||||
* fraction of that on a large record (see the CHANGELOG entry), but the pass
|
||||
* loops over EVERY account on the instance and a self-host's disk is whatever
|
||||
* it is; the window's job is to be impossible to hit for the right reasons, and
|
||||
* a failure that arrives at the two-hour mark is a real one worth reading.
|
||||
* pg-boss refuses anything at or past 24 hours.
|
||||
*
|
||||
* The default `retryLimit: 2` stays. The old redeliveries were pointless
|
||||
* because the first attempt could not succeed either; a transient failure — a
|
||||
* pool hiccup, a locked row — genuinely benefits from a second attempt, and the
|
||||
* alternative is losing the week.
|
||||
*
|
||||
* The options ride on the SEND, not on `createQueue`: pg-boss's `create_queue()`
|
||||
* ends in `ON CONFLICT DO NOTHING`, so a queue-level default would apply to a
|
||||
* fresh database and be inert on every instance that already has the queue —
|
||||
* exactly the trap `reconcileQueuePolicies` exists to work around.
|
||||
*/
|
||||
|
||||
export const DATA_BACKUP_QUEUE = "data-backup";
|
||||
|
||||
export const DATA_BACKUP_SEND_OPTIONS = {
|
||||
expireInSeconds: 2 * 60 * 60,
|
||||
} as const;
|
||||
@@ -138,6 +138,10 @@ import {
|
||||
} from "@/lib/jobs/environment-fetch";
|
||||
import { recordError } from "@/lib/jobs/worker-status";
|
||||
import { jobDone, type JobOutcome } from "@/lib/jobs/job-outcome";
|
||||
import {
|
||||
DATA_BACKUP_QUEUE,
|
||||
DATA_BACKUP_SEND_OPTIONS,
|
||||
} from "@/lib/jobs/data-backup-policy";
|
||||
import { workerLog, BOOT_BACKFILL_STAGGER_SECONDS } from "./shared";
|
||||
import {
|
||||
createAndSchedule,
|
||||
@@ -221,8 +225,6 @@ import {
|
||||
type AchievementUnlockSweepPayload,
|
||||
} from "@/lib/jobs/achievement-unlock-sweep";
|
||||
|
||||
const DATA_BACKUP_QUEUE = "data-backup";
|
||||
|
||||
const DATA_BACKUP_CRON = "0 3 * * 0"; // weekly Sunday at 03:00
|
||||
|
||||
const RATE_LIMIT_CLEANUP_QUEUE = "rate-limit-cleanup";
|
||||
@@ -478,7 +480,11 @@ const schedules: ScheduleEntry[] = [
|
||||
// v1.25 (W-ENV) — daily 02:10 Europe/Berlin discovery tick (empty payload)
|
||||
// that fans out one per-user environment fetch per opted-in account.
|
||||
[ENVIRONMENT_FETCH_QUEUE, ENVIRONMENT_FETCH_CRON],
|
||||
[DATA_BACKUP_QUEUE, DATA_BACKUP_CRON],
|
||||
// The send options are the point of this tuple: without them the weekly
|
||||
// snapshot inherits pg-boss's 15-minute expiration and is killed mid-run on
|
||||
// any sizeable record. See `data-backup-policy.ts` for why the window rides
|
||||
// the schedule rather than the queue.
|
||||
[DATA_BACKUP_QUEUE, DATA_BACKUP_CRON, DATA_BACKUP_SEND_OPTIONS],
|
||||
[RATE_LIMIT_CLEANUP_QUEUE, RATE_LIMIT_CLEANUP_CRON, cronIsTheRetry],
|
||||
[IDEMPOTENCY_CLEANUP_QUEUE, IDEMPOTENCY_CLEANUP_CRON, cronIsTheRetry],
|
||||
[AUDIT_LOG_CLEANUP_QUEUE, AUDIT_LOG_CLEANUP_CRON],
|
||||
|
||||
Reference in new issue
Block a user