Harden the backup tree's atomic copy (closes #22)
check / check (push) Successful in 27s

The backup copy now fsyncs its temp file before the rename and the
directory after it, using the download writer's new fsyncPath helper.
Each backup run deletes .quak-backup-*.tmp files whose process is no
longer running, leaving those of a concurrent backup alone. The rename
sites and the README backup layout state that a symlink at the
destination is replaced and the new file takes the temp file's
permissions, and the README names the temp files. Adds tests for a
missing and an unwritable destination directory for downloadFile and
downloadThumbnail.

Model: opus-5-5
This commit was merged in pull request #85.
This commit is contained in:
2026-09-23 02:44:46 +02:00
parent d545dcd8b1
commit ed535be1da
6 changed files with 223 additions and 16 deletions
+51 -9
View File
@@ -29,19 +29,20 @@
// rather than counted forever, which would poison a scheduled backup's exit code.
import {
copyFileSync,
lstatSync,
mkdirSync,
readdirSync,
readFileSync,
readlinkSync,
renameSync,
rmSync,
statSync,
symlinkSync,
writeFileSync,
} from "node:fs";
import { copyFile, rename, rm } from "node:fs/promises";
import { basename, dirname, join, relative } from "node:path";
import { fsyncPath } from "./download/index.js";
import { safeExtension, sanitizeFileName } from "./filename.js";
import type { Collection, EnteFile } from "./model/types.js";
@@ -154,8 +155,12 @@ const errorMessage = (err: unknown): string =>
err instanceof Error ? err.message : String(err);
// Copy bytes into `dest` via a temp file in the same directory plus rename, so
// `dest` appears only once it is whole ("present means complete").
const copyAtomic = (src: string, dest: string): void => {
// `dest` appears only once it is whole ("present means complete"). As in the
// download writer, the temp file is fsynced before the rename and the directory
// after it, so a power cut cannot leave a correctly named but short original.
// The temp name carries this process's ID so a later run can tell a leftover
// from a copy still in progress (see `removeLeftoverTempFiles`).
const copyAtomic = async (src: string, dest: string): Promise<void> => {
if (src === dest) return;
const tmp = join(
dirname(dest),
@@ -164,10 +169,45 @@ const copyAtomic = (src: string, dest: string): void => {
.slice(2)}.tmp`,
);
try {
copyFileSync(src, tmp);
renameSync(tmp, dest);
await copyFile(src, tmp);
await fsyncPath(tmp);
// `rename` replaces the destination's directory entry: an existing
// symlink at `dest` is replaced, not followed, and the new file has
// the temp file's permissions (copied from `src`).
await rename(tmp, dest);
await fsyncPath(dirname(dest));
} finally {
rmSync(tmp, { force: true });
await rm(tmp, { force: true });
}
};
// A process-ID check: signal 0 delivers nothing and only reports whether the
// process exists. EPERM means it exists but belongs to another user.
const isRunning = (pid: number): boolean => {
try {
process.kill(pid, 0);
return true;
} catch (err) {
return (err as NodeJS.ErrnoException).code === "EPERM";
}
};
// Delete the temp files `copyAtomic` leaves behind when a backup is killed
// before its rename. Only files whose process is no longer running are
// removed, so a backup running at the same time keeps its own. A reused
// process ID can only keep a leftover a while longer, never remove a live one.
const removeLeftoverTempFiles = (dir: string): void => {
let names: string[];
try {
names = readdirSync(dir);
} catch {
return;
}
for (const name of names) {
const match = /^\.quak-backup-.*-(\d+)-[0-9a-z]*\.tmp$/.exec(name);
if (match && !isRunning(Number(match[1]))) {
rmSync(join(dir, name), { force: true });
}
}
};
@@ -254,6 +294,8 @@ export const runBackup = async (
mkdirSync(originalsDir, { recursive: true });
mkdirSync(collectionsDir, { recursive: true });
if (includeThumbnails) mkdirSync(thumbnailsDir, { recursive: true });
removeLeftoverTempFiles(originalsDir);
removeLeftoverTempFiles(thumbnailsDir);
const ledgerPath = join(downloadDirectory, "failures.json");
const ledger = loadLedger(ledgerPath);
@@ -321,7 +363,7 @@ export const runBackup = async (
try {
log(`Fetching original ${file.metadata.title} (${fileID})...`);
const { path } = await lib.original(fileID);
copyAtomic(path, dest);
await copyAtomic(path, dest);
downloaded++;
} catch (err) {
log(
@@ -342,7 +384,7 @@ export const runBackup = async (
if (isPresent(dest)) continue;
try {
const { path } = await lib.thumbnail(fileID);
copyAtomic(path, dest);
await copyAtomic(path, dest);
} catch (err) {
recordFailure(
file,
+17 -6
View File
@@ -158,6 +158,18 @@ const streamDecrypt = async (
return totalPlain;
};
// Fsync a file or a directory, so its contents (for a directory, its entries)
// are on stable storage. Exported for the backup tree's copy, which needs the
// same durability as the writer below.
export const fsyncPath = async (path: string): Promise<void> => {
const handle = await open(path, "r");
try {
await handle.sync();
} finally {
await handle.close();
}
};
// Stage a write to `destination` atomically and durably, then rename it into
// place. `fill` writes the contents into the open temp file handle — either the
// whole buffer at once (`writeAtomic`) or chunk by chunk as they decrypt
@@ -193,16 +205,15 @@ const stageAtomic = async (
} finally {
await handle.close();
}
// `rename` replaces the destination's directory entry rather than
// writing through it: an existing symlink at `destination` is
// replaced, not followed, and the new file has the temp file's
// permissions, not those of the file it replaced.
await rename(tmpPath, destination);
// Fsync the directory so the rename itself survives a crash: renaming
// over a synced temp file still leaves the new directory entry in the
// page cache until the directory is synced.
const dirHandle = await open(dir, "r");
try {
await dirHandle.sync();
} finally {
await dirHandle.close();
}
await fsyncPath(dir);
} catch (err) {
// Best-effort cleanup. A failure to remove the temporary file must
// never replace the error that actually explains what went wrong.