A backup whose lock is lost stops with one line, not a stack trace (closes #184)
lockBackupDirectory now passes proper-lockfile an onCompromised that records why the lock was lost, where the default threw from a timer and ended the process. The backup checks its lock before each original and before it writes failures.json, and fails with "lost the lock on DIR: REASON", which quak backup prints through run as one line before exiting 1. Releasing a lost lock leaves backup.lock alone, as it may be another backup's. The lock of each directory is kept by path so that a backup run with lockHeld, as quak backup runs it, finds the lock its caller took. Model: opus-5-5
This commit is contained in:
@@ -761,9 +761,9 @@ drops it.
|
|||||||
`backup.lock` keeps two backups of the same directory from running at once, such
|
`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
|
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
|
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
|
removes the lock when it ends, whether it succeeds or fails, unless it lost the
|
||||||
it before it opens its library, so a refused run sends no request. The lock is a
|
lock (see below); `quak backup` takes it before it opens its library, so a
|
||||||
directory that
|
refused run sends no request. The lock is a directory that
|
||||||
[proper-lockfile](https://github.com/moxystudio/node-proper-lockfile) creates
|
[proper-lockfile](https://github.com/moxystudio/node-proper-lockfile) creates
|
||||||
and keeps touching while the backup runs. A second backup of the directory, from
|
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
|
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
|
`collections/`, so it is never taken for an original, and the removal of old
|
||||||
album directories never touches it.
|
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
|
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
|
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
|
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`),
|
back to the library's), `includeOriginals` (default `true`),
|
||||||
`includeThumbnails` (default `false`), `onlyAlbumNames`, `verify` (default
|
`includeThumbnails` (default `false`), `onlyAlbumNames`, `verify` (default
|
||||||
`false`), `onProgress`, and `lockHeld` (default `false`). See Backup layout
|
`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 <dir>: <reason>` before it starts another file.
|
||||||
|
|
||||||
### Request pools
|
### Request pools
|
||||||
|
|
||||||
|
|||||||
@@ -25,6 +25,15 @@ declares one.
|
|||||||
|
|
||||||
# Completed Steps
|
# 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
|
- 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
|
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
|
`quak backup` from storing anything (issue 181). The refresh leaves it out, an
|
||||||
|
|||||||
+37
-4
@@ -2,7 +2,8 @@
|
|||||||
//
|
//
|
||||||
// `lib.backup()` takes the lock in `downloadDirectory` (unless its caller
|
// `lib.backup()` takes the lock in `downloadDirectory` (unless its caller
|
||||||
// already holds it), and fails at once when another backup of it holds the
|
// 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
|
// backup before any file is touched), then, for every file in scope, puts its
|
||||||
// original at its save path under `downloadDirectory`, as `Photo.download()`
|
// original at its save path under `downloadDirectory`, as `Photo.download()`
|
||||||
// does, waits for an ML data fetch, and rebuilds the derived views (per-file
|
// does, waits for an ML data fetch, and rebuilds the derived views (per-file
|
||||||
@@ -114,7 +115,8 @@ export interface BackupOptions {
|
|||||||
onProgress?: ProgressCallback;
|
onProgress?: ProgressCallback;
|
||||||
// The caller already holds the lock in `downloadDirectory`, taken with
|
// The caller already holds the lock in `downloadDirectory`, taken with
|
||||||
// `lockBackupDirectory`, and releases it itself, so the backup does not
|
// `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;
|
lockHeld?: boolean;
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -521,6 +523,9 @@ const runLockedBackup = async (
|
|||||||
const log = opts.onProgress ?? (() => {});
|
const log = opts.onProgress ?? (() => {});
|
||||||
const only = opts.onlyAlbumNames ? new Set(opts.onlyAlbumNames) : undefined;
|
const only = opts.onlyAlbumNames ? new Set(opts.onlyAlbumNames) : undefined;
|
||||||
const verify = opts.verify ?? false;
|
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...");
|
log("Refreshing library...");
|
||||||
await lib.refresh();
|
await lib.refresh();
|
||||||
@@ -642,6 +647,7 @@ const runLockedBackup = async (
|
|||||||
// that cannot be read is recorded as failed and left where it is.
|
// that cannot be read is recorded as failed and left where it is.
|
||||||
if (includeOriginals) {
|
if (includeOriginals) {
|
||||||
for (const [fileID, file] of distinct) {
|
for (const [fileID, file] of distinct) {
|
||||||
|
if (lock?.lost) throw lock.lost;
|
||||||
let stored = storedAtSavePath(downloadDirectory, file);
|
let stored = storedAtSavePath(downloadDirectory, file);
|
||||||
let mismatch = false;
|
let mismatch = false;
|
||||||
if (stored !== undefined && verify) {
|
if (stored !== undefined && verify) {
|
||||||
@@ -864,7 +870,9 @@ const runLockedBackup = async (
|
|||||||
// its failure resolved; a file gone from the library (deleted) or outside
|
// 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
|
// 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
|
// 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()]) {
|
for (const fileID of [...ledger.keys()]) {
|
||||||
if (!failedThisRun.has(fileID)) ledger.delete(fileID);
|
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<string, { lost?: Error }>();
|
||||||
|
|
||||||
// Only one backup of a directory runs at a time, in this process or another.
|
// 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
|
// Creates `downloadDirectory` if it is missing, takes the lock in it and
|
||||||
// returns the function that releases it; while another backup holds the lock,
|
// 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
|
// 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
|
// 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.
|
// 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 (
|
export const lockBackupDirectory = async (
|
||||||
downloadDirectory: string,
|
downloadDirectory: string,
|
||||||
): Promise<() => Promise<void>> => {
|
): Promise<() => Promise<void>> => {
|
||||||
mkdirSync(downloadDirectory, { recursive: true });
|
mkdirSync(downloadDirectory, { recursive: true });
|
||||||
|
const lock: { lost?: Error } = {};
|
||||||
|
let release: () => Promise<void>;
|
||||||
try {
|
try {
|
||||||
return await lockfile.lock(downloadDirectory, {
|
release = await lockfile.lock(downloadDirectory, {
|
||||||
lockfilePath: join(downloadDirectory, "backup.lock"),
|
lockfilePath: join(downloadDirectory, "backup.lock"),
|
||||||
|
onCompromised: (err) => {
|
||||||
|
lock.lost = new Error(
|
||||||
|
`lost the lock on ${downloadDirectory}: ${err.message}`,
|
||||||
|
);
|
||||||
|
},
|
||||||
});
|
});
|
||||||
} catch (err) {
|
} catch (err) {
|
||||||
if ((err as NodeJS.ErrnoException).code !== "ELOCKED") throw err;
|
if ((err as NodeJS.ErrnoException).code !== "ELOCKED") throw err;
|
||||||
@@ -905,6 +930,14 @@ export const lockBackupDirectory = async (
|
|||||||
{ code: "ELOCKED" },
|
{ 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
|
// Runs the backup holding the lock, which it takes and releases itself unless
|
||||||
|
|||||||
@@ -572,7 +572,8 @@ export class Library {
|
|||||||
// library's, each original at its save path, with a durable failure
|
// library's, each original at its save path, with a durable failure
|
||||||
// ledger (issue #51). Takes the lock in that directory first, unless
|
// ledger (issue #51). Takes the lock in that directory first, unless
|
||||||
// `opts.lockHeld` says the caller holds it, failing at once while another
|
// `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
|
// completed refresh, as `fresh()` does, joining one already running, and
|
||||||
// rejects before touching any file but the lock when it fails. Then puts
|
// rejects before touching any file but the lock when it fails. Then puts
|
||||||
// pending originals at their save paths as `Photo.download()` does (and
|
// pending originals at their save paths as `Photo.download()` does (and
|
||||||
|
|||||||
Reference in New Issue
Block a user