From 874d02294e0a163beae511b4f16f34c145ef19e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Sun, 27 Sep 2026 13:13:50 +0200 Subject: [PATCH] fix(backup): let the restore job's budget admit large accounts A restore is refused before it deletes anything when its transaction limit would run past three quarters of the job's expiry. At two hours that refused a same-size replacement of about four million readings. The expiry is now six hours, which admits ten million over ten million; a dead worker still frees the account within minutes through the heartbeat, so the longer expiry does not hold an account longer. The refusal now names both counts, the readings in the backup and the ones the account holds now (removed readings included until the nightly clean-up), since clearing the latter costs time too. --- src/lib/export/restore-backup.ts | 16 ++++++--- .../__tests__/backup-restore-budget.test.ts | 34 +++++++++++++++++++ src/lib/jobs/backup-restore.ts | 22 ++++++++---- tests/integration/backup-restore-job.test.ts | 4 +++ 4 files changed, 66 insertions(+), 10 deletions(-) create mode 100644 src/lib/jobs/__tests__/backup-restore-budget.test.ts diff --git a/src/lib/export/restore-backup.ts b/src/lib/export/restore-backup.ts index e2b317ce1..0de217c1f 100644 --- a/src/lib/export/restore-backup.ts +++ b/src/lib/export/restore-backup.ts @@ -185,13 +185,17 @@ export interface RestoreResponse { * to half a CPU and 40 MB/s of writes, where its transaction ran 16 min of a * 37 min budget. */ -function restoreTransactionTimeoutMs( +export function restoreTransactionTimeoutMs( fileMeasurements: number, currentMeasurements: number, ): number { return 120_000 + fileMeasurements + Math.ceil(currentMeasurements / 10); } +function readingsPhrase(count: number): string { + return `${count} ${count === 1 ? "reading" : "readings"}`; +} + function decodeEncryptedBytes(encoded: string): Uint8Array { const decoded = Buffer.from(encoded, "base64"); const bytes = new Uint8Array(new ArrayBuffer(decoded.byteLength)); @@ -309,7 +313,7 @@ export type RestoreBackupOutcome = * Time kept free after the transaction for the rollup folds and the audit row, * when the restore checks it fits the caller's deadline. */ -const AFTER_TRANSACTION_ALLOWANCE_MS = 10 * 60 * 1000; +export const RESTORE_AFTER_TRANSACTION_ALLOWANCE_MS = 10 * 60 * 1000; function refused( status: number, @@ -636,7 +640,7 @@ export async function restoreBackup( ); if ( input.deadline !== undefined && - Date.now() + transactionTimeoutMs + AFTER_TRANSACTION_ALLOWANCE_MS > + Date.now() + transactionTimeoutMs + RESTORE_AFTER_TRANSACTION_ALLOWANCE_MS > input.deadline ) { await auditLog("admin.backups.restore.failed", { @@ -647,12 +651,16 @@ export async function restoreBackup( ownerId, reason: "time_budget", measurements: streamed.measurementCount, + currentMeasurements, }, }); + // Both counts, because both cost time: the account's readings are deleted + // (removed ones included, until the nightly clean-up purges them) before + // the file's are written. return refused( 503, "time_budget", - `A backup of ${streamed.measurementCount} readings needs more time than the restore job allows. Nothing was changed.`, + `Restoring ${readingsPhrase(streamed.measurementCount)} in the backup over the ${readingsPhrase(currentMeasurements)} this account holds now (removed readings count until the nightly clean-up) needs more time than the restore job allows. Nothing was changed.`, ); } diff --git a/src/lib/jobs/__tests__/backup-restore-budget.test.ts b/src/lib/jobs/__tests__/backup-restore-budget.test.ts new file mode 100644 index 000000000..bf9203483 --- /dev/null +++ b/src/lib/jobs/__tests__/backup-restore-budget.test.ts @@ -0,0 +1,34 @@ +/** + * The restore job's expiry must not be what refuses a plausible account. + * + * `restoreBackup` refuses, before deleting anything, a file whose transaction + * limit would run past the job's budget (three quarters of the pg-boss + * expiry). At two hours that refused a same-size replacement of about four + * million readings, a record a long-running Apple Health sync reaches. The + * expiry is sized so ten million readings over ten million still fit; the + * expiry is only the backstop for a dead worker, which the heartbeat sweep + * notices within minutes. + */ +import { describe, expect, it } from "vitest"; + +import { BACKUP_RESTORE_EXPIRE_SECONDS } from "@/lib/jobs/backup-restore"; +import { JOB_BUDGET_SHARE } from "@/lib/jobs/job-budget"; +import { + RESTORE_AFTER_TRANSACTION_ALLOWANCE_MS, + restoreTransactionTimeoutMs, +} from "@/lib/export/restore-backup"; + +describe("restore job budget", () => { + it("admits ten million readings replacing ten million", () => { + const budgetMs = BACKUP_RESTORE_EXPIRE_SECONDS * 1000 * JOB_BUDGET_SHARE; + const needed = + restoreTransactionTimeoutMs(10_000_000, 10_000_000) + + RESTORE_AFTER_TRANSACTION_ALLOWANCE_MS; + expect(needed).toBeLessThan(budgetMs); + }); + + it("stays inside pg-boss's own expiry ceiling", () => { + // pg-boss refuses an expiry of 24 hours or more at send time. + expect(BACKUP_RESTORE_EXPIRE_SECONDS).toBeLessThan(24 * 60 * 60); + }); +}); diff --git a/src/lib/jobs/backup-restore.ts b/src/lib/jobs/backup-restore.ts index b170ea1ea..0e76db75e 100644 --- a/src/lib/jobs/backup-restore.ts +++ b/src/lib/jobs/backup-restore.ts @@ -31,7 +31,7 @@ * - One restore per account at a time. The job table's partial unique index * refuses a second queued or running row for the same account, and the * route answers that with 409. - * - A time budget. The job's expiry is two hours; `restoreBackup` refuses, + * - A time budget. The job's expiry is six hours; `restoreBackup` refuses, * before deleting anything, a file whose transaction limit would run past * three quarters of it (`jobDeadline`), the same share the other long * passes stop at. pg-boss does not retry the job (`retryLimit: 0`): a @@ -84,12 +84,22 @@ export const BACKUP_RESTORE_QUEUE = "backup-restore"; export const BACKUP_RESTORE_ACTIVE_CODE = "backup.restore.active"; /** - * Two hours. The restore of 1.25 million readings took a minute and a half; - * the limit is sized so a record many times that still fits, while a job whose - * worker died does not keep its pg-boss row active for longer than an operator - * would wait. + * Six hours. The expiry is what bounds the size of a restore: `restoreBackup` + * refuses a file whose transaction limit (`restoreTransactionTimeoutMs`, two + * minutes plus a second per 1 000 readings in the file and per 10 000 in the + * account) would run past three quarters of it. At two hours that refused a + * same-size replacement of about four million readings, which a long-running + * Apple Health sync reaches; six hours admits ten million over ten million + * (`backup-restore-budget.test.ts`). The transaction limit is itself about + * twice the slowest restore measured (1.89 million readings in 16 minutes on + * a database held to half a CPU), so the real work finishes well inside it. + * + * The expiry is not what frees an account whose worker died: once its + * heartbeat is older than {@link BACKUP_RESTORE_STALE_AFTER_MS}, the next + * restore request's admission check and the sweep fail or re-queue it, so a + * longer expiry does not keep an account locked longer. pg-boss refuses an expiry of 24 hours or more. */ -export const BACKUP_RESTORE_EXPIRE_SECONDS = 2 * 60 * 60; +export const BACKUP_RESTORE_EXPIRE_SECONDS = 6 * 60 * 60; export const BACKUP_RESTORE_SEND_OPTIONS = { retryLimit: 0, diff --git a/tests/integration/backup-restore-job.test.ts b/tests/integration/backup-restore-job.test.ts index d21403c4b..670143467 100644 --- a/tests/integration/backup-restore-job.test.ts +++ b/tests/integration/backup-restore-job.test.ts @@ -648,6 +648,10 @@ describe("backup restore as a background job", () => { const job = await readBackupRestoreJob(data.jobId); expect(job?.status).toBe("failed"); expect(job?.failure?.code).toBe("time_budget"); + // Both halves of the work are named: what the file carries and what the + // account holds now, since the clearing of the latter costs time too. + expect(job?.failure?.message).toContain("1 reading in the backup"); + expect(job?.failure?.message).toContain("1 reading this account holds now"); expect(await measurementIdsOf(admin.id)).toEqual(["live-1"]); }); });