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, + ) + } +}