diff --git a/README.md b/README.md index bf1d9ac..f288dfe 100644 --- a/README.md +++ b/README.md @@ -524,10 +524,10 @@ stack trace. All three exit with status 3, which means the user must run `quak login` again. `quak logout` is the exception: with no file it says there is no session and exits 0, and it handles a corrupt file or a failed server call as described below. `quak backup` meets an expired session on the refresh that -starts every run, before it touches any file. A session that stops working -partway through a backup instead fails each remaining file into `failures.json`, -so that run exits 1 and the next one stops at its refresh with status 3. No -command but `quak login` ever prompts. +starts every run, before it touches any file but its lock (see "Backup layout"). +A session that stops working partway through a backup instead fails each +remaining file into `failures.json`, so that run exits 1 and the next one stops +at its refresh with status 3. No command but `quak login` ever prompts. `quak logout` ends the session on the server, so the token in `session.json` stops working even in a copy of the file, and then deletes the file. If the @@ -620,6 +620,7 @@ the smallest does not. (symlink) .json collection metadata + file list account.json the account's email and user ID + backup.lock the lock a running backup holds (see below) failures.json files that failed and have not yet succeeded ``` @@ -654,6 +655,22 @@ succeeds, or once it is no longer in the library or in the backup's scope. The library's `lib.backup({ includeThumbnails: true })` also writes `thumbnails/.jpg` beside `collections/`; `quak backup` does not. +`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. 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 +`quak: another backup of is running` and exits with status 2. It opens its +library before the backup takes the lock, so a refused run still waits for the +refresh that opening the library starts before it exits. A run that is killed +leaves the lock behind; once it has gone 10 seconds untouched, the next run +takes it 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 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 @@ -901,17 +918,19 @@ photos newest first). `lib.subscribe({ onChange })` delivers a `LibraryChange` `SimilarResult[]` (`{ fileID, score }`, cosine similarity, most similar first, default limit 20). quak bundles no text encoder, so `searchByEmbedding` takes a query vector the caller produced elsewhere. -- `await lib.backup(opts?)` → `BackupResult`. 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, each file's JSON with its ML data, with a - durable failure ledger. 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`, and `onProgress`. See - Backup layout above for the tree it writes. +- `await lib.backup(opts?)` → `BackupResult`. It takes the lock in the download + directory, and fails at once with an error whose `code` is `ELOCKED` while + another backup of it runs. 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, each file's JSON with its ML data, with a durable failure + ledger. 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`, and `onProgress`. See Backup layout above for the + tree it writes. ### Request pools diff --git a/TODO.md b/TODO.md index 3fb0506..c059e89 100644 --- a/TODO.md +++ b/TODO.md @@ -25,6 +25,15 @@ declares one. # Completed Steps +- 2026-10-06: Two backups of the same directory never run at once (issue 169). + `lib.backup()` takes a lock, `backup.lock` in its download directory, made + with `proper-lockfile`, before its refresh, and removes it when it ends, + whether it succeeds or fails. A second backup of the directory, from another + process or the same one, fails at once with an error naming the directory; + `quak backup` prints it as one line and exits 2. A lock that has gone 10 + seconds untouched, left by a run that was killed, is taken over by the next + run. + - 2026-10-06: When the server answers HTTP 401 and that ends a command that loads the saved session, because the server no longer accepts its token, the command prints one line, diff --git a/package.json b/package.json index 779a239..63f26ca 100644 --- a/package.json +++ b/package.json @@ -36,6 +36,7 @@ "devDependencies": { "@eslint/js": "9.38.0", "@types/node": "22.18.13", + "@types/proper-lockfile": "4.1.4", "eslint": "9.38.0", "prettier": "3.8.1", "typescript": "5.9.3", @@ -50,6 +51,7 @@ "fast-srp-hap": "2.0.4", "fflate": "0.8.3", "jpeg-js": "0.4.4", - "libsodium-wrappers-sumo": "0.8.4" + "libsodium-wrappers-sumo": "0.8.4", + "proper-lockfile": "4.1.2" } } diff --git a/src/backup.ts b/src/backup.ts index bfe1dd4..1d9fdec 100644 --- a/src/backup.ts +++ b/src/backup.ts @@ -1,11 +1,12 @@ // The backup command, rebuilt on the library API (issue #51). // -// `lib.backup()` 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 sidecars, per-collection symlink trees, per-collection JSON) -// from the model. The on-disk layout: +// `lib.backup()` takes the lock in `downloadDirectory`, 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 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 sidecars, per-collection symlink +// trees, per-collection JSON) from the model. The on-disk layout: // // / // YYYY/YYYY-MM/YYYY-MM-DD/ @@ -15,6 +16,7 @@ // collections// symlink to the original // collections/<name>.json per-collection metadata // account.json the account's email and user ID +// backup.lock the lock, while a backup runs // failures.json durable ledger of unresolved failures // // A live photo's original is its image and its video, each with its own @@ -54,6 +56,7 @@ import { writeFileSync, } from "node:fs"; import { dirname, extname, join, relative, resolve } from "node:path"; +import lockfile from "proper-lockfile"; import { removeLeftoverTempFiles } from "./download/index.js"; import { sanitizeFileName, withExtension } from "./filename.js"; @@ -398,17 +401,12 @@ const writeAlbumJSON = ( writeFileSync(path, JSON.stringify(album, null, 2)); }; -export const runBackup = async ( +// The backup itself, which `runBackup` below runs while it holds the lock. +const runLockedBackup = async ( lib: BackupLibrary, opts: BackupOptions, + downloadDirectory: string, ): Promise<BackupResult> => { - const downloadDirectory = opts.downloadDirectory; - if (!downloadDirectory) { - throw new Error( - "backup requires a downloadDirectory (pass one to backup() or " + - "open the library with one)", - ); - } const includeOriginals = opts.includeOriginals ?? true; const includeThumbnails = opts.includeThumbnails ?? false; const log = opts.onProgress ?? (() => {}); @@ -676,3 +674,39 @@ export const runBackup = async ( errors, }; }; + +// Only one backup of a directory runs at a time, in this process or another: +// a second one 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 the backup runs. One it has not touched for 10 seconds +// was left by a run that was killed, and is taken over. +export const runBackup = async ( + lib: BackupLibrary, + opts: BackupOptions, +): Promise<BackupResult> => { + const downloadDirectory = opts.downloadDirectory; + if (!downloadDirectory) { + throw new Error( + "backup requires a downloadDirectory (pass one to backup() or " + + "open the library with one)", + ); + } + mkdirSync(downloadDirectory, { recursive: true }); + let release: () => Promise<void>; + try { + release = await lockfile.lock(downloadDirectory, { + lockfilePath: join(downloadDirectory, "backup.lock"), + }); + } catch (err) { + if ((err as NodeJS.ErrnoException).code !== "ELOCKED") throw err; + throw Object.assign( + new Error(`another backup of ${downloadDirectory} is running`), + { code: "ELOCKED" }, + ); + } + try { + return await runLockedBackup(lib, opts, downloadDirectory); + } finally { + await release(); + } +}; diff --git a/src/cli-commands.ts b/src/cli-commands.ts index 30aa4ee..fba91bb 100644 --- a/src/cli-commands.ts +++ b/src/cli-commands.ts @@ -22,6 +22,7 @@ import { } from "./client.js"; import { init } from "./crypto/index.js"; import { + type BackupResult, defaultCacheDirectory, Library, type LibraryClient, @@ -418,12 +419,20 @@ export const backupCommand = async ( precacheOriginals: false, }); try { - const result = await lib.backup({ - downloadDirectory: dir, - onProgress: (msg) => { - if (!opts.json) ctx.stderr.write(msg + "\n"); - }, - }); + let result: BackupResult; + try { + result = await lib.backup({ + downloadDirectory: dir, + onProgress: (msg) => { + if (!opts.json) ctx.stderr.write(msg + "\n"); + }, + }); + } catch (err) { + // Another backup of `dir` is running and holds its lock. + if ((err as NodeJS.ErrnoException).code !== "ELOCKED") throw err; + ctx.stderr.write(`quak: ${(err as Error).message}\n`); + return 2; + } if (opts.json) { ctx.stdout.write(JSON.stringify(result, null, 2) + "\n"); diff --git a/src/library/index.ts b/src/library/index.ts index 65a9daa..2b14a4d 100644 --- a/src/library/index.ts +++ b/src/library/index.ts @@ -566,13 +566,15 @@ 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). Waits for a completed refresh first, as `fresh()` - // does, joining one already running, and rejects before touching any file - // when it fails. Then puts pending originals at their save paths as - // `Photo.download()` does (and optional thumbnails) through the content - // cache and pools, waits for an ML data fetch, and rebuilds the derived - // symlink/JSON views from the model, each file's JSON with its ML data, - // beside an `account.json` with the account's email and user ID. + // ledger (issue #51). Takes the lock in that directory first, failing at + // once while another backup of it runs (see `runBackup`). 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 + // optional thumbnails) through the content cache and pools, waits for an + // ML data fetch, and rebuilds the derived symlink/JSON views from the + // model, each file's JSON with its ML data, beside an `account.json` with + // the account's email and user ID. // Throws before any network work when no content cache backs the // originals it must fetch. backup(opts?: BackupOptions): Promise<BackupResult> { diff --git a/test/cli/backup.test.ts b/test/cli/backup.test.ts index 5424242..b00e955 100644 --- a/test/cli/backup.test.ts +++ b/test/cli/backup.test.ts @@ -42,6 +42,7 @@ import { readlinkSync, rmSync, symlinkSync, + utimesSync, writeFileSync, } from "node:fs"; import { spawnSync } from "node:child_process"; @@ -825,7 +826,81 @@ describe("the refresh before a backup", () => { ); expect(source.originalCalls).toBe(0); - expect(existsSync(outDir)).toBe(false); + // The backup made the directory for its lock, and removed the lock. + expect(readdirSync(outDir)).toEqual([]); + await lib.close(); + }); +}); + +describe("the backup lock", () => { + const lockPath = (outDir: string): string => join(outDir, "backup.lock"); + + it("refuses a second backup of the directory while one runs", async () => { + await fillCache(); + const client = new HeldClient(); + const lib = await openLibrary(stubSource(), client); + const outDir = join(root, "backup"); + // The first backup holds the lock once it starts its refresh, which + // `HeldClient` keeps from finishing. + let refreshing!: () => void; + const started = new Promise<void>((resolve) => { + refreshing = resolve; + }); + const first = lib.backup({ + downloadDirectory: outDir, + onProgress: (msg) => { + if (msg === "Refreshing library...") refreshing(); + }, + }); + await started; + + await expect(lib.backup({ downloadDirectory: outDir })).rejects.toThrow( + `another backup of ${outDir} is running`, + ); + + client.release(); + expect((await first).failed).toBe(0); + await lib.close(); + }); + + it("releases the lock after a backup succeeds", async () => { + const lib = await openLibrary(stubSource()); + const outDir = join(root, "backup"); + + await lib.backup({ downloadDirectory: outDir }); + + expect(existsSync(lockPath(outDir))).toBe(false); + // So the next backup of the directory runs. + expect((await lib.backup({ downloadDirectory: outDir })).skipped).toBe( + 3, + ); + await lib.close(); + }); + + it("releases the lock after a backup fails", async () => { + const lib = await openLibrary(stubSource(), new FailingClient()); + const outDir = join(root, "backup"); + + await expect(lib.backup({ downloadDirectory: outDir })).rejects.toThrow( + "HTTP 401 from server", + ); + + expect(existsSync(lockPath(outDir))).toBe(false); + await lib.close(); + }); + + it("takes over a lock left by a run that was killed", async () => { + const outDir = join(root, "backup"); + // A killed run's lock, last touched a minute ago. + mkdirSync(lockPath(outDir), { recursive: true }); + const minuteAgo = new Date(Date.now() - 60_000); + utimesSync(lockPath(outDir), minuteAgo, minuteAgo); + const lib = await openLibrary(stubSource()); + + const result = await lib.backup({ downloadDirectory: outDir }); + + expect(result.downloaded).toBe(3); + expect(existsSync(lockPath(outDir))).toBe(false); await lib.close(); }); }); diff --git a/test/cli/commands.test.ts b/test/cli/commands.test.ts index 633cc35..1e6844d 100644 --- a/test/cli/commands.test.ts +++ b/test/cli/commands.test.ts @@ -18,6 +18,7 @@ import { readFileSync, rmSync, statSync, + utimesSync, writeFileSync, } from "node:fs"; import { join } from "node:path"; @@ -792,7 +793,7 @@ describe("backup", () => { expect(code).toBe(1); expect(runText).toBe("quak: HTTP 503 from server\n"); expect(stderr.text).toBe("Starting backup...\nRefreshing library...\n"); - expect(existsSync(dir)).toBe(false); + expect(readdirSync(dir)).toEqual([]); }); // A real saved session, read back by `loadSession`, whose server answers @@ -823,7 +824,23 @@ describe("backup", () => { `quak: the saved session is no longer valid; run "quak login"\n`, ); expect(stderr.text).toBe("Starting backup...\nRefreshing library...\n"); - expect(existsSync(dir)).toBe(false); + expect(readdirSync(dir)).toEqual([]); + }); + + it("exits 2 with one line naming the directory while another backup of it runs", async () => { + const dir = join(root, "backup"); + // The lock another backup holds. Its modification time is set an hour + // ahead, so it stays current however long this test takes. + const lock = join(dir, "backup.lock"); + mkdirSync(lock, { recursive: true }); + const hourAhead = new Date(Date.now() + 3_600_000); + utimesSync(lock, hourAhead, hourAhead); + + expect(await backupCommand(context(), dir, {})).toBe(2); + expect(stderr.text).toBe( + `Starting backup...\nquak: another backup of ${dir} is running\n`, + ); + expect(readdirSync(dir)).toEqual(["backup.lock"]); }); }); diff --git a/yarn.lock b/yarn.lock index 4386074..d49442b 100644 --- a/yarn.lock +++ b/yarn.lock @@ -535,6 +535,18 @@ dependencies: undici-types "~6.21.0" +"@types/proper-lockfile@4.1.4": + version "4.1.4" + resolved "https://registry.yarnpkg.com/@types/proper-lockfile/-/proper-lockfile-4.1.4.tgz#cd9fab92bdb04730c1ada542c356f03620f84008" + integrity sha512-uo2ABllncSqg9F1D4nugVl9v93RmjxF6LJzQLMLDdPaXCUIDPeOJ21Gbqi43xNKzBi/WQ0Q0dICqufzQbMjipQ== + dependencies: + "@types/retry" "*" + +"@types/retry@*": + version "0.12.5" + resolved "https://registry.yarnpkg.com/@types/retry/-/retry-0.12.5.tgz#f090ff4bd8d2e5b940ff270ab39fd5ca1834a07e" + integrity sha512-3xSjTp3v03X/lSQLkczaN9UIEwJMoMCA1+Nb5HfbJEQWogdeQIyVtTvxPXDQjZ5zws8rFQfVfRdz03ARihPJgw== + "@typescript-eslint/eslint-plugin@8.46.2": version "8.46.2" resolved "https://registry.yarnpkg.com/@typescript-eslint/eslint-plugin/-/eslint-plugin-8.46.2.tgz#dc4ab93ee3d7e6c8e38820a0d6c7c93c7183e2dc" @@ -1140,6 +1152,11 @@ globals@^14.0.0: resolved "https://registry.yarnpkg.com/globals/-/globals-14.0.0.tgz#898d7413c29babcf6bafe56fcadded858ada724e" integrity sha512-oahGvuMGQlPw/ivIYBjVSrWAfWLBeku5tpPE2fOPLi+WHffIWbuh2tCjhyQhTBPMf5E9jDEH4FOmTYgYwbKwtQ== +graceful-fs@^4.2.4: + version "4.2.11" + resolved "https://registry.yarnpkg.com/graceful-fs/-/graceful-fs-4.2.11.tgz#4183e4e8bf08bb6e05bbb2f7d2e0c8f712ca40e3" + integrity sha512-RbJ5/jmFcNNCcDV5o9eTnBLJ/HszWV0P73bc+Ff4nS/rJj+YaS6IGyiOL0VoBYX+l1Wrl3k63h/KrH+nhJ0XvQ== + graphemer@^1.4.0: version "1.4.0" resolved "https://registry.yarnpkg.com/graphemer/-/graphemer-1.4.0.tgz#fb2f1d55e0e3a1849aeffc90c4fa0dd53a0e66c6" @@ -1414,6 +1431,15 @@ prettier@3.8.1: resolved "https://registry.yarnpkg.com/prettier/-/prettier-3.8.1.tgz#edf48977cf991558f4fcbd8a3ba6015ba2a3a173" integrity sha512-UOnG6LftzbdaHZcKoPFtOcCKztrQ57WkHDeRD9t/PTQtmT0NHSeWWepj6pS0z/N7+08BHFDQVUrfmfMRcZwbMg== +proper-lockfile@4.1.2: + version "4.1.2" + resolved "https://registry.yarnpkg.com/proper-lockfile/-/proper-lockfile-4.1.2.tgz#c8b9de2af6b2f1601067f98e01ac66baa223141f" + integrity sha512-TjNPblN4BwAWMXU8s9AEz4JmQxnD1NNL7bNOY/AKUzyamc379FWASUhc/K1pL2noVb+XmZKLL68cjzLsiOAMaA== + dependencies: + graceful-fs "^4.2.4" + retry "^0.12.0" + signal-exit "^3.0.2" + punycode@^2.1.0: version "2.3.1" resolved "https://registry.yarnpkg.com/punycode/-/punycode-2.3.1.tgz#027422e2faec0b25e1549c3e1bd8309b9133b6e5" @@ -1429,6 +1455,11 @@ resolve-from@^4.0.0: resolved "https://registry.yarnpkg.com/resolve-from/-/resolve-from-4.0.0.tgz#4abcd852ad32dd7baabfe9b40e00a36db5f392e6" integrity sha512-pb/MYmXstAkysRFx8piNI1tGFNQIFA3vkE3Gq4EuA1dF6gHp/+vgZqsCGJapvy8N3Q+4o7FwvquPJcnZ7RYy4g== +retry@^0.12.0: + version "0.12.0" + resolved "https://registry.yarnpkg.com/retry/-/retry-0.12.0.tgz#1b42a6266a21f07421d1b0b54b7dc167b01c013b" + integrity sha512-9LkiTwjUh6rT555DtE9rTX+BKByPfrMzEAtnlEtdEwr3Nkffwiihqe2bWADg+OQRjt9gl6ICdmB/ZFDCGAtSow== + reusify@^1.0.4: version "1.1.0" resolved "https://registry.yarnpkg.com/reusify/-/reusify-1.1.0.tgz#0fe13b9522e1473f51b558ee796e08f11f9b489f" @@ -1502,6 +1533,11 @@ siginfo@^2.0.0: resolved "https://registry.yarnpkg.com/siginfo/-/siginfo-2.0.0.tgz#32e76c70b79724e3bb567cb9d543eb858ccfaf30" integrity sha512-ybx0WO1/8bSBLEWXZvEd7gMW3Sn3JFlW3TvX1nREbDLRNQNaeNN8WK0meBwPdAaOI7TtRRRJn/Es1zhrrCHu7g== +signal-exit@^3.0.2: + version "3.0.7" + resolved "https://registry.yarnpkg.com/signal-exit/-/signal-exit-3.0.7.tgz#a9a1767f8af84155114eaabd73f99273c8f59ad9" + integrity sha512-wnD2ZE+l+SPC/uoS0vXeE9L1+0wuaMqKlfz9AMUo38JsyLSBWSFcHR1Rri62LZc12vLr1gb3jl7iwQhgwpAbGQ== + signal-exit@^4.1.0: version "4.1.0" resolved "https://registry.yarnpkg.com/signal-exit/-/signal-exit-4.1.0.tgz#952188c1cbd546070e2dd20d0f41c0ae0530cb04"