diff --git a/src/lib/database-backup/__tests__/dump-core.test.ts b/src/lib/database-backup/__tests__/dump-core.test.ts index b819434a4..34ee6baa7 100644 --- a/src/lib/database-backup/__tests__/dump-core.test.ts +++ b/src/lib/database-backup/__tests__/dump-core.test.ts @@ -10,9 +10,14 @@ * tests that keep its plain-JS reading of both from drifting. */ import { - mkdtempSync, + chmodSync, createReadStream, + fstatSync, + mkdirSync, + mkdtempSync, + readdirSync, rmSync, + statSync, utimesSync, writeFileSync, } from "node:fs"; @@ -46,6 +51,7 @@ import { resolveBackupSchedule, resolveDailyKeep, selectForDeletion, + verifyArchive, type DirectoryProbe, type DumpSet, type Env, @@ -126,6 +132,44 @@ describe("dump names", () => { }); }); +/** How many of this process's descriptors are open on `path`'s file. */ +function openHandlesOn(path: string): number { + const { dev, ino } = statSync(path); + return readdirSync("/dev/fd").filter((fd) => { + try { + const s = fstatSync(Number(fd)); + return s.dev === dev && s.ino === ino; + } catch { + return false; + } + }).length; +} + +describe("reading an archive back", () => { + // The worker lives for weeks and checks a dump every night. A handle it + // keeps on a checked dump keeps that dump's blocks allocated after + // retention deletes it, so the volume fills with "deleted" dumps. + it("leaves no handle open on the dump once the listing is read", async () => { + const dir = tempDir(); + const bin = join(dir, "bin"); + mkdirSync(bin); + // A pg_restore that does what the real one does with a custom-format + // archive: read the table of contents and exit without reading the rest. + writeFileSync( + join(bin, "pg_restore"), + `#!/bin/sh\nprintf '; header\\n1; 2615 2200 TABLE public a\\n2; 2615 2201 TABLE public b\\n'\n`, + ); + chmodSync(join(bin, "pg_restore"), 0o755); + vi.stubEnv("PATH", `${bin}:${process.env.PATH ?? ""}`); + const dump = join(dir, "big.dump"); + writeFileSync(dump, Buffer.alloc(16 * 1024 * 1024, 7)); + + expect(await verifyArchive(dump)).toBe(2); + await new Promise((resolve) => setTimeout(resolve, 200)); + expect(openHandlesOn(dump)).toBe(0); + }); +}); + describe("configuration", () => { it("keeps at least one daily dump and ignores what is not a number", () => { expect(resolveDailyKeep({})).toBe(7); diff --git a/src/lib/database-backup/dump-core.mjs b/src/lib/database-backup/dump-core.mjs index dd8618aea..227e707c4 100644 --- a/src/lib/database-backup/dump-core.mjs +++ b/src/lib/database-backup/dump-core.mjs @@ -739,16 +739,19 @@ export async function serverVersion(client) { /** * Run one Postgres client tool. Standard output goes to `stdout` when given - * (a file stream) and is collected otherwise; standard input comes from - * `stdin` when given. Rejects with the tool's own last lines on a non-zero - * exit, and kills it at `deadline` (epoch ms). + * (a file stream) and is collected otherwise. Rejects with the tool's own + * last lines on a non-zero exit, and kills it at `deadline` (epoch ms). + * + * A file the tool reads is named in `args`, never streamed to its standard + * input: `pg_restore --list` reads an archive's table of contents and exits, + * and a stream piped into it would stay open on the file. In the long-lived + * worker that kept every checked dump allocated after retention deleted it. * * @param {string} tool * @param {string[]} args * @param {{ * env?: Record; * stdout?: NodeJS.WritableStream; - * stdin?: NodeJS.ReadableStream; * deadline?: number; * }} [options] * @returns {Promise} @@ -757,7 +760,7 @@ export function runTool(tool, args, options = {}) { return new Promise((resolve, reject) => { const child = spawn(tool, args, { env: { ...process.env, ...(options.env ?? {}) }, - stdio: [options.stdin ? "pipe" : "ignore", "pipe", "pipe"], + stdio: ["ignore", "pipe", "pipe"], }); /** @type {Buffer[]} */ const out = []; @@ -787,16 +790,6 @@ export function runTool(tool, args, options = {}) { child.stderr.on("data", (c) => { err = (err + c.toString("utf8")).slice(-4000); }); - if (options.stdin && child.stdin) { - options.stdin.on("error", (e) => { - child.kill("SIGTERM"); - reject(e); - }); - child.stdin.on("error", () => { - // The tool closed its input early; its exit code says why. - }); - options.stdin.pipe(child.stdin); - } child.on("error", (e) => { if (timer) clearTimeout(timer); reject( @@ -841,9 +834,7 @@ export async function pgDumpVersion() { * @param {string} path */ export async function verifyArchive(path) { - const listing = await runTool("pg_restore", ["--list"], { - stdin: createReadStream(path), - }); + const listing = await runTool("pg_restore", ["--list", path]); const entries = listing .split("\n") .filter((line) => line.trim() && !line.startsWith(";")).length;