From e73c7b3d6856baaadef51a49962dad2eba57b593 Mon Sep 17 00:00:00 2001 From: jarek Date: Tue, 18 Aug 2026 10:05:37 +0200 Subject: [PATCH] backup helper wait uses an unbounded deadline --- src/lib/server/backups/models.ts | 12 +++++++----- src/lib/server/docker.ts | 11 +++++++---- src/lib/server/helper-wait-core.ts | 28 ++++++++++++++++++++++++++++ 3 files changed, 42 insertions(+), 9 deletions(-) create mode 100644 src/lib/server/helper-wait-core.ts diff --git a/src/lib/server/backups/models.ts b/src/lib/server/backups/models.ts index 6cdbc8a..1c471b0 100644 --- a/src/lib/server/backups/models.ts +++ b/src/lib/server/backups/models.ts @@ -304,12 +304,14 @@ export function readConfigIdFromTags(tags: string[]): number | null { * cap killed healthy work mid-run (#1382). A stuck op is handled by manual cancel (the * helper is a named container killed by cancelBackup, and every run shows as `running` in * the execution log) and the helper reaper — the same model backrest/zerobyte use (neither - * times out a backup). The two tiers remain so callers can still opt into a cap via env: - * `RESTIC_TIMEOUT` (interactive: list/ls/init) and `RESTIC_MAINTENANCE_TIMEOUT` - * (data: backup/restore/prune). Both default to 0 (off). */ + * times out a backup). ONE env caps everything: `RESTIC_TIMEOUT` (milliseconds, 0 = off) + * applies to every restic operation - the host `restic` spawn AND the helper container. + * The tiers stay in the call API only as a log label; they no longer carry separate + * limits (previously RESTIC_MAINTENANCE_TIMEOUT set the data tier; folded into one). */ +const RESTIC_TIMEOUT_MS = Number(process.env.RESTIC_TIMEOUT ?? 0); export const TIMEOUTS = { - interactive: Number(process.env.RESTIC_TIMEOUT ?? 0), - data: Number(process.env.RESTIC_MAINTENANCE_TIMEOUT ?? 0), + interactive: RESTIC_TIMEOUT_MS, + data: RESTIC_TIMEOUT_MS, } as const; export type TimeoutTier = keyof typeof TIMEOUTS; diff --git a/src/lib/server/docker.ts b/src/lib/server/docker.ts index 993e264..e0ea1fb 100644 --- a/src/lib/server/docker.ts +++ b/src/lib/server/docker.ts @@ -15,6 +15,7 @@ import * as tls from 'node:tls'; import { createHash } from 'node:crypto'; import { pumpWebStreamToWritable } from './stream-pump'; import { computeRequestTimeoutMs } from './backups/request-timeout'; +import { helperWaitDeadline } from './helper-wait-core'; import type { Environment } from './db'; import { getSetting } from './db'; import { getAdditionalVolumeBinds, dedupeVolumesForRecreate } from './mount-dedupe'; @@ -5056,11 +5057,13 @@ export async function runContainerWithStreaming(options: { // Applies to every helper this function runs — backup/restore AND the image scanner. let exitCode: number | undefined; if (process.env.HELPERS_WAIT_MODE !== 'wait') { - // Bound by the caller's timeout (backup passes a long 'data' tier, the - // scanner passes 600_000). Fall back to 1h so poll mode never hangs. - // One line so the log confirms which exit-signal mode is active (#1344). + // Bounded by the caller's timeout (scanner passes 600_000, probes 60_000); + // the backup helper passes 0 = unbounded. One line so the log confirms which + // exit-signal mode is active (#1344). console.log(`[runContainerWithStreaming] Awaiting exit of ${options.name ?? containerId.slice(0, 12)} via POLL mode`); - const deadline = Date.now() + (options.timeout && options.timeout > 0 ? options.timeout : 3_600_000); + // A positive timeout caps the wait; 0/omitted is UNBOUNDED (see helper-wait-core: + // the backup helper passes 0 on purpose, bounded by cancel + the reaper - #1382). + const deadline = helperWaitDeadline(options.timeout, Date.now()); while (exitCode === undefined && Date.now() < deadline) { try { const insp = await dockerFetch(`/containers/${containerId}/json`, {}, options.envId); diff --git a/src/lib/server/helper-wait-core.ts b/src/lib/server/helper-wait-core.ts new file mode 100644 index 0000000..98e4526 --- /dev/null +++ b/src/lib/server/helper-wait-core.ts @@ -0,0 +1,28 @@ +/** + * Pure deadline policy for waiting on a helper container's exit (runContainerWithStreaming + * POLL mode). Kept import-light so it unit-tests under the bun runner - docker.ts pulls + * better-sqlite3 transitively and can't load there. + * + * The rule: a POSITIVE timeout caps the wait; anything else (0 or undefined) is UNBOUNDED. + * There is deliberately NO hidden default cap - the backup helper passes 0 on purpose (a + * large backup legitimately runs for hours, bounded by manual cancel + the reaper, not a + * wall clock - #1382). The old `timeout || 3_600_000` turned "unbounded" into 60 minutes + * and force-killed healthy backups mid-run. + */ + +/** + * The cap in ms for a helper wait: the caller's timeout when positive, else 0 (=unbounded). + * Returned separately from the deadline so callers/tests can reason about "is this capped". + */ +export function helperWaitCapMs(timeout: number | undefined): number { + return timeout && timeout > 0 ? timeout : 0; +} + +/** + * The absolute deadline (ms epoch) for a helper wait given the caller's timeout and the + * current time. `Infinity` means unbounded (poll until the container exits or is removed). + */ +export function helperWaitDeadline(timeout: number | undefined, now: number): number { + const cap = helperWaitCapMs(timeout); + return cap > 0 ? now + cap : Infinity; +}