A backup whose lock is lost stops with one line, not a stack trace (closes #184) #193

Merged
clawbot merged 4 commits from issue-184-lost-lock into next 2026-10-14 07:37:27 +02:00
7 changed files with 313 additions and 50 deletions
+32 -19
View File
@@ -761,9 +761,9 @@ drops it.
`backup.lock` keeps two backups of the same directory from running at once, such
as a cron run that starts while the previous one is still going. A backup
creates `<dir>` if it is missing and takes the lock before its refresh, and
removes the lock when it ends, whether it succeeds or fails; `quak backup` takes
it before it opens its library, so a refused run sends no request. The lock is a
directory that
removes the lock when it ends, whether it succeeds or fails, unless it lost the
lock (see below); `quak backup` takes it before it opens its library, so a
refused run sends no request. The lock is a directory that
[proper-lockfile](https://github.com/moxystudio/node-proper-lockfile) creates
and keeps touching while the backup runs. A second backup of the directory, from
another process or the same one, fails at once: `quak backup` prints
@@ -775,6 +775,16 @@ over, so nobody has to remove it. The lock is outside the date folders and
`collections/`, so it is never taken for an original, and the removal of old
album directories never touches it.
A running backup loses its lock when someone removes `backup.lock`, or when
another backup takes it over because the running one left it untouched for 10
seconds, as a stopped or stalled process does. The backup finds this out the
next time it goes to touch the lock, which it does every 5 seconds, and then
stops before it starts another file. The original it is downloading at that
moment is still stored. It leaves `failures.json` and `backup.lock` as they are,
since another backup may be using them. `quak backup` prints
`quak: lost the lock on <dir>: <reason>` as one line and exits 1, and
`lib.backup()` rejects with `lost the lock on <dir>: <reason>`.
A collection's directory and JSON are named after the collection, and a symlink
after the file's title, both with unsafe characters replaced. When two
collections would get the same name, or two symlinks in one collection the same
@@ -1060,21 +1070,24 @@ photos newest first). `lib.subscribe({ onChange })` delivers a `LibraryChange`
directory, and fails at once with an error whose `code` is `ELOCKED` while
another backup of it runs. A caller can instead take the lock itself, before
it opens its library, as `quak backup` does: `await lockBackupDirectory(dir)`
takes it, failing the same way, and returns the function that releases it, and
the caller passes `lockHeld: true` to the backup. It waits for a refresh as
`fresh()` does, puts every in-scope original not already at its save path
there as `photo.download()` does (and, with `includeThumbnails`, fetches
thumbnails) through the content cache, waits for an ML data fetch, and
rebuilds the on-disk backup tree with a durable failure ledger. Each file's
JSON holds its ML data and, for an image (for a live photo, its image), its
original's EXIF and XMP, with its dimensions for a JPEG only; a video's JSON
holds none of these three. A fetched original is written straight to its save
path and not into the cache, which then counts it as present; one the cache
already held is copied from there. `BackupOptions`: `downloadDirectory` (falls
back to the library's), `includeOriginals` (default `true`),
`includeThumbnails` (default `false`), `onlyAlbumNames`, `verify` (default
`false`), `onProgress`, and `lockHeld` (default `false`). See Backup layout
above for the tree it writes.
takes it, failing the same way, and returns a `BackupLock`, whose `release()`
removes the lock unless it was lost and whose `lost` holds why it was lost.
The caller passes it to the backup as `lock`, and releases it once the backup
ends. It waits for a refresh as `fresh()` does, puts every in-scope original
not already at its save path there as `photo.download()` does (and, with
`includeThumbnails`, fetches thumbnails) through the content cache, waits for
an ML data fetch, and rebuilds the on-disk backup tree with a durable failure
ledger. Each file's JSON holds its ML data and, for an image (for a live
photo, its image), its original's EXIF and XMP, with its dimensions for a JPEG
only; a video's JSON holds none of these three. A fetched original is written
straight to its save path and not into the cache, which then counts it as
present; one the cache already held is copied from there. `BackupOptions`:
`downloadDirectory` (falls back to the library's), `includeOriginals` (default
`true`), `includeThumbnails` (default `false`), `onlyAlbumNames`, `verify`
(default `false`), `onProgress`, and `lock`. See Backup layout above for the
tree it writes. Whether the backup took the lock or its caller did, once the
lock is lost the backup rejects with `lost the lock on <dir>: <reason>` before
it starts another file.
### Request pools
@@ -1134,7 +1147,7 @@ from a very old client, is stored unchecked.
- `src/library/mlsearch.ts`: `MLDataAPI`, `SimilarResult`
- `src/exif.ts`: `ExifTags`, `PhotoExif`
- `src/library/pools.ts`: `RequestPools`, `RequestPoolsOptions`, `BoundedPool`
- `src/backup.ts`: `BackupOptions`, `BackupResult`, `BackupError`
- `src/backup.ts`: `BackupLock`, `BackupOptions`, `BackupResult`, `BackupError`
- `src/client.ts`: `Client`, `LoginOptions`, `ClientSnapshot`
- `src/api/client.ts`: `ApiClient`, `ApiClientOptions`, `StreamOptions`
- `src/errors.ts`: `ApiError`, `TruncatedStreamError`
+10
View File
@@ -25,6 +25,16 @@ declares one.
# Completed Steps
- 2026-10-14: A backup whose lock is lost no longer dies with a stack trace
(issue 184). `lockBackupDirectory` gives proper-lockfile an `onCompromised`
that records why the lock was lost instead of throwing from its timer, and
returns the lock, which `quak backup` passes to `lib.backup()` as `lock`. The
backup then stops before it starts another file with
`lost the lock on DIR: REASON`, which `quak backup` prints as one line before
it exits 1. Releasing a lost lock leaves `backup.lock` alone. Tests in
`test/cli/backup-end-to-end.test.ts` remove the lock, or put another backup's
in its place, while a download is held open.
- 2026-10-14: An album or file quak cannot decrypt, or whose metadata is not
JSON, no longer fails the refresh, so one bad record no longer stops
`quak backup` from storing anything (issue 181). The refresh leaves it out, an
+61 -21
View File
@@ -1,8 +1,9 @@
// The backup command, rebuilt on the library API (issue #51).
//
// `lib.backup()` takes the lock in `downloadDirectory` (unless its caller
// already holds it), and fails at once when another backup of it holds the
// lock. It waits for a completed refresh of the library (a failed one fails the
// passes one in), and fails at once when another backup of it holds the
// lock; when the lock is lost while it runs, it fails before it starts another
// file. It waits for a completed refresh of the library (a failed one fails the
// backup before any file is touched), then, for every file in scope, puts its
// original at its save path under `downloadDirectory`, as `Photo.download()`
// does, waits for an ML data fetch, and rebuilds the derived views (per-file
@@ -96,6 +97,14 @@ import type { Collection, EnteFile, FileMetadata } from "./model/types.js";
export type ProgressCallback = (message: string) => void;
// The lock in a backup directory, as `lockBackupDirectory` returns it.
export interface BackupLock {
// Removes `backup.lock`, unless the lock was lost.
release(): Promise<void>;
// Why the lock was lost, once it is.
lost?: Error;
}
export interface BackupOptions {
// Where the backup tree lives. `lib.backup()` defaults it to the library's
// download directory; `runBackup` with none throws before any network
@@ -112,10 +121,11 @@ export interface BackupOptions {
// bytes do not match the content hash its metadata records. Default false.
verify?: boolean;
onProgress?: ProgressCallback;
// The caller already holds the lock in `downloadDirectory`, taken with
// `lockBackupDirectory`, and releases it itself, so the backup does not
// take it. `quak backup` takes it before it opens its library.
lockHeld?: boolean;
// The lock in `downloadDirectory`, when the caller took it with
// `lockBackupDirectory` and releases it itself. The backup then does not
// take the lock, and fails once this one is lost. `quak backup` takes it
// before it opens its library.
lock?: BackupLock;
}
export interface BackupError {
@@ -510,11 +520,14 @@ const writeAlbumJSON = (
writeFileSync(path, JSON.stringify(album, null, 2));
};
// The backup itself, which `runBackup` below runs while the lock is held.
// The backup itself, which `runBackup` below runs while `lock` is held. Once
// the lock is lost, another backup may be writing to `downloadDirectory`, so
// this one fails before it starts another file.
const runLockedBackup = async (
lib: BackupLibrary,
opts: BackupOptions,
downloadDirectory: string,
lock: BackupLock,
): Promise<BackupResult> => {
const includeOriginals = opts.includeOriginals ?? true;
const includeThumbnails = opts.includeThumbnails ?? false;
@@ -524,6 +537,7 @@ const runLockedBackup = async (
log("Refreshing library...");
await lib.refresh();
if (lock.lost) throw lock.lost;
const collectionsDir = join(downloadDirectory, "collections");
const thumbnailsDir = join(downloadDirectory, "thumbnails");
@@ -642,6 +656,7 @@ const runLockedBackup = async (
// that cannot be read is recorded as failed and left where it is.
if (includeOriginals) {
for (const [fileID, file] of distinct) {
if (lock.lost) throw lock.lost;
let stored = storedAtSavePath(downloadDirectory, file);
let mismatch = false;
if (stored !== undefined && verify) {
@@ -721,6 +736,7 @@ const runLockedBackup = async (
if (includeThumbnails) {
for (const [fileID, file] of distinct) {
if (lock.lost) throw lock.lost;
const dest = join(thumbnailsDir, `${fileID}.jpg`);
if (isPresent(dest)) continue;
try {
@@ -753,6 +769,7 @@ const runLockedBackup = async (
log(`FAILED ML data: ${mlDataError}`);
}
for (const file of distinct.values()) {
if (lock.lost) throw lock.lost;
const stored = storedAtSavePath(downloadDirectory, file);
if (stored === undefined) continue;
const path = withExtension(
@@ -796,6 +813,7 @@ const runLockedBackup = async (
const albumDirNames = new Map(
allCollections.map((c, i) => [c.id, dirNames[i]!]),
);
if (lock.lost) throw lock.lost;
try {
removeStaleAlbumDirs(
collectionsDir,
@@ -807,6 +825,7 @@ const runLockedBackup = async (
}
for (const c of collections) {
if (lock.lost) throw lock.lost;
const colDirName = albumDirNames.get(c.id)!;
const colDir = join(collectionsDir, colDirName);
mkdirSync(colDir, { recursive: true });
@@ -864,7 +883,9 @@ const runLockedBackup = async (
// its failure resolved; a file gone from the library (deleted) or outside
// this run's scope is not something this run can resolve, so keeping its
// stale entry would keep the exit code non-zero forever — a single
// since-deleted photo would fail every future scheduled backup.
// since-deleted photo would fail every future scheduled backup. A backup
// whose lock was lost leaves the ledger as it is.
if (lock.lost) throw lock.lost;
for (const fileID of [...ledger.keys()]) {
if (!failedThisRun.has(fileID)) ledger.delete(fileID);
}
@@ -885,18 +906,36 @@ const runLockedBackup = async (
// Only one backup of a directory runs at a time, in this process or another.
// Creates `downloadDirectory` if it is missing, takes the lock in it and
// returns the function that releases it; while another backup holds the lock,
// fails at once with an error whose `code` is `ELOCKED`. The lock is the
// directory `backup.lock`, whose modification time proper-lockfile keeps
// current while it is held. One it has not touched for 10 seconds was left by
// a run that could not remove it, and is taken over.
// returns it; while another backup holds the lock, fails at once with an error
// whose `code` is `ELOCKED`. The lock is the directory `backup.lock`, whose
// modification time proper-lockfile keeps current while it is held. One it has
// not touched for 10 seconds was left by a run that could not remove it, and
// is taken over.
//
// proper-lockfile finds the lock lost when `backup.lock` is gone or is another
// backup's, and calls `onCompromised` from a timer, where throwing would end
// the process. The `onCompromised` given here sets the lock's `lost` instead,
// and the backup holding the lock stops before it starts another file (see
// `runLockedBackup`). Releasing a lost lock leaves `backup.lock` alone, since
// it may be another backup's.
export const lockBackupDirectory = async (
downloadDirectory: string,
): Promise<() => Promise<void>> => {
): Promise<BackupLock> => {
mkdirSync(downloadDirectory, { recursive: true });
let release: () => Promise<void>;
const lock: BackupLock = {
release: async () => {
if (lock.lost === undefined) await release();
},
};
try {
return await lockfile.lock(downloadDirectory, {
release = await lockfile.lock(downloadDirectory, {
lockfilePath: join(downloadDirectory, "backup.lock"),
onCompromised: (err) => {
lock.lost = new Error(
`lost the lock on ${downloadDirectory}: ${err.message}`,
);
},
});
} catch (err) {
if ((err as NodeJS.ErrnoException).code !== "ELOCKED") throw err;
@@ -905,10 +944,11 @@ export const lockBackupDirectory = async (
{ code: "ELOCKED" },
);
}
return lock;
};
// Runs the backup holding the lock, which it takes and releases itself unless
// the caller already holds it (`opts.lockHeld`).
// the caller passes the lock it took (`opts.lock`).
export const runBackup = async (
lib: BackupLibrary,
opts: BackupOptions,
@@ -920,13 +960,13 @@ export const runBackup = async (
"open the library with one)",
);
}
if (opts.lockHeld) {
return runLockedBackup(lib, opts, downloadDirectory);
if (opts.lock) {
return runLockedBackup(lib, opts, downloadDirectory, opts.lock);
}
const release = await lockBackupDirectory(downloadDirectory);
const lock = await lockBackupDirectory(downloadDirectory);
try {
return await runLockedBackup(lib, opts, downloadDirectory);
return await runLockedBackup(lib, opts, downloadDirectory, lock);
} finally {
await release();
await lock.release();
}
};
+5 -5
View File
@@ -20,7 +20,7 @@ import {
type ClientSnapshot,
type LoginOptions,
} from "./client.js";
import { lockBackupDirectory } from "./backup.js";
import { lockBackupDirectory, type BackupLock } from "./backup.js";
import { init } from "./crypto/index.js";
import {
defaultCacheDirectory,
@@ -416,9 +416,9 @@ export const backupCommand = async (
ctx.stderr.write("Starting backup...\n");
// The lock is taken before the library opens and starts its refresh, so a
// run refused while another backup of `dir` runs sends no request.
let release: () => Promise<void>;
let lock: BackupLock;
try {
release = await lockBackupDirectory(dir);
lock = await lockBackupDirectory(dir);
} catch (err) {
if ((err as NodeJS.ErrnoException).code !== "ELOCKED") throw err;
ctx.stderr.write(`quak: ${(err as Error).message}\n`);
@@ -440,7 +440,7 @@ export const backupCommand = async (
try {
const result = await lib.backup({
downloadDirectory: dir,
lockHeld: true,
lock,
verify: opts.verify,
onProgress: (msg) => {
if (!opts.json) ctx.stderr.write(msg + "\n");
@@ -477,7 +477,7 @@ export const backupCommand = async (
await lib.close();
}
} finally {
await release();
await lock.release();
}
};
+1
View File
@@ -69,6 +69,7 @@ export {
type EnsureEvent,
lockBackupDirectory,
runBackup,
type BackupLock,
type BackupOptions,
type BackupResult,
type BackupError,
+5 -3
View File
@@ -96,6 +96,7 @@ import { runBackup, type BackupOptions, type BackupResult } from "../backup.js";
export {
lockBackupDirectory,
runBackup,
type BackupLock,
type BackupOptions,
type BackupResult,
type BackupError,
@@ -570,9 +571,10 @@ export class Library {
// Back up every in-scope file to `opts.downloadDirectory`, or else the
// library's, each original at its save path, with a durable failure
// ledger (issue #51). Takes the lock in that directory first, unless
// `opts.lockHeld` says the caller holds it, failing at once while another
// backup of it runs (see `runBackup`). Waits for a
// ledger (issue #51). Takes the lock in that directory first, unless the
// caller passes the lock it took as `opts.lock`, failing at once while
// another backup of it runs, and before it starts another file once the
// lock is lost (see `lockBackupDirectory`). Waits for a
// completed refresh, as `fresh()` does, joining one already running, and
// rejects before touching any file but the lock when it fails. Then puts
// pending originals at their save paths as `Photo.download()` does (and
+199 -2
View File
@@ -9,7 +9,8 @@
* data this file encrypts with libsodium, as Ente's clients encrypt it. So
* quak's own code fetches each original, decrypts it, checks it against the
* hash its uploader recorded and unpacks a live photo, and quak's own retries
* repeat a failed request.
* repeat a failed request. Two tests run `lib.backup()` instead, as a program
* using quak as a library does.
*
* The account, user 42, has three albums:
*
@@ -24,6 +25,7 @@
import {
existsSync,
lstatSync,
mkdirSync,
mkdtempSync,
readdirSync,
readFileSync,
@@ -36,7 +38,16 @@ import { join } from "node:path";
import { Writable } from "node:stream";
import { gzipSync } from "node:zlib";
import sodium from "libsodium-wrappers-sumo";
import { afterAll, beforeAll, describe, expect, it } from "vitest";
import {
afterAll,
afterEach,
beforeAll,
beforeEach,
describe,
expect,
it,
vi,
} from "vitest";
import {
backupCommand,
@@ -47,6 +58,7 @@ import { run } from "../../src/cli-run.js";
import { loadSession } from "../../src/cli-session.js";
import { Client } from "../../src/client.js";
import { init, toBase64 } from "../../src/crypto/index.js";
import { Library, lockBackupDirectory } from "../../src/library/index.js";
import { UNATTENDED_RETRY_OPTIONS } from "../../src/retry.js";
import {
buildAccount,
@@ -633,3 +645,188 @@ describe("quak backup", () => {
expect(stderr).toContain(" Verified: 2\n Mismatched: 1\n");
});
});
// `backup.lock` is a directory that proper-lockfile touches every 5 seconds,
// from a timer, while the backup runs. Each time, it first looks at the lock
// on disk. The lock is lost when it is gone, or when another backup has made
// its own in its place, as one does with a lock nobody touched for 10
// seconds. Another backup may then be writing to the directory, so the backup
// stops before it starts another file.
//
// The fake server holds one file's download open until the test lets it go,
// so the lock can be lost while the backup runs: beach.jpg's, the first file
// backed up, unless the test names another. Only `setTimeout` and
// `clearTimeout` are faked, so a test can fire proper-lockfile's timer without
// waiting 5 seconds.
describe("a backup whose lock is lost", () => {
let dir: string;
let lockPath: string;
let downloads: number[];
let heldFile: number;
let heldRequested: Promise<void>;
let releaseHeld: () => void;
let fetch: typeof globalThis.fetch;
beforeEach(() => {
dir = mkdtempSync(join(root, "backup-"));
lockPath = join(dir, "backup.lock");
downloads = [];
heldFile = BEACH.id;
let requested!: () => void;
heldRequested = new Promise((resolve) => {
requested = resolve;
});
const released = new Promise<void>((resolve) => {
releaseHeld = resolve;
});
const recording = recordingDownloads(downloads);
fetch = async (input, init) => {
if (downloadOf(input) === heldFile) {
requested();
await released;
}
return recording(input, init);
};
vi.useFakeTimers({ toFake: ["setTimeout", "clearTimeout"] });
});
afterEach(() => {
vi.useRealTimers();
});
// Fire proper-lockfile's timer, and wait for the look at the lock on disk
// that it starts to finish: Node lists a file system call in progress as
// an `FSReqCallback`.
const fireLockTimer = async (): Promise<void> => {
vi.advanceTimersByTime(5_000);
while (process.getActiveResourcesInfo().includes("FSReqCallback")) {
await new Promise((resolve) => setImmediate(resolve));
}
};
// beach.jpg, whose download was under way, is stored; no other file is
// started. `quak backup` prints why as one line, with no stack trace, and
// exits 1.
it("stops quak backup before its next file when backup.lock is removed, and exits 1", async () => {
const result = backup(dir, {}, fetch);
await heldRequested;
rmSync(lockPath, { recursive: true });
await fireLockTimer();
releaseHeld();
const { code, stderr } = await result;
expect(code).toBe(1);
expect(stderr).toBe(
"Starting backup...\n" +
"Refreshing library...\n" +
"Fetching original beach.jpg (100)...\n" +
`quak: lost the lock on ${dir}: ENOENT: no such file or directory, stat '${lockPath}'\n`,
);
expect(downloads).toEqual([BEACH.id]);
expect(readFileSync(join(dir, DAY, "2024-03-01.100.jpg"))).toEqual(
Buffer.from(JPEG_WITH_EXIF),
);
});
// Here the lock is lost while toast.mp4, the last original, downloads.
// All three originals are stored, beside what the backup wrote before it
// started them, but no file's JSON and no album is written.
it("stops quak backup before any file's JSON when the lock is lost during the last original", async () => {
heldFile = TOAST_VIDEO.id;
const result = backup(dir, {}, fetch);
await heldRequested;
rmSync(lockPath, { recursive: true });
await fireLockTimer();
releaseHeld();
const { code, stderr } = await result;
expect(code).toBe(1);
expect(stderr).toContain(`quak: lost the lock on ${dir}: `);
expect(listTree(dir)).toEqual([
"2024",
"2024/2024-03",
"2024/2024-03/2024-03-01",
"2024/2024-03/2024-03-01/2024-03-01.100.jpg",
"2024/2024-03/2024-03-01/2024-03-01.200.mp4",
"2024/2024-03/2024-03-01/2024-03-01.300.heic",
"2024/2024-03/2024-03-01/2024-03-01.300.livephoto.json",
"2024/2024-03/2024-03-01/2024-03-01.300.mov",
"account.json",
"collections",
]);
});
// Nothing touches the lock but quak, which finds it as it left it each
// time it looks, so the backup runs to the end and removes its lock.
it("lets quak backup run to the end while its lock stays", async () => {
const result = backup(dir, {}, fetch);
await heldRequested;
await fireLockTimer();
await fireLockTimer();
releaseHeld();
const { code } = await result;
expect(code).toBe(0);
expect(downloads.sort()).toEqual([100, 200, 300]);
expect(existsSync(lockPath)).toBe(false);
});
// The library a program using quak opens to back `dir` up.
const openLibrary = () =>
Library.open({
client: loadSession(join(sessionDir, "session.json"), { fetch })!,
cacheDirectory: `${dir}-cache`,
downloadDirectory: dir,
refreshIntervalSeconds: Infinity,
precacheThumbnails: false,
precacheOriginals: false,
});
// A program using quak as a library sees the same: `lib.backup()` rejects
// with the reason. Here another backup has made its own lock in place of
// quak's, and that lock is left where it is.
it("rejects lib.backup() when another backup has taken the lock over", async () => {
const lib = await openLibrary();
try {
const result = lib.backup();
await heldRequested;
rmSync(lockPath, { recursive: true });
mkdirSync(lockPath);
await fireLockTimer();
releaseHeld();
await expect(result).rejects.toThrow(
`lost the lock on ${dir}: Unable to update lock within the stale threshold`,
);
expect(downloads).toEqual([BEACH.id]);
expect(existsSync(lockPath)).toBe(true);
} finally {
await lib.close();
}
});
// A program can take the lock before it opens its library, as `quak
// backup` does: `lockBackupDirectory` returns the lock, which the program
// passes to `lib.backup()` and releases once the backup ends. The backup
// checks it as it checks a lock it took, and the lock's `lost` says why it
// was lost.
it("rejects lib.backup() given its caller's lock once that lock is lost", async () => {
const lock = await lockBackupDirectory(dir);
const lib = await openLibrary();
try {
const result = lib.backup({ lock });
await heldRequested;
rmSync(lockPath, { recursive: true });
await fireLockTimer();
releaseHeld();
const lost = `lost the lock on ${dir}: ENOENT: no such file or directory, stat '${lockPath}'`;
await expect(result).rejects.toThrow(lost);
expect(lock.lost?.message).toBe(lost);
expect(downloads).toEqual([BEACH.id]);
} finally {
await lib.close();
await lock.release();
}
});
});