diff --git a/README.md b/README.md index 0a7e7a3..0095718 100644 --- a/README.md +++ b/README.md @@ -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 `` 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 : ` as one line and exits 1, and +`lib.backup()` rejects with `lost the lock on : `. + 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 @@ -1074,7 +1084,9 @@ photos newest first). `lib.subscribe({ onChange })` delivers a `LibraryChange` 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. + 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 : ` before it starts another file. ### Request pools diff --git a/TODO.md b/TODO.md index e170323..7013286 100644 --- a/TODO.md +++ b/TODO.md @@ -25,6 +25,15 @@ 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. The + backup then stops before it starts another original, and before it writes + `failures.json`, 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 diff --git a/src/backup.ts b/src/backup.ts index 5deae14..5eb88ed 100644 --- a/src/backup.ts +++ b/src/backup.ts @@ -2,7 +2,8 @@ // // `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 +// 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 @@ -114,7 +115,8 @@ export interface BackupOptions { 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. + // take it, but still fails once that lock is lost. `quak backup` takes it + // before it opens its library. lockHeld?: boolean; } @@ -521,6 +523,9 @@ const runLockedBackup = async ( const log = opts.onProgress ?? (() => {}); const only = opts.onlyAlbumNames ? new Set(opts.onlyAlbumNames) : undefined; const verify = opts.verify ?? false; + // The lock `lockBackupDirectory` took for this backup. Once it is lost, + // another backup may be writing to `downloadDirectory`. + const lock = heldLocks.get(resolve(downloadDirectory)); log("Refreshing library..."); await lib.refresh(); @@ -642,6 +647,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) { @@ -864,7 +870,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); } @@ -883,6 +891,10 @@ const runLockedBackup = async ( }; }; +// The lock this process holds on each backup directory, by the directory's +// absolute path, until it is released. `lost` is set once the lock is lost. +const heldLocks = new Map(); + // 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, @@ -890,13 +902,26 @@ const runLockedBackup = async ( // 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 backup holding the lock stops before it starts another file +// instead (see `runLockedBackup`), and releasing a lost lock leaves +// `backup.lock` alone. export const lockBackupDirectory = async ( downloadDirectory: string, ): Promise<() => Promise> => { mkdirSync(downloadDirectory, { recursive: true }); + const lock: { lost?: Error } = {}; + let release: () => Promise; 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,6 +930,14 @@ export const lockBackupDirectory = async ( { code: "ELOCKED" }, ); } + const path = resolve(downloadDirectory); + heldLocks.set(path, lock); + return async () => { + // Once this lock was lost, another backup in this process may have + // taken the directory. + if (heldLocks.get(path) === lock) heldLocks.delete(path); + if (lock.lost === undefined) await release(); + }; }; // Runs the backup holding the lock, which it takes and releases itself unless diff --git a/src/library/index.ts b/src/library/index.ts index 443f4d4..294f56f 100644 --- a/src/library/index.ts +++ b/src/library/index.ts @@ -572,7 +572,8 @@ export class Library { // 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 + // 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