diff --git a/src/servers/sidecar/music/index.ts b/src/servers/sidecar/music/index.ts index 083d9270..f28aafbc 100644 --- a/src/servers/sidecar/music/index.ts +++ b/src/servers/sidecar/music/index.ts @@ -6,7 +6,6 @@ import { streamAudioFile } from './stream-audio'; import { cliampUpgradeData, musicWebsocket } from './cliamp-ws'; import { ensurePulseAudio } from './pulse-audio'; import { startNightlyReindex, stopNightlyReindex } from './nightly-reindex'; -import { startMusicWatcher, stopMusicWatcher } from './watcher'; import { reindexNow, reindexFull, @@ -136,8 +135,14 @@ await ensureCacheSetup(); // Nightly full reindex at 3am (staged + atomic swap). startNightlyReindex(); -// Recursive watcher on ~/Music → localized reindex on any change. -startMusicWatcher(); +// No filesystem watcher on ~/Music. Bun's recursive fs.watch costs one inotify watch per ENTRY, files +// included — ~92k for this library against a 65536 ceiling — so it could never establish, and the +// ENOSPC came back asynchronously as an unhandled 'error' event that killed this whole sidecar 17k +// times over. It also drained the per-UID watch pool, starving every other watcher on the machine. +// Reindexing is triggered instead: the ↻ button in the music browser (POST /reindex, incremental) and +// the nightly full rebuild above. `reindexFolder` in the indexer is retained and currently unused — it +// is the targeted hook for whatever writes to ~/Music (slskd, transmission, download-media) to declare +// the one folder it just wrote, which is the cheap version of what the watcher was guessing at. // PulseAudio daemon + the `virtual_out` null sink both cliamp halves depend on. Officer used to do this at // its own boot, which meant every restart of a process with no audio responsibilities re-checked the sink. @@ -487,7 +492,6 @@ const connection = createSidecarConnector({ function shutdown(signal: string) { console.log(`[music] ${signal} received, shutting down...`); stopNightlyReindex(); - stopMusicWatcher(); try { server.stop(true); } catch { diff --git a/src/servers/sidecar/music/indexer.ts b/src/servers/sidecar/music/indexer.ts index c5ad27c4..ab398eb3 100644 --- a/src/servers/sidecar/music/indexer.ts +++ b/src/servers/sidecar/music/indexer.ts @@ -1,5 +1,5 @@ import { readdir, stat, mkdir, writeFile, readFile, rm, symlink, rename, lstat } from 'node:fs/promises'; -import { existsSync } from 'node:fs'; +import { existsSync, readdirSync } from 'node:fs'; import { join, relative, basename, dirname } from 'node:path'; import { homedir } from 'node:os'; @@ -39,8 +39,8 @@ const COVER_MAX_PX = 600; // Cache-format version. Bump when the build produces NEW per-album outputs (so far: v2 added video // posters + lyrics). A manifest written by an older CACHE_VERSION forces a one-time FULL rebuild — the // per-album `v` skip only applies once the cache is already at the current format. -// v1 → initial (meta + cover) v2 → + posters/ + lyrics/ -const CACHE_VERSION = 2; +// v1 → initial (meta + cover) v2 → + posters/ + lyrics/ v3 → + lyrics/poster COUNTS in the manifest +const CACHE_VERSION = 3; // ── Staging slots + atomic symlink swap ── // The live cache path (CACHE_ROOT) is a SYMLINK to a slot dir; all readers + incremental writes follow @@ -118,6 +118,12 @@ type ManifestEntry = { videos?: number; images?: number; disco?: boolean; + // How many files the last build wrote into /lyrics and /posters. Recorded ONLY so the + // incremental skip can notice one has gone missing — see `outputsExist` in buildFolder. Without them + // the integrity check could verify meta.json/cover.jpg/discography.json and nothing else, so a lost + // lyrics file or poster left the album skipped forever and only a full rebuild restored it. + lyrics?: number; + posters?: number; }; type Manifest = { version: number; generatedAt: number; albums: Record }; @@ -543,8 +549,20 @@ async function runBuild(outRoot: string, prev: Manifest): Promise { try { await mkdir(outRoot, { recursive: true }); + walkFailures = 0; await walk(MUSIC_ROOT, prev, next, outRoot); + // An incremental build survives an unreadable folder by carrying the previous entries forward (see + // walk). A from-scratch build has no previous entries to carry — every unreadable folder is simply + // absent from `next`, and publishing that slot would delete those albums from the live index for + // real. Refuse instead: reindexFull discards a slot whose build errored and leaves the live index + // alone, so a disk that hiccups during the nightly costs one skipped night, not a hole in the library. + if (walkFailures > 0 && !Object.keys(prev.albums).length) { + throw new Error( + `${walkFailures} folder(s) unreadable during a from-scratch build — refusing to publish a partial index`, + ); + } + // Prune cache dirs for albums that vanished from the library (only meaningful when prev is populated). for (const rel of Object.keys(prev.albums)) { if (!next.albums[rel]) { @@ -580,6 +598,10 @@ async function runBuild(outRoot: string, prev: Manifest): Promise { // All index-mutating ops (full, incremental, localized) run one-at-a-time through this lock, so a // localized reindex can never race a full reindex's atomic swap (which would corrupt the manifest). let indexLock: Promise = Promise.resolve(); + +// Folders the current build could not read for a reason other than "it is gone". Reset at the start of +// every runBuild; consulted at the end to decide whether a from-scratch index is safe to publish. +let walkFailures = 0; function withIndexLock(fn: () => Promise): Promise { const result = indexLock.then(fn, fn); indexLock = result.then( @@ -597,9 +619,22 @@ async function persistManifest(): Promise { /** * Incremental, live: rebuild only changed albums (unchanged ones skip by version stamp), updating the - * live cache + manifest in place. Fast — the manual reindex button uses this. + * live cache + manifest in place. Fast — the manual reindex button uses this. The one exception is a + * cache-format upgrade, which is delegated to the staged full rebuild below. */ -export function reindexNow(): Promise { +export async function reindexNow(): Promise { + // A cache written by an older CACHE_VERSION forces every album to rebuild (see `formatCurrent` in + // buildFolder). Doing that through THIS path would rewrite all ~6k albums inside the LIVE cache, each + // one rm'd and then regenerated, so for the ~40 minutes it runs a reader can hit an album whose files + // are momentarily missing. The staged path does identical work into a fresh slot and swaps atomically, + // so hand the format upgrade to it. Checked before withIndexLock — that lock is a plain promise chain + // and is not reentrant, so calling reindexFull() inside it would deadlock. + const current = await loadManifest(); + if (current.version !== CACHE_VERSION) { + console.log(`[music] cache format v${current.version} → v${CACHE_VERSION}: routing to a staged rebuild`); + return reindexFull(); + } + return withIndexLock(async () => { const next = await runBuild(CACHE_ROOT, await loadManifest()); if (!status.error) { @@ -635,9 +670,15 @@ export function reindexFull(): Promise { } /** - * Localized reindex of a single folder (the watcher's unit): rebuild just this album/artist's cache - * entry in the LIVE cache and patch the manifest; a vanished folder is pruned. Serialized with the - * full/incremental builds so it never races the atomic swap. + * Localized reindex of a single folder: rebuild just this album/artist's cache entry in the LIVE cache + * and patch the manifest; a vanished folder is pruned. Serialized with the full/incremental builds so + * it never races the atomic swap. + * + * CURRENTLY UNUSED. It was the filesystem watcher's unit of work until that was removed (see the note + * in index.ts — the recursive watch could not fit in the inotify budget). Kept because it is the right + * hook for a writer that already knows what it wrote: slskd, transmission or the download-media task + * calling this with the one folder it just created is strictly cheaper and more accurate than either + * watching 92k inodes or walking the whole tree. */ export function reindexFolder(rel: string, opts?: { recursive?: boolean }): Promise { return withIndexLock(async () => { @@ -649,7 +690,7 @@ export function reindexFolder(rel: string, opts?: { recursive?: boolean }): Prom // Reindex `rel` against the live manifest/cache. Always: rebuild rel's own entry and prune any manifest // descendants whose top-level child dir has vanished from disk (catches a renamed/removed subdir when the -// watcher only saw the new-name or the parent event). When `recursive`, also descends into each subdir — +// caller only knows the new name, or only the parent). When `recursive`, also descends into each subdir — // so a newly-appeared or renamed-in container's album children get indexed (a shallow reindex of a // container is a no-op, since containers hold no audio of their own). Returns whether the manifest changed // so the caller persists exactly once. Runs inside `withIndexLock` via reindexFolder. @@ -724,8 +765,8 @@ async function pruneSubtree(rel: string): Promise { // Build ONE folder's cache entry (meta / cover / posters / lyrics / discography) into `outRoot`, // honoring the per-folder version skip. Returns its manifest entry, or null if the folder holds nothing -// indexable. This is the single-folder unit shared by the full/incremental walk AND the watcher's -// localized reindex. `files` are the folder's file entries (dirs excluded); `prevEntry`/`prevVersion` +// indexable. This is the single-folder unit shared by the full/incremental walk AND `reindexFolder`'s +// localized rebuild. `files` are the folder's file entries (dirs excluded); `prevEntry`/`prevVersion` // drive the skip check. async function buildFolder( dirAbs: string, @@ -801,11 +842,30 @@ async function buildFolder( prevEntry?.cover ? 'cover.jpg' : null, hasDisco ? 'discography.json' : null, ].filter((f): f is string => f !== null); - const outputsExist = expected.every((f) => existsSync(join(cacheDir, f))); + // A build writes FIVE kinds of output; the three above are single files, lyrics/ and posters/ are + // directories of them. Checking only the files meant a lost lyrics file or poster kept a matching `v` + // and a passing existence check, so the album was skipped on every incremental forever and only the + // nightly full restored it — the "inconsistencies the normal reindex misses". Compare counts instead. + // `>=` deliberately: a missing output must rebuild, a stray extra one need not. + const cachedFileCount = (name: string): number => { + try { + return readdirSync(join(cacheDir, name)).length; + } catch { + return 0; + } + }; + const outputsExist = + expected.every((f) => existsSync(join(cacheDir, f))) && + cachedFileCount('lyrics') >= (prevEntry?.lyrics ?? 0) && + cachedFileCount('posters') >= (prevEntry?.posters ?? 0); // Only trust the per-album `v` skip once the cache is already at the current format — an older // CACHE_VERSION means new outputs (posters/lyrics) may be missing, so rebuild every folder once. const formatCurrent = prevVersion === CACHE_VERSION; + // Carried forward untouched on a skip (nothing was rewritten, so the previous counts still describe + // what is on disk); recomputed from what the build actually wrote below. + let lyricsWritten = prevEntry?.lyrics ?? 0; + let postersWritten = prevEntry?.posters ?? 0; if (formatCurrent && prevEntry?.v === v && outputsExist) { status.albumsSkipped += 1; } else { @@ -849,6 +909,11 @@ async function buildFolder( emitProgress(); return vmeta; }); + // Both dirs were just rm'd and rebuilt, and each writer emits exactly one file per item it + // reports (resolveTrackLyrics → `.`, the poster pass → `posters/