From 4f8257ef72f1fa40038899411bb473579ba1e994 Mon Sep 17 00:00:00 2001 From: Caleb Allen Date: Mon, 17 Aug 2026 12:24:49 -0400 Subject: [PATCH] fix(database): never retire the catalog the index build derives MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The stale-shape repair dropped the CI catalog on its 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 the drop was correct by the app's rule: a client's catalog is *downloaded*, so a wrong shape costs a minute of re-fetching the artifact, while keeping it costs every Explore read. It is the wrong rule for one database. cmd/indexbuild's catalog is not downloaded, it is what the artifact is cut from — the only way back is the ~205 GB dump stream the /cache volume exists to avoid. And that database is deliberately kept in the older encoding, which `fix(indexexport): read an index older than the binary` exists to tolerate, so the shape does not match by design and would have been dropped on every run. retireLibraryTables, right beside it, never touches the catalog for exactly this reason. The repair reached past that protection because it runs inside database.NewDB, which cmd/indexbuild also calls. So the policy is a build tag, which is how this project already tells the index tools apart (runtime_indexbuild.go, servicestartup.go, dumpbuild_stub.go): Cache tables are rebuilt in the app and never in cmd/indexbuild. Owned and Derived are still repaired in both — that is the half this database can safely discard, and retireLibraryTables already discards it. The residual trade is deliberate: a future explore_index column will now fail the index job loudly on applySchema rather than silently costing it a 205 GB rebuild. A human should decide that one. TestTheCatalogSurvivesAStaleShape is the accident, symptom first, with the shape the real database is in — every current column, ids and entity type still text. It fails with "the catalog was retired" when the policy is flipped back. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01AfVYUVExXsx1nSWrXN8mAh --- backend/database/staleshape.go | 8 ++ backend/database/staleshape_policy.go | 15 ++++ .../database/staleshape_policy_indexbuild.go | 37 ++++++++ cmd/indexbuild/staleschema_test.go | 89 +++++++++++++++++++ 4 files changed, 149 insertions(+) create mode 100644 backend/database/staleshape_policy.go create mode 100644 backend/database/staleshape_policy_indexbuild.go diff --git a/backend/database/staleshape.go b/backend/database/staleshape.go index 118f249..41022ac 100644 --- a/backend/database/staleshape.go +++ b/backend/database/staleshape.go @@ -115,6 +115,14 @@ func retireStaleTables( continue } + // Whether a stale Cache table may be rebuilt is decided per + // binary, at compile time: the app re-downloads its catalog in + // about a minute, cmd/indexbuild would re-derive it from ~205 GB + // of dumps. See staleshape_policy.go. + if entry.Kind == datamap.Cache && !retireStaleCache { + continue + } + reason, err := staleReason(ctx, db, table, columns) if err != nil { return err diff --git a/backend/database/staleshape_policy.go b/backend/database/staleshape_policy.go new file mode 100644 index 0000000..3f88981 --- /dev/null +++ b/backend/database/staleshape_policy.go @@ -0,0 +1,15 @@ +//go:build !indexbuild + +package database + +// retireStaleCache reports whether a Cache table whose shape no longer +// matches the schema may be dropped and rebuilt. +// +// In the app: yes. The only Cache table large enough to care about is +// the catalog, and the app does not derive it — it downloads it. A +// stale one costs about a minute of re-fetching the artifact, and +// keeping it costs every Explore read on the install, because a +// projection naming a column the table does not have fails outright. +// +// In cmd/indexbuild: no, and the file next to this one says why. +const retireStaleCache = true diff --git a/backend/database/staleshape_policy_indexbuild.go b/backend/database/staleshape_policy_indexbuild.go new file mode 100644 index 0000000..5557826 --- /dev/null +++ b/backend/database/staleshape_policy_indexbuild.go @@ -0,0 +1,37 @@ +//go:build indexbuild + +package database + +// retireStaleCache is false here, and this is the whole reason the +// policy is a build tag rather than a rule inside retireStaleTables. +// +// The index database is the one place in this project where the catalog +// is *derived* rather than downloaded. Rebuilding it is a ~205 GB dump +// stream over hours, resumed across runs from a checkpoint on a +// persistent volume; that volume exists for no other purpose. The app's +// answer to a stale catalog — drop it, fetch the artifact again — is +// not available here, because this database *is* what the artifact is +// cut from. +// +// This was not hypothetical. The repair shipped without it and dropped +// the CI catalog on its 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 shape mismatch was real and the drop was correct by the app's +// rule. It was still wrong here: that database is deliberately kept in +// the older encoding, which is what `fix(indexexport): read an index +// older than the binary` exists to tolerate. A rule that is right for +// every install and catastrophic for one database has to be told which +// one it is in, and a build tag is how this project already tells the +// index tools apart (backend/events/runtime_indexbuild.go, +// backend/explore/servicestartup.go, dumpbuild_stub.go). +// +// cmd/indexbuild has its own repair for the half it *can* safely +// discard: retireLibraryTables drops every table the datamap does not +// classify as Cache, which is empty by construction in that database. +// Between the two, the library half is repaired and the catalog is +// never touched. +const retireStaleCache = false diff --git a/cmd/indexbuild/staleschema_test.go b/cmd/indexbuild/staleschema_test.go index a0a3b1e..130e791 100644 --- a/cmd/indexbuild/staleschema_test.go +++ b/cmd/indexbuild/staleschema_test.go @@ -125,3 +125,92 @@ func count(t *testing.T, dbPath, query string) int { return n } + +// TestTheCatalogSurvivesAStaleShape is the accident written down. +// +// The app repairs a stale Cache table by dropping it: its catalog is +// downloaded, so a wrong shape costs about a minute of re-fetching and +// keeping it costs every Explore read. Applied here that rule is +// catastrophic — this database is what the artifact is *cut from*, so +// there is nothing to re-fetch and the only way back is the ~205 GB +// dump stream the /cache volume exists to avoid. +// +// It shipped without that distinction and dropped the real CI catalog +// on 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: that database is deliberately kept in the +// older encoding, which `fix(indexexport): read an index older than the +// binary` exists to tolerate. So the shape will not match, every run, +// by design — and the catalog must survive it anyway. +func TestTheCatalogSurvivesAStaleShape(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) + } + + // The shape the real index database is in: every current column, + // but the ids and the entity type still text. That is what the + // exporter's backward-compatibility fix tolerates, and it is what + // the repair saw and called stale. + exec(t, dbPath, ` + DROP TABLE explore_index; + CREATE TABLE explore_index ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + entity_type TEXT NOT NULL, + mbid TEXT NOT NULL, + title TEXT NOT NULL, + artist_name TEXT NOT NULL, + artist_mbid TEXT NOT NULL, + aliases TEXT NOT NULL DEFAULT '', + popularity INTEGER NOT NULL DEFAULT 0, + listener_count INTEGER NOT NULL DEFAULT 0, + duration INTEGER NOT NULL DEFAULT 0, + caa_release_mbid TEXT NOT NULL DEFAULT '', + release_name TEXT NOT NULL DEFAULT '', + primary_type TEXT NOT NULL DEFAULT '', + secondary_types TEXT NOT NULL DEFAULT '', + release_date TEXT NOT NULL DEFAULT '', + total_tracks INTEGER NOT NULL DEFAULT 0, + artist_type TEXT NOT NULL DEFAULT '', + country TEXT NOT NULL DEFAULT '', + disambiguation TEXT NOT NULL DEFAULT '', + sort_name TEXT NOT NULL DEFAULT '', + in_library INTEGER NOT NULL DEFAULT 0, + is_similar INTEGER NOT NULL DEFAULT 0, + local_artist_id INTEGER, + local_release_group_id INTEGER, + local_recording_id INTEGER, + discog_fetched INTEGER NOT NULL DEFAULT 0, + UNIQUE(mbid) + ); + INSERT INTO explore_index + (entity_type, mbid, title, artist_name, artist_mbid) + VALUES ('artist', 'a-b-c', 'A Catalog Row', 'An Artist', 'd-e-f'); + `) + + if _, err := database.NewDB(logger); err != nil { + t.Fatalf("open with a stale catalog shape: %v", err) + } + + if got := count(t, dbPath, "SELECT COUNT(*) FROM explore_index"); got != 1 { + t.Fatalf( + "explore_index rows = %d, want 1 — the catalog was retired, "+ + "which costs this database a ~205GB rebuild", + got, + ) + } +}