diff --git a/.gitea/workflows/index-artifact.yml b/.gitea/workflows/index-artifact.yml index db36b2d..77e0c91 100644 --- a/.gitea/workflows/index-artifact.yml +++ b/.gitea/workflows/index-artifact.yml @@ -53,6 +53,10 @@ on: # Runs share one persistent working directory, so they must not overlap. # A push landing mid-build waits rather than corrupting the checkpoint. +# +# That directory holds the only copy of a catalog nothing can cheaply +# re-derive: see docs/index-cache.md for the snapshot it takes and the +# restore, which is minutes against the hours a rebuild costs. concurrency: group: search-index cancel-in-progress: false diff --git a/.planning/NOTES.md b/.planning/NOTES.md index 68973b1..e25d116 100644 --- a/.planning/NOTES.md +++ b/.planning/NOTES.md @@ -3404,3 +3404,44 @@ Three things worth keeping from it: rebuilding. That is the right default, and it means the next schema change touching `explore_index` needs a deliberate plan for this one database rather than none. + +## Two guards for the index cache, and what each one is worth (2026-08-17) + +Both come out of the incident above, and they protect different halves +of it. + +**`TestNoCacheTableIsRetiredHere` asserts the outcome, not the +mechanism.** The test that shipped with the fix pins one table in one +wrong shape, which is the failure that happened; what actually cost the +rebuild was a destructive repair added at `database.NewDB` — the +chokepoint every binary here shares — without asking which binary it was +in. The next one will have a different name and a different reason. So +this puts *every* `datamap` Cache table into a shape the schema has +moved past, opens the database the way `cmd/indexbuild` does, and +requires all of them to still be there. + +Three things it got right by being written this way. The table list is +`datamap.ByKind(Cache)`, so the two credit tables added the same day +were covered without anyone adding them — flipping the policy back fails +on **five** tables including `artist_credit_part` and +`artist_credit_ref`, where the single-table test fails on one. It +asserts rows survive as well as the table, because SQLite does an +implicit DELETE before a DROP and a repair that recreated the table +would otherwise look identical. And it *accepts* an error from `NewDB`, +because that is the trade the fix documents: loud failure instead of a +silent day of downloading. + +**`scripts/index-cache-snapshot.sh` covers the half no test can.** The +volume held the only copy of a catalog whose rebuild is hours of someone +else's bandwidth. `VACUUM INTO` rather than `cp`, because a byte copy of +a live SQLite file is a corrupt file of plausible size; the staging +directory is deliberately not copied, since a build resumes without it; +and the snapshot is reopened and asked for its catalog row count before +any rotation happens. Both failure paths were exercised rather than +argued: a corrupt source and an empty catalog each exit non-zero, delete +their own output, and leave the previous snapshots in place. + +`docs/index-cache.md` is the restore procedure, and the number that +makes it worth having: a restored snapshot resolves to `refresh` and +folds in the incremental listens since — minutes, against the 3–23 h a +rebuild was estimating. diff --git a/cmd/indexbuild/staleschema_test.go b/cmd/indexbuild/staleschema_test.go index 130e791..b459ebe 100644 --- a/cmd/indexbuild/staleschema_test.go +++ b/cmd/indexbuild/staleschema_test.go @@ -12,6 +12,7 @@ import ( _ "modernc.org/sqlite" "yellowjacket/backend/database" + "yellowjacket/backend/datamap" "yellowjacket/backend/system" ) @@ -214,3 +215,97 @@ func TestTheCatalogSurvivesAStaleShape(t *testing.T) { ) } } + +// TestNoCacheTableIsRetiredHere is the general form of the accident +// above, and it exists because the specific one is not the risk. +// +// `TestTheCatalogSurvivesAStaleShape` pins one table in one wrong shape, +// which is the failure that happened. What cost the ~205 GB was not that +// shape: it was a destructive repair added to `database.NewDB` -- the +// one chokepoint every binary in this project shares -- without asking +// which binary it was running in. The next such repair will have a +// different name and a different reason, and this database still cannot +// afford it. +// +// So the assertion is about the *outcome* rather than the mechanism: put +// every Cache table in a shape the schema has certainly moved past, open +// the database the way cmd/indexbuild does, and require that all of them +// are still there afterwards. Any future repair that drops one fails +// here regardless of how it decides to. +// +// Two things about it are deliberate. +// +// The table list comes from `datamap.ByKind(Cache)` rather than being +// written out, so a Cache table added next year is covered by this test +// on the day it is added -- the same reason `TestCatalogCoversSchema` +// reads the schema instead of a list. +// +// And `NewDB` returning an error is *accepted*, because that is the +// trade the fix documents: with Cache tables no longer rebuilt here, a +// shape the schema moved past now fails this job loudly instead of +// silently costing it a day of downloading. Loud is fine. Gone is not. +func TestNoCacheTableIsRetiredHere(t *testing.T) { + logger := slog.New(slog.DiscardHandler) + + t.Setenv("YJ_HOME", t.TempDir()) + + dataDir, err := system.GetUserDataDirPath() + if err != nil { + t.Fatalf("resolve data dir: %v", err) + } + + dbPath := filepath.Join(dataDir, "yj.db") + + if _, err := database.NewDB(logger); err != nil { + t.Fatalf("first open: %v", err) + } + + // An FTS table is four shadow tables and cannot be given a "wrong + // shape" meaningfully; the repair skips them for the same reason and + // retires them with their parent, which the parents below cover. + var cache []string + + for _, table := range datamap.ByKind(datamap.Cache) { + if table.FTS { + continue + } + + cache = append(cache, table.Name) + } + + if len(cache) == 0 { + t.Fatal("no Cache tables to check: the datamap or this test is wrong") + } + + for _, name := range cache { + // A shape nothing in the current schema describes. What matters + // is only that it disagrees; the real mismatch was one column's + // type. + exec(t, dbPath, ` + DROP TABLE IF EXISTS `+name+`; + CREATE TABLE `+name+` (id INTEGER PRIMARY KEY, moved_past TEXT); + INSERT INTO `+name+` (moved_past) VALUES ('irreplaceable'); + `) + } + + // The error is not the assertion: see the note above. + _, _ = database.NewDB(logger) + + for _, name := range cache { + rows := count(t, dbPath, + `SELECT count(*) FROM sqlite_master WHERE type = 'table' AND name = '`+name+`'`) + if rows == 0 { + t.Errorf("%s was retired: in this database a Cache table is derived, "+ + "not downloaded, and dropping one costs the ~205 GB dump stream", name) + + continue + } + + // Present but emptied is the same loss wearing a different + // shape: SQLite does an implicit DELETE before a DROP, and a + // repair that recreated the table would look identical here. + if n := count(t, dbPath, `SELECT count(*) FROM `+name); n == 0 { + t.Errorf("%s survived but was emptied", name) + } + } +} diff --git a/docs/index-cache.md b/docs/index-cache.md new file mode 100644 index 0000000..5c48c44 --- /dev/null +++ b/docs/index-cache.md @@ -0,0 +1,99 @@ +# The index cache, and why it has a snapshot + +`/srv/yellowjacket/index-cache` on the Gitea host is the `YJ_HOME` the +search-index job keeps between runs — `.gitea/workflows/index-artifact.yml` +mounts it at `/cache`. It holds the catalog every user eventually +downloads, and it is the one database in this project that is +**derived rather than downloaded**. + +That is the whole reason this document exists. An install with a broken +catalog re-fetches the ~0.6 GB artifact and is fine in a minute. This +database *is* what that artifact is cut from, so its only route back is +re-streaming the MetaBrainz dumps: hours, at a rate that belongs to +someone else's server, holding a runner of capacity 1 the entire time. + +## What happened on 2026-08-17 + +A schema repair (`fix(database): retire a table whose shape the schema +moved past`) dropped every table whose live shape disagreed with the +schema, before `applySchema`. Correct for the app. Applied here it +deleted the catalog 19 seconds into the first run: + +``` +retiring a table ... table=explore_index + reason="column entity_type is TEXT, schema declares INTEGER" +index maintenance mode=build reason="no completed import yet" +``` + +The mismatch was real and deliberate: this database is kept in the older +text encoding, which `artifactStoresText` and `sourceColumns` exist to +tolerate. So it would have been judged stale on *every* run. + +Two things came out of it. `retireStaleCache` is now a build tag — +false under `indexbuild`, true in the app — and +`TestNoCacheTableIsRetiredHere` asserts the outcome rather than the +mechanism, so the next destructive repair fails a test instead of a +production volume. And the volume got the snapshot it should always have +had, below. + +## Taking snapshots + +```sh +scripts/index-cache-snapshot.sh [SOURCE_HOME] [DEST_DIR] [KEEP] +``` + +Defaults: `/srv/yellowjacket/index-cache`, `/srv/yellowjacket/index-snapshots`, +keep 2. On the Gitea host, daily and away from the Monday 04:00 build: + +``` +30 5 * * * /path/to/index-cache-snapshot.sh >> /var/log/yj-index-snapshot.log 2>&1 +``` + +Three properties worth knowing before trusting it: + +- **It uses `VACUUM INTO`, not `cp`.** The database may be open, and a + byte copy of a live SQLite file is a corrupt file of plausible size. + `VACUUM INTO` takes a read lock and writes a consistent, compacted + copy; it is safe to run while a build is in progress. +- **It does not copy `data/explore-staging`.** That is a resumable + checkpoint of work in flight — large, constantly changing, and a build + resumes without it. What cannot be cheaply re-derived is the finished + catalog, which is in the database. +- **It verifies before it rotates.** Each snapshot is reopened and asked + for its catalog row count; a run that produces an unreadable or empty + file fails loudly, deletes its own output, and leaves the previous + snapshots alone. Both paths are exercised, not assumed. + +## Restoring + +Stop anything that might be using the volume first — the job holds it +for the length of a build, and the concurrency group (`search-index`) +means a queued run will start the moment one ends. + +```sh +cd /srv/yellowjacket +mv index-cache/data/yj.db index-cache/data/yj.db.broken # keep it until you are sure +cp index-snapshots/yj-index-.db index-cache/data/yj.db +chown --reference=index-cache/data/yj.db.broken index-cache/data/yj.db +``` + +Then dispatch the workflow with `mode=auto`. A restored snapshot is +older than the dumps, so `indexbuild` resolves to `refresh` and folds in +the incremental listens since — which is minutes, not hours. + +Two notes on what a restore does *not* need. The staging directory can +be deleted; it will be rebuilt if a build is needed. And the published +artifact is untouched by any of this: users keep downloading the last +good one until a run reports `complete=true` and `changed=true` +republishes. + +## The trade this leaves open + +With Cache tables no longer retired under `indexbuild`, a future +`explore_index` column change will fail this job **loudly** — at +`applySchema`, or at the first query naming the column — rather than +silently rebuilding. That is the right default: loud is recoverable and +a silent day of downloading is not. It does mean the next schema change +touching `explore_index` needs a deliberate plan for this one database: +take a snapshot, apply the change to a copy, or accept a rebuild +knowingly. diff --git a/scripts/index-cache-snapshot.sh b/scripts/index-cache-snapshot.sh new file mode 100755 index 0000000..39a4654 --- /dev/null +++ b/scripts/index-cache-snapshot.sh @@ -0,0 +1,89 @@ +#!/bin/sh +# Snapshot the index build's database, which is the only copy of it. +# +# `/srv/yellowjacket/index-cache` is the `YJ_HOME` the index job keeps +# between runs (`.gitea/workflows/index-artifact.yml` mounts it at +# `/cache`). Its catalog is *derived*, not downloaded: the only way to +# rebuild it is to re-stream the MetaBrainz dumps, which is hours at a +# rate that is someone else's to decide. On 2026-08-17 a schema repair +# dropped it and cost exactly that. +# +# So it gets a snapshot, and this is the script a cron on that host runs. +# It is deliberately not part of the workflow: a backup that only exists +# while the thing it protects is being modified is not a backup. +# +# Usage (on the Gitea host): +# +# scripts/index-cache-snapshot.sh [SOURCE_HOME] [DEST_DIR] [KEEP] +# +# SOURCE_HOME default /srv/yellowjacket/index-cache +# DEST_DIR default /srv/yellowjacket/index-snapshots +# KEEP how many to retain, default 2 +# +# Suggested cron — daily, and nowhere near the Monday 04:00 build: +# +# 30 5 * * * /path/to/index-cache-snapshot.sh >> /var/log/yj-index-snapshot.log 2>&1 +# +# Three things about it are load-bearing. +# +# **`VACUUM INTO`, not `cp`.** The database may be open, and a byte copy +# of a live SQLite file is a corrupt file with a plausible size. +# `VACUUM INTO` takes a read lock, writes a consistent compacted copy, +# and is safe while the index job is running — it costs the snapshot's +# own write, not the source's availability. +# +# **The staging directory is not copied.** `/cache/data/explore-staging` +# is a resumable checkpoint of work in flight; it is large, it changes +# constantly, and a build resumes without it. What cannot be re-derived +# cheaply is the finished catalog, which is in the database. +# +# **A snapshot that is not verified is a belief.** Each one is opened +# and asked for its catalog row count before the old ones are rotated +# out, so a run that produced an unreadable file leaves the previous +# good snapshot in place and fails loudly. +set -eu + +SOURCE_HOME="${1:-/srv/yellowjacket/index-cache}" +DEST_DIR="${2:-/srv/yellowjacket/index-snapshots}" +KEEP="${3:-2}" + +DB="$SOURCE_HOME/data/yj.db" +STAMP=$(date +%Y%m%d-%H%M%S) +OUT="$DEST_DIR/yj-index-$STAMP.db" + +die() { echo "index-snapshot: $*" >&2; exit 1; } + +command -v sqlite3 >/dev/null 2>&1 || die "sqlite3 is not installed" +[ -f "$DB" ] || die "no database at $DB (is SOURCE_HOME right?)" + +mkdir -p "$DEST_DIR" + +# Headroom: the copy is at most the size of the source, usually less +# (VACUUM compacts). Refusing here beats a half-written snapshot. +need_kb=$(du -k "$DB" | cut -f1) +free_kb=$(df -Pk "$DEST_DIR" | awk 'NR == 2 { print $4 }') +[ "$free_kb" -gt "$need_kb" ] || die "not enough space in $DEST_DIR (need ~${need_kb}K, have ${free_kb}K)" + +# A failed snapshot must leave nothing behind. `VACUUM INTO` refuses an +# existing file, so a partial one from a disk-full write would block +# every later run -- and worse, rotation counts files by name, so it +# would eventually be kept *instead of* a good one. +cleanup() { [ -n "${KEPT:-}" ] || rm -f "$OUT"; } +trap cleanup EXIT + +echo "index-snapshot: $DB -> $OUT" +sqlite3 "$DB" "VACUUM INTO '$OUT'" || die "VACUUM INTO failed" + +rows=$(sqlite3 "$OUT" "SELECT count(*) FROM explore_index" 2>/dev/null) \ + || die "snapshot is unreadable: keeping the previous ones" +[ "${rows:-0}" -gt 0 ] || die "snapshot has an empty catalog: keeping the previous ones" + +KEPT=1 + +echo "index-snapshot: ok, $rows catalog rows, $(du -h "$OUT" | cut -f1)" + +# Rotate only after the new one has been verified. +ls -1t "$DEST_DIR"/yj-index-*.db 2>/dev/null | tail -n +"$((KEEP + 1))" | while read -r old; do + echo "index-snapshot: removing $old" + rm -f "$old" +done