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"]); }); });