test(database): the next destructive repair fails a test, not a volume
The fix for the dropped catalog pins one table in one wrong shape, which is the failure that happened. What cost the rebuild was more general: a destructive repair added at `database.NewDB` -- the chokepoint every binary in this project shares -- without asking which binary it runs in. The next one will have a different name and a different reason. So `TestNoCacheTableIsRetiredHere` asserts the outcome instead: put every `datamap` Cache table into a shape the schema has moved past, open the database the way cmd/indexbuild does, and require all of them to still be there. Driving it from `datamap.ByKind` is what makes it cover tables nobody remembered -- flipping the policy back fails on five, including the two artist-credit tables added the same day, where the existing test fails on one. It asserts the rows survive too, because SQLite does an implicit DELETE before a DROP and a repair that recreated the table would look identical. And it accepts an error from `NewDB`, because that is the documented trade: loud is recoverable, gone is not. `scripts/index-cache-snapshot.sh` covers the half no test can reach. The volume holds the only copy of a catalog that costs hours of someone else's bandwidth to re-derive. `VACUUM INTO` rather than `cp`, since a byte copy of a live SQLite file is a corrupt file of plausible size; the resumable staging directory is skipped; and each snapshot is reopened and asked for its catalog row count before anything is rotated out. A corrupt source and an empty catalog were both exercised: each exits non-zero, removes its own output, and leaves the previous snapshots alone. docs/index-cache.md is the restore, and the reason to bother: a restored snapshot resolves to `refresh` and folds in the listens since, which is minutes against the 3-23h this rebuild has been estimating.
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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-<stamp>.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.
|
||||
Executable
+89
@@ -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
|
||||
Reference in New Issue
Block a user