mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-27 02:44:50 +01:00
fix(db): write backups atomically via .partial staging then rename
The health check treated any non-empty .sql.gz as a valid backup, but the writer streamed gzip output directly to the final path, so a crashed dump could leave a truncated archive that counted as fresh. Both backup engines now stream to <name>.sql.gz.partial and rename on success, making .sql.gz complete-by-convention; failures clean up partials and pruning removes orphaned partials older than 24h. Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
c2ea33846c
commit
259f1bf1ff
4 files changed
+51
-13
No files matched your search
@@ -122,6 +122,19 @@ describeEmbeddedPostgres("runDatabaseBackup", () => {
|
||||
`;
|
||||
}
|
||||
|
||||
const stalePartial = path.join(
|
||||
backupDir,
|
||||
"paperclip-test-20200101-000000.sql.gz.partial",
|
||||
);
|
||||
fs.writeFileSync(stalePartial, "orphaned crashed backup");
|
||||
const staleMtime = new Date(Date.now() - 48 * 60 * 60 * 1000);
|
||||
fs.utimesSync(stalePartial, staleMtime, staleMtime);
|
||||
const freshPartial = path.join(
|
||||
backupDir,
|
||||
"paperclip-test-20990101-000000.sql.gz.partial",
|
||||
);
|
||||
fs.writeFileSync(freshPartial, "concurrent in-progress backup");
|
||||
|
||||
const result = await runDatabaseBackup({
|
||||
connectionString: sourceConnectionString,
|
||||
backupDir,
|
||||
@@ -133,6 +146,11 @@ describeEmbeddedPostgres("runDatabaseBackup", () => {
|
||||
expect(result.backupFile).toMatch(/paperclip-test-.*\.sql\.gz$/);
|
||||
expect(result.sizeBytes).toBeGreaterThan(0);
|
||||
expect(fs.existsSync(result.backupFile)).toBe(true);
|
||||
// The completed archive is renamed from its .partial staging path,
|
||||
// stale orphaned partials are pruned, recent ones are left alone.
|
||||
expect(fs.existsSync(`${result.backupFile}.partial`)).toBe(false);
|
||||
expect(fs.existsSync(stalePartial)).toBe(false);
|
||||
expect(fs.existsSync(freshPartial)).toBe(true);
|
||||
|
||||
await runDatabaseRestore({
|
||||
connectionString: restoreConnectionString,
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { createReadStream, createWriteStream, existsSync, mkdirSync, readdirSync, statSync, unlinkSync } from "node:fs";
|
||||
import { createReadStream, createWriteStream, existsSync, mkdirSync, readdirSync, renameSync, statSync, unlinkSync } from "node:fs";
|
||||
import { basename, resolve } from "node:path";
|
||||
import { createInterface } from "node:readline";
|
||||
import { spawn } from "node:child_process";
|
||||
@@ -125,10 +125,19 @@ function pruneOldBackups(backupDir: string, retention: BackupRetentionPolicy, fi
|
||||
type BackupEntry = { name: string; fullPath: string; mtimeMs: number };
|
||||
const entries: BackupEntry[] = [];
|
||||
|
||||
const stalePartialCutoff = now - 24 * 60 * 60 * 1000;
|
||||
for (const name of readdirSync(backupDir)) {
|
||||
if (!name.startsWith(`${filenamePrefix}-`)) continue;
|
||||
if (!name.endsWith(".sql") && !name.endsWith(".sql.gz")) continue;
|
||||
const fullPath = resolve(backupDir, name);
|
||||
if (name.endsWith(".sql.gz.partial")) {
|
||||
// Orphaned in-progress archives from a crashed backup run.
|
||||
const stat = statSync(fullPath);
|
||||
if (stat.mtimeMs < stalePartialCutoff) {
|
||||
try { unlinkSync(fullPath); } catch { /* ignore */ }
|
||||
}
|
||||
continue;
|
||||
}
|
||||
if (!name.endsWith(".sql") && !name.endsWith(".sql.gz")) continue;
|
||||
const stat = statSync(fullPath);
|
||||
entries.push({ name, fullPath, mtimeMs: stat.mtimeMs });
|
||||
}
|
||||
@@ -536,6 +545,9 @@ export async function runDatabaseBackup(opts: RunDatabaseBackupOptions): Promise
|
||||
mkdirSync(opts.backupDir, { recursive: true });
|
||||
const sqlFile = resolve(opts.backupDir, `${filenamePrefix}-${timestamp()}.sql`);
|
||||
const backupFile = `${sqlFile}.gz`;
|
||||
// Stream to a .partial path and rename on success so a .sql.gz file only
|
||||
// ever exists once the archive is complete (crash/kill leaves .partial).
|
||||
const partialFile = `${backupFile}.partial`;
|
||||
const writer = createBufferedTextFileWriter(sqlFile);
|
||||
|
||||
try {
|
||||
@@ -545,9 +557,10 @@ export async function runDatabaseBackup(opts: RunDatabaseBackupOptions): Promise
|
||||
await closeSql();
|
||||
await runPgDumpBackup({
|
||||
connectionString: opts.connectionString,
|
||||
backupFile,
|
||||
backupFile: partialFile,
|
||||
connectTimeout,
|
||||
});
|
||||
renameSync(partialFile, backupFile);
|
||||
await writer.abort();
|
||||
const sizeBytes = statSync(backupFile).size;
|
||||
const prunedCount = pruneOldBackups(opts.backupDir, retention, filenamePrefix);
|
||||
@@ -557,8 +570,10 @@ export async function runDatabaseBackup(opts: RunDatabaseBackupOptions): Promise
|
||||
prunedCount,
|
||||
};
|
||||
} catch (error) {
|
||||
if (existsSync(backupFile)) {
|
||||
try { unlinkSync(backupFile); } catch { /* ignore */ }
|
||||
for (const staleFile of [partialFile, backupFile]) {
|
||||
if (existsSync(staleFile)) {
|
||||
try { unlinkSync(staleFile); } catch { /* ignore */ }
|
||||
}
|
||||
}
|
||||
if (backupEngine === "pg_dump") {
|
||||
throw error;
|
||||
@@ -957,8 +972,9 @@ export async function runDatabaseBackup(opts: RunDatabaseBackupOptions): Promise
|
||||
|
||||
// Compress the SQL file with gzip
|
||||
const sqlReadStream = createReadStream(sqlFile);
|
||||
const gzWriteStream = createWriteStream(backupFile);
|
||||
const gzWriteStream = createWriteStream(partialFile);
|
||||
await pipeline(sqlReadStream, createGzip(), gzWriteStream);
|
||||
renameSync(partialFile, backupFile);
|
||||
unlinkSync(sqlFile);
|
||||
|
||||
const sizeBytes = statSync(backupFile).size;
|
||||
@@ -971,11 +987,10 @@ export async function runDatabaseBackup(opts: RunDatabaseBackupOptions): Promise
|
||||
};
|
||||
} catch (error) {
|
||||
await writer.abort();
|
||||
if (existsSync(backupFile)) {
|
||||
try { unlinkSync(backupFile); } catch { /* ignore */ }
|
||||
}
|
||||
if (existsSync(sqlFile)) {
|
||||
try { unlinkSync(sqlFile); } catch { /* ignore */ }
|
||||
for (const staleFile of [partialFile, backupFile, sqlFile]) {
|
||||
if (existsSync(staleFile)) {
|
||||
try { unlinkSync(staleFile); } catch { /* ignore */ }
|
||||
}
|
||||
}
|
||||
throw error;
|
||||
} finally {
|
||||
|
||||
@@ -166,7 +166,7 @@ describe("GET /health", () => {
|
||||
expect(res.body.warnings).toEqual(res.body.databaseBackup.warnings);
|
||||
});
|
||||
|
||||
it("ignores zero-byte backup archives when picking the latest backup", async () => {
|
||||
it("ignores zero-byte and in-progress .partial archives when picking the latest backup", async () => {
|
||||
const backupDir = fs.mkdtempSync(path.join(os.tmpdir(), "paperclip-health-backups-"));
|
||||
const goodBackup = path.join(backupDir, "paperclip-20260705-031702.sql.gz");
|
||||
fs.writeFileSync(goodBackup, "backup");
|
||||
@@ -182,6 +182,8 @@ describe("GET /health", () => {
|
||||
new Date("2026-07-06T12:00:00.000Z"),
|
||||
new Date("2026-07-06T12:00:00.000Z"),
|
||||
);
|
||||
const partialBackup = path.join(backupDir, "paperclip-20260706-123000.sql.gz.partial");
|
||||
fs.writeFileSync(partialBackup, "in-progress dump");
|
||||
const app = createApp(createHealthyDb(), testServerInfo, {
|
||||
enabled: true,
|
||||
backupDir,
|
||||
|
||||
@@ -93,7 +93,10 @@ async function findLatestBackup(backupDir: string, nowMs: number) {
|
||||
if (isMissingFileError(error)) continue;
|
||||
throw error;
|
||||
}
|
||||
// Zero-byte archives are failed/partial dumps, not restorable backups.
|
||||
// The backup writer streams to `<name>.sql.gz.partial` and renames on
|
||||
// success, so `.sql.gz` files are complete by convention (the extension
|
||||
// filter above excludes in-progress/crashed partials). Zero-byte archives
|
||||
// from older writers are still skipped as failed dumps.
|
||||
if (fileStat.size === 0) continue;
|
||||
if (!latest || fileStat.mtimeMs > latest.mtimeMs) {
|
||||
latest = { fullPath, mtimeMs: fileStat.mtimeMs, size: fileStat.size };
|
||||
|
||||
Reference in new issue
Block a user