From 23a5b1a3cb1d20ecc19fe7d059c20050018e5e5c Mon Sep 17 00:00:00 2001 From: Luke Tornquist Date: Fri, 30 Oct 2026 16:36:45 -0400 Subject: [PATCH 1/6] fix(db): auto-migrate artifact sync indexes on small databases Migrations 0076 and 0078 refuse any populated table whose index was not prebuilt concurrently, so every auto-migrating relay with artifact data crashloops on upgrade, including the Helm chart default. Both migrations are checksum-frozen, so the startup runner now builds their missing indexes first under the same 1s lock and 5s statement budgets migration 0049 uses. Small databases upgrade like empty ones; large ones still fail closed with an error pointing at docs/artifact-sync-deployment.md, and prebuilt indexes are skipped before CREATE INDEX takes its lock. Co-authored-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz> Co-Authored-By: Claude Opus 5.5 Signed-off-by: Luke Tornquist --- crates/buzz-db/src/runtime/migration.rs | 217 +++++++++++++++++++++++- docs/artifact-sync-deployment.md | 7 + docs/database.md | 6 + migrations/AGENTS.md | 3 + 4 files changed, 229 insertions(+), 4 deletions(-) diff --git a/crates/buzz-db/src/runtime/migration.rs b/crates/buzz-db/src/runtime/migration.rs index 5bd47588b6f..d2b47fbecfd 100644 --- a/crates/buzz-db/src/runtime/migration.rs +++ b/crates/buzz-db/src/runtime/migration.rs @@ -53,6 +53,10 @@ pub(crate) async fn run_migrations_through(pool: &PgPool, target: i64) -> Result async fn run_migrations_locked(conn: &mut PgConnection) -> Result<()> { reject_legacy_nip_rs_cardinality_ambiguity(conn).await?; + for (version, indexes) in INLINE_INDEX_BUILDS { + MIGRATOR.run_to(version - 1, &mut *conn).await?; + build_indexes_before_migration(conn, *version, indexes).await?; + } MIGRATOR.run(&mut *conn).await?; // The replica-fence proof (see `replica_fence`) requires the commit-time // `created_at` floor trigger from migration 0021 — correctly shaped — on @@ -65,6 +69,95 @@ async fn run_migrations_locked(conn: &mut PgConnection) -> Result<()> { Ok(()) } +/// Indexes that a migration verifies but refuses to build on a populated +/// table, keyed by that migration's version, as `(name, definition)`. +/// +/// Migrations 0076 and 0078 are checksum-frozen and expect every populated +/// database to prebuild these indexes with `CREATE INDEX CONCURRENTLY` +/// (`docs/artifact-sync-deployment.md`). Startup migration builds a missing +/// index here instead, under the same bounded budgets 0049 uses, so a small +/// populated database upgrades like an empty one and a large one still fails +/// closed. `CONCURRENTLY` is not used: a replica blocked on the migration lock +/// holds a snapshot that a concurrent build would wait for. +const INLINE_INDEX_BUILDS: &[(i64, &[(&str, &str)])] = &[ + ( + 76, + &[( + "idx_artifact_revisions_artifact", + "CREATE INDEX idx_artifact_revisions_artifact \ + ON public.artifact_revisions (community_id, artifact_id)", + )], + ), + ( + 78, + &[ + ( + "artifact_heads_modified", + "CREATE INDEX artifact_heads_modified \ + ON public.artifact_heads (community_id, modified_at, artifact_id)", + ), + ( + "artifact_heads_channel", + "CREATE INDEX artifact_heads_channel \ + ON public.artifact_heads (community_id, channel_id, artifact_id)", + ), + ( + "channel_members_sync", + "CREATE INDEX channel_members_sync \ + ON public.channel_members (community_id, pubkey, sync_modified_at, channel_id)", + ), + ( + "artifact_revisions_moves", + "CREATE INDEX artifact_revisions_moves \ + ON public.artifact_revisions (community_id, modified_at, event_id) \ + WHERE source_channel_id IS NOT NULL", + ), + ], + ), +]; + +/// Build the missing `indexes` in one bounded transaction unless migration +/// `version` is already recorded. An existing index, valid or not, is left +/// for the migration to verify, and is skipped before `CREATE INDEX` so a +/// prebuilt deployment never requests the writer-conflicting lock. +async fn build_indexes_before_migration( + conn: &mut PgConnection, + version: i64, + indexes: &[(&'static str, &'static str)], +) -> Result<()> { + let applied: bool = + sqlx::query_scalar("SELECT EXISTS (SELECT 1 FROM _sqlx_migrations WHERE version = $1)") + .bind(version) + .fetch_one(&mut *conn) + .await?; + if applied { + return Ok(()); + } + let build = async { + let mut tx = conn.begin().await?; + sqlx::raw_sql("SET LOCAL lock_timeout = '1s'; SET LOCAL statement_timeout = '5s'") + .execute(&mut *tx) + .await?; + for (name, definition) in indexes { + let exists: bool = sqlx::query_scalar("SELECT to_regclass($1) IS NOT NULL") + .bind(format!("public.{name}")) + .fetch_one(&mut *tx) + .await?; + if !exists { + sqlx::raw_sql(*definition).execute(&mut *tx).await?; + } + } + tx.commit().await + }; + build.await.map_err(|error| { + crate::DbError::InvalidData(format!( + "migration {version} blocked: building its indexes within the startup budget failed \ + ({error}); prebuild them concurrently as described in \ + docs/artifact-sync-deployment.md, then retry" + )) + }) +} + /// Run `op` while holding the exclusive schema/destruction session lock. /// /// `op` receives ownership of the detached connection that owns the advisory @@ -2062,9 +2155,9 @@ mod postgres_tests { if *path == this_file { assert_eq!( (macro_hits, run_hits, run_to_hits), - (1, 1, 1), + (1, 1, 2), "migration.rs must embed the migrator once, run it once in production, \ - and expose exactly one test-only bounded run" + and have one test-only and one production bounded run" ); } else if *path == push_gateway_exception { continue; @@ -2101,9 +2194,16 @@ mod postgres_tests { source[entry..locked].contains("with_exclusive_schema_destruction_lock("), "run_migrations must delegate through the exclusive schema/destruction lock" ); + let production_run_to_site = source[run_to_site + 1..] + .find(&migrator_run_to) + .map(|offset| run_to_site + 1 + offset) + .expect("production bounded migrator run site"); assert!( - run_site > locked && run_site < wrapper, - "the production migrator run site must live inside run_migrations_locked" + run_site > locked + && run_site < wrapper + && production_run_to_site > locked + && production_run_to_site < run_site, + "the production migrator run sites must live inside run_migrations_locked" ); assert!( run_to_site > entry @@ -3080,6 +3180,115 @@ mod postgres_tests { .expect("second post-apply reconciliation"); } + /// Migrations 0076 and 0078 refuse a populated table whose index was not + /// prebuilt concurrently. Startup auto-migration must still upgrade a small + /// populated database, and must fail closed without recording either + /// migration when the bounded inline build cannot take its lock. + #[tokio::test] + #[ignore = "requires Postgres"] + async fn populated_artifact_tables_auto_migrate_through_inline_index_builds() { + let pool = connect_test_pool().await; + reset_public_schema(&pool).await; + MIGRATOR + .run_to(75, &pool) + .await + .expect("apply migrations 1-75"); + + let community_id = uuid::Uuid::new_v4(); + sqlx::query("INSERT INTO communities (id, host) VALUES ($1, $2)") + .bind(community_id) + .bind(format!("artifact-{}.example", community_id.simple())) + .execute(&pool) + .await + .expect("insert community"); + let artifact_id = uuid::Uuid::new_v4(); + sqlx::query( + "INSERT INTO artifact_revisions (community_id, event_id, artifact_id) \ + VALUES ($1, $2, $3)", + ) + .bind(community_id) + .bind(vec![1_u8; 32]) + .bind(artifact_id) + .execute(&pool) + .await + .expect("insert artifact revision"); + sqlx::query( + "INSERT INTO artifact_heads \ + (community_id, artifact_id, event_id, channel_id, artifact_type) \ + VALUES ($1, $2, $3, $4, 'doc')", + ) + .bind(community_id) + .bind(artifact_id) + .bind(vec![1_u8; 32]) + .bind(uuid::Uuid::new_v4()) + .execute(&pool) + .await + .expect("insert artifact head"); + + let mut holder = pool.begin().await.expect("begin lock holder"); + sqlx::query("LOCK TABLE artifact_revisions IN ROW EXCLUSIVE MODE") + .execute(&mut *holder) + .await + .expect("hold a writer lock"); + let blocked = run_migrations(&pool) + .await + .expect_err("a build that cannot take its lock must fail closed") + .to_string(); + assert!( + blocked.contains("migration 76 blocked") + && blocked.contains("docs/artifact-sync-deployment.md"), + "unexpected error: {blocked}" + ); + assert_eq!(applied_versions(&pool).await.last().copied(), Some(75)); + holder.rollback().await.expect("release writer lock"); + + // A prebuilt index is skipped without requesting the build lock, so + // the same held writer lock first blocks 0077's ALTER TABLE instead. + sqlx::query( + "CREATE INDEX idx_artifact_revisions_artifact \ + ON public.artifact_revisions (community_id, artifact_id)", + ) + .execute(&pool) + .await + .expect("prebuild the 0076 index"); + let mut holder = pool.begin().await.expect("begin second lock holder"); + sqlx::query("LOCK TABLE artifact_revisions IN ROW EXCLUSIVE MODE") + .execute(&mut *holder) + .await + .expect("hold a writer lock again"); + let blocked = run_migrations(&pool) + .await + .expect_err("0077 cannot alter a table under a held writer lock") + .to_string(); + assert!( + !blocked.contains("migration 76 blocked"), + "a prebuilt index must not be rebuilt: {blocked}" + ); + assert_eq!(applied_versions(&pool).await.last().copied(), Some(76)); + holder.rollback().await.expect("release second writer lock"); + + run_migrations(&pool) + .await + .expect("populated small database auto-migrates"); + let versions = applied_versions(&pool).await; + assert!( + [76, 77, 78] + .iter() + .all(|version| versions.contains(version)), + "artifact sync migrations not recorded: {versions:?}" + ); + let valid: i64 = sqlx::query_scalar( + "SELECT count(*) FROM pg_index i JOIN pg_class c ON c.oid = i.indexrelid \ + WHERE c.relname IN ('idx_artifact_revisions_artifact', 'artifact_heads_modified', \ + 'artifact_heads_channel', 'channel_members_sync', 'artifact_revisions_moves') \ + AND i.indisvalid AND i.indisready AND i.indislive", + ) + .fetch_one(&pool) + .await + .expect("read index flags"); + assert_eq!(valid, 5); + } + #[tokio::test] #[ignore = "requires Postgres"] async fn pre_0007_ambiguous_nip_rs_data_blocks_without_mutation_and_allows_retry() { diff --git a/docs/artifact-sync-deployment.md b/docs/artifact-sync-deployment.md index e4bf6abb82e..6602be66a58 100644 --- a/docs/artifact-sync-deployment.md +++ b/docs/artifact-sync-deployment.md @@ -44,6 +44,13 @@ database can build the indexes under the migration's short budgets. Migration `0076` also verifies the exact definition and all three flags. Remove an invalid index with `DROP INDEX CONCURRENTLY` before a new build. +Relay startup migration and `buzz-admin migrate` build any of these indexes +that are missing before applying `0076` and `0078`. They use the migrations' +one-second lock and five-second statement budgets, which is enough for a small +database. A database too large for that budget fails closed with an error that +points here and records neither migration. Prebuild concurrently as above, then +retry. + After all index builds succeed, run `buzz-admin migrate` with the target `DATABASE_URL`. It verifies and records `0076`, reapplies the idempotent `0077`, then verifies and records `0078`. Direct SQL application does not record the diff --git a/docs/database.md b/docs/database.md index 0a3c9b58e70..971495d3064 100644 --- a/docs/database.md +++ b/docs/database.md @@ -415,6 +415,12 @@ parity assertion on the table for that window. online schema change tool to build it for them. CONCURRENTLY cannot run inside the migration transaction, so follow the prebuild-then-verify procedure in [thread-window-deployment.md](thread-window-deployment.md). + When the index is missing, the migration builds it inline under its lock + and statement budgets, so a small populated database still upgrades by + auto-migration and a large one fails closed until an operator prebuilds it. + A migration MUST NOT refuse a table only because it has rows. Migrations + 0076 and 0078 did, so the startup runner builds their indexes under the + same budgets before running them. Partitioned parents need per-partition concurrent builds attached to an `ON ONLY` parent index. - The declarative schema source declares indexes with plain `CREATE INDEX`. diff --git a/migrations/AGENTS.md b/migrations/AGENTS.md index 82d72c94a92..d3770150718 100644 --- a/migrations/AGENTS.md +++ b/migrations/AGENTS.md @@ -24,3 +24,6 @@ Before changing anything in this directory, read migration fails closed if its backfill has not run, and its release notes name the writer release that upgrades must not skip. - Bound every lock (`SET LOCAL lock_timeout`). +- Build a missing index inline under the lock and statement budgets, as + `0049` does. Never refuse a table only because it has rows: that breaks + every auto-migrating deployment with data, however small. From ea51ce2338f4d545b9e118c5cd41511cb6f500ee Mon Sep 17 00:00:00 2001 From: Luke Tornquist Date: Fri, 30 Oct 2026 16:53:58 -0400 Subject: [PATCH 2/6] fix(db): build each inline migration index in its own transaction Review follow-up: one transaction kept every earlier ShareLock until the last build committed, so writes to artifact_heads could stay blocked across all four 0078 builds. Commit each build on its own, and describe partial progress accurately in the deployment doc and test. Co-authored-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz> Co-Authored-By: Claude Opus 5.5 Signed-off-by: Luke Tornquist --- crates/buzz-db/src/runtime/migration.rs | 57 +++++++++++++++++++------ docs/artifact-sync-deployment.md | 7 +-- 2 files changed, 49 insertions(+), 15 deletions(-) diff --git a/crates/buzz-db/src/runtime/migration.rs b/crates/buzz-db/src/runtime/migration.rs index d2b47fbecfd..3182d4ae9d2 100644 --- a/crates/buzz-db/src/runtime/migration.rs +++ b/crates/buzz-db/src/runtime/migration.rs @@ -116,10 +116,11 @@ const INLINE_INDEX_BUILDS: &[(i64, &[(&str, &str)])] = &[ ), ]; -/// Build the missing `indexes` in one bounded transaction unless migration -/// `version` is already recorded. An existing index, valid or not, is left -/// for the migration to verify, and is skipped before `CREATE INDEX` so a -/// prebuilt deployment never requests the writer-conflicting lock. +/// Build each missing index in `indexes` in its own bounded transaction +/// unless migration `version` is already recorded, so no build holds its +/// writer-conflicting lock past its own budget. An existing index, valid or +/// not, is left for the migration to verify, and is skipped before +/// `CREATE INDEX` so a prebuilt deployment never requests that lock. async fn build_indexes_before_migration( conn: &mut PgConnection, version: i64, @@ -134,20 +135,21 @@ async fn build_indexes_before_migration( return Ok(()); } let build = async { - let mut tx = conn.begin().await?; - sqlx::raw_sql("SET LOCAL lock_timeout = '1s'; SET LOCAL statement_timeout = '5s'") - .execute(&mut *tx) - .await?; for (name, definition) in indexes { let exists: bool = sqlx::query_scalar("SELECT to_regclass($1) IS NOT NULL") .bind(format!("public.{name}")) - .fetch_one(&mut *tx) + .fetch_one(&mut *conn) .await?; if !exists { + let mut tx = conn.begin().await?; + sqlx::raw_sql("SET LOCAL lock_timeout = '1s'; SET LOCAL statement_timeout = '5s'") + .execute(&mut *tx) + .await?; sqlx::raw_sql(*definition).execute(&mut *tx).await?; + tx.commit().await?; } } - tx.commit().await + Ok::<(), sqlx::Error>(()) }; build.await.map_err(|error| { crate::DbError::InvalidData(format!( @@ -3182,8 +3184,9 @@ mod postgres_tests { /// Migrations 0076 and 0078 refuse a populated table whose index was not /// prebuilt concurrently. Startup auto-migration must still upgrade a small - /// populated database, and must fail closed without recording either - /// migration when the bounded inline build cannot take its lock. + /// populated database. When a bounded inline build cannot take its lock it + /// must fail closed without recording that migration, keeping earlier + /// migrations and each index already built in its own transaction. #[tokio::test] #[ignore = "requires Postgres"] async fn populated_artifact_tables_auto_migrate_through_inline_index_builds() { @@ -3267,6 +3270,36 @@ mod postgres_tests { assert_eq!(applied_versions(&pool).await.last().copied(), Some(76)); holder.rollback().await.expect("release second writer lock"); + // Block the third 0078 build. The two artifact_heads builds before it + // commit separately, so neither holds its lock while the third waits. + run_migrations_through(&pool, 77) + .await + .expect("apply migration 77"); + let mut holder = pool.begin().await.expect("begin third lock holder"); + sqlx::query("LOCK TABLE channel_members IN ROW EXCLUSIVE MODE") + .execute(&mut *holder) + .await + .expect("hold a channel_members writer lock"); + let blocked = run_migrations(&pool) + .await + .expect_err("a blocked 0078 build must fail closed") + .to_string(); + assert!( + blocked.contains("migration 78 blocked"), + "unexpected error: {blocked}" + ); + assert_eq!(applied_versions(&pool).await.last().copied(), Some(77)); + let committed: bool = + sqlx::query_scalar("SELECT to_regclass('public.artifact_heads_modified') IS NOT NULL") + .fetch_one(&pool) + .await + .expect("read earlier 0078 build"); + assert!( + committed, + "an earlier build must commit before the next one" + ); + holder.rollback().await.expect("release third writer lock"); + run_migrations(&pool) .await .expect("populated small database auto-migrates"); diff --git a/docs/artifact-sync-deployment.md b/docs/artifact-sync-deployment.md index 6602be66a58..affb01611b5 100644 --- a/docs/artifact-sync-deployment.md +++ b/docs/artifact-sync-deployment.md @@ -47,9 +47,10 @@ Remove an invalid index with `DROP INDEX CONCURRENTLY` before a new build. Relay startup migration and `buzz-admin migrate` build any of these indexes that are missing before applying `0076` and `0078`. They use the migrations' one-second lock and five-second statement budgets, which is enough for a small -database. A database too large for that budget fails closed with an error that -points here and records neither migration. Prebuild concurrently as above, then -retry. +database. Each index builds in its own transaction. A database too large for that +budget fails closed with an error that points here. The migration whose index +failed is not recorded, while earlier migrations and indexes stay committed. +Prebuild the rest concurrently as above, then retry. After all index builds succeed, run `buzz-admin migrate` with the target `DATABASE_URL`. It verifies and records `0076`, reapplies the idempotent `0077`, From 64cce23d2c1fe00be9cc47bd8108a337d38ae1d2 Mon Sep 17 00:00:00 2001 From: Luke Tornquist Date: Wed, 4 Nov 2026 16:58:32 -0500 Subject: [PATCH 3/6] fix(db): build 0081's index at startup and forbid populated-table refusals Migration 0081 refuses a populated nip_fi_profile_heads whose name index was not prebuilt, so every auto-migrating relay with verified profiles crash-loops on that version. The startup runner now builds its index first, like 0076 and 0078, under the same bounded budgets. Rule 6 now says a migration applies its own change on a populated database and never refuses one; operator prebuilds are an option for large databases, not a precondition. A source test rejects any migration that refuses a populated table unless it is a frozen one with an inline build, and the populated upgrade test boots a database with data through every embedded migration. The budget-exceeded hint now applies only to lock and statement timeouts. Co-authored-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz> Co-Authored-By: Claude Opus 5.5 Signed-off-by: Luke Tornquist --- AGENTS.md | 6 +- crates/buzz-db/src/runtime/migration.rs | 123 ++++++++++++++++++------ docs/artifact-sync-deployment.md | 6 +- docs/database.md | 38 +++++--- docs/profile-search.md | 10 +- migrations/AGENTS.md | 4 +- 6 files changed, 138 insertions(+), 49 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index ec7e7f87cf8..4a02fcc6c0d 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -17,8 +17,10 @@ Before planning or reviewing a non-trivial change: 4. For any change to `migrations/`, `schema/`, or schema-dependent `buzz-db` queries, read [docs/database.md](docs/database.md): no new triggers, functions, or foreign keys; no data backfills in migrations; update every - schema location; and keep every migration compatible with the relay that - is currently deployed. + schema location; keep every migration compatible with the relay that + is currently deployed; never make a migration refuse a populated table; + and test it against a populated database that auto-migrated through the + earlier migrations, not only a fresh one. 5. Check that the proposed design advances, or at least does not contradict, that product intent. Call out any intentional tension explicitly. diff --git a/crates/buzz-db/src/runtime/migration.rs b/crates/buzz-db/src/runtime/migration.rs index 3182d4ae9d2..33cc9ec62b7 100644 --- a/crates/buzz-db/src/runtime/migration.rs +++ b/crates/buzz-db/src/runtime/migration.rs @@ -53,9 +53,9 @@ pub(crate) async fn run_migrations_through(pool: &PgPool, target: i64) -> Result async fn run_migrations_locked(conn: &mut PgConnection) -> Result<()> { reject_legacy_nip_rs_cardinality_ambiguity(conn).await?; - for (version, indexes) in INLINE_INDEX_BUILDS { + for (version, doc, indexes) in INLINE_INDEX_BUILDS { MIGRATOR.run_to(version - 1, &mut *conn).await?; - build_indexes_before_migration(conn, *version, indexes).await?; + build_indexes_before_migration(conn, *version, doc, indexes).await?; } MIGRATOR.run(&mut *conn).await?; // The replica-fence proof (see `replica_fence`) requires the commit-time @@ -70,18 +70,24 @@ async fn run_migrations_locked(conn: &mut PgConnection) -> Result<()> { } /// Indexes that a migration verifies but refuses to build on a populated -/// table, keyed by that migration's version, as `(name, definition)`. +/// table, keyed by that migration's version, with the document that describes +/// its concurrent prebuild, as `(name, definition)`. /// -/// Migrations 0076 and 0078 are checksum-frozen and expect every populated -/// database to prebuild these indexes with `CREATE INDEX CONCURRENTLY` -/// (`docs/artifact-sync-deployment.md`). Startup migration builds a missing -/// index here instead, under the same bounded budgets 0049 uses, so a small -/// populated database upgrades like an empty one and a large one still fails -/// closed. `CONCURRENTLY` is not used: a replica blocked on the migration lock -/// holds a snapshot that a concurrent build would wait for. -const INLINE_INDEX_BUILDS: &[(i64, &[(&str, &str)])] = &[ +/// Migrations 0076, 0078 and 0081 are checksum-frozen and expect every +/// populated database to prebuild these indexes with `CREATE INDEX +/// CONCURRENTLY`. Startup migration builds a missing index here instead, under +/// the same bounded budgets 0049 uses, so a small populated database upgrades +/// like an empty one and a large one still fails closed. `CONCURRENTLY` is not +/// used: a replica blocked on the migration lock holds a snapshot that a +/// concurrent build would wait for. New migrations build their own indexes +/// inline (docs/database.md Rule 6); the `frozen_prebuild_guards_have_inline_builds` +/// test keeps this list equal to the migrations that refuse populated tables. +type InlineIndexBuilds = &'static [(&'static str, &'static str)]; + +const INLINE_INDEX_BUILDS: &[(i64, &str, InlineIndexBuilds)] = &[ ( 76, + "docs/artifact-sync-deployment.md", &[( "idx_artifact_revisions_artifact", "CREATE INDEX idx_artifact_revisions_artifact \ @@ -90,6 +96,7 @@ const INLINE_INDEX_BUILDS: &[(i64, &[(&str, &str)])] = &[ ), ( 78, + "docs/artifact-sync-deployment.md", &[ ( "artifact_heads_modified", @@ -114,6 +121,15 @@ const INLINE_INDEX_BUILDS: &[(i64, &[(&str, &str)])] = &[ ), ], ), + ( + 81, + "docs/profile-search.md", + &[( + "idx_nip_fi_profile_heads_names", + // The exact pg_get_indexdef form migration 0081 verifies. + r#"CREATE INDEX idx_nip_fi_profile_heads_names ON public.nip_fi_profile_heads USING gin (to_tsvector('simple'::regconfig, ((((COALESCE((jsonb_path_query_first(event, 'strict $."tags"[*]?(@[0] == "display_name")[1]'::jsonpath, '{}'::jsonb, true) #>> '{}'::text[]), ''::text) || ' '::text) || COALESCE((jsonb_path_query_first(event, 'strict $."tags"[*]?(@[0] == "username")[1]'::jsonpath, '{}'::jsonb, true) #>> '{}'::text[]), ''::text)) || ' '::text) || COALESCE((jsonb_path_query_first(event, 'strict $."tags"[*]?(@[0] == "full_name")[1]'::jsonpath, '{}'::jsonb, true) #>> '{}'::text[]), ''::text))))"#, + )], + ), ]; /// Build each missing index in `indexes` in its own bounded transaction @@ -124,7 +140,8 @@ const INLINE_INDEX_BUILDS: &[(i64, &[(&str, &str)])] = &[ async fn build_indexes_before_migration( conn: &mut PgConnection, version: i64, - indexes: &[(&'static str, &'static str)], + doc: &str, + indexes: InlineIndexBuilds, ) -> Result<()> { let applied: bool = sqlx::query_scalar("SELECT EXISTS (SELECT 1 FROM _sqlx_migrations WHERE version = $1)") @@ -152,10 +169,18 @@ async fn build_indexes_before_migration( Ok::<(), sqlx::Error>(()) }; build.await.map_err(|error| { + // Only a lock or statement timeout means the table is too large or + // too busy for the startup budget; pass any other failure through. + let over_budget = error + .as_database_error() + .and_then(|db| db.code()) + .is_some_and(|code| code == "55P03" || code == "57014"); + if !over_budget { + return error.into(); + } crate::DbError::InvalidData(format!( "migration {version} blocked: building its indexes within the startup budget failed \ - ({error}); prebuild them concurrently as described in \ - docs/artifact-sync-deployment.md, then retry" + ({error}); prebuild them concurrently as described in {doc}, then retry" )) }) } @@ -3182,14 +3207,15 @@ mod postgres_tests { .expect("second post-apply reconciliation"); } - /// Migrations 0076 and 0078 refuse a populated table whose index was not - /// prebuilt concurrently. Startup auto-migration must still upgrade a small - /// populated database. When a bounded inline build cannot take its lock it - /// must fail closed without recording that migration, keeping earlier - /// migrations and each index already built in its own transaction. + /// Migrations 0076, 0078 and 0081 refuse a populated table whose index was + /// not prebuilt concurrently. A relay that auto-migrated through earlier + /// releases and holds data must still boot through every migration. When a + /// bounded inline build cannot take its lock it must fail closed without + /// recording that migration, keeping earlier migrations and each index + /// already built in its own transaction. #[tokio::test] #[ignore = "requires Postgres"] - async fn populated_artifact_tables_auto_migrate_through_inline_index_builds() { + async fn populated_database_auto_migrates_through_every_migration() { let pool = connect_test_pool().await; reset_public_schema(&pool).await; MIGRATOR @@ -3300,26 +3326,69 @@ mod postgres_tests { ); holder.rollback().await.expect("release third writer lock"); + // Seed a verified profile head before 0081, as on any relay that has + // published NIP-FI verified profiles. + run_migrations_through(&pool, 80) + .await + .expect("apply migrations through 80"); + sqlx::query( + "INSERT INTO nip_fi_profile_heads (community_id, pubkey, issuer, issued_at, event) \ + VALUES ($1, $2, $3, 1, $4)", + ) + .bind(community_id) + .bind("a".repeat(64)) + .bind("b".repeat(64)) + .bind(serde_json::json!({"tags": [["display_name", "Wes Johnson"]]})) + .execute(&pool) + .await + .expect("insert verified profile head"); + run_migrations(&pool) .await .expect("populated small database auto-migrates"); let versions = applied_versions(&pool).await; - assert!( - [76, 77, 78] - .iter() - .all(|version| versions.contains(version)), - "artifact sync migrations not recorded: {versions:?}" + let embedded: Vec = MIGRATOR.iter().map(|migration| migration.version).collect(); + assert_eq!( + versions, embedded, + "a populated database must boot through every embedded migration" ); let valid: i64 = sqlx::query_scalar( "SELECT count(*) FROM pg_index i JOIN pg_class c ON c.oid = i.indexrelid \ WHERE c.relname IN ('idx_artifact_revisions_artifact', 'artifact_heads_modified', \ - 'artifact_heads_channel', 'channel_members_sync', 'artifact_revisions_moves') \ + 'artifact_heads_channel', 'channel_members_sync', 'artifact_revisions_moves', \ + 'idx_nip_fi_profile_heads_names') \ AND i.indisvalid AND i.indisready AND i.indislive", ) .fetch_one(&pool) .await .expect("read index flags"); - assert_eq!(valid, 5); + assert_eq!(valid, 6); + } + + /// A migration that refuses a populated table crash-loops every + /// auto-migrating relay with data. Only the frozen ones may, and each must + /// have an inline build the startup runner applies first. + #[test] + fn frozen_prebuild_guards_have_inline_builds() { + let dir = std::path::Path::new(env!("CARGO_MANIFEST_DIR")).join("../../migrations"); + let mut guarded: Vec = std::fs::read_dir(&dir) + .expect("read migrations dir") + .filter_map(|entry| { + let path = entry.expect("read migration entry").path(); + let name = path.file_name()?.to_str()?.to_owned(); + let version: i64 = name.split('_').next()?.parse().ok()?; + let sql = std::fs::read_to_string(&path).expect("read migration"); + sql.contains("RAISE EXCEPTION 'prebuild ").then_some(version) + }) + .collect(); + guarded.sort_unstable(); + let inline: Vec = INLINE_INDEX_BUILDS.iter().map(|(v, _, _)| *v).collect(); + assert_eq!( + guarded, inline, + "a migration must build its own index instead of refusing a populated table \ + (docs/database.md Rule 6); only frozen guards listed in INLINE_INDEX_BUILDS may remain" + ); + assert_eq!(inline, [76, 78, 81], "do not add new prebuild guards"); } #[tokio::test] diff --git a/docs/artifact-sync-deployment.md b/docs/artifact-sync-deployment.md index affb01611b5..78461169191 100644 --- a/docs/artifact-sync-deployment.md +++ b/docs/artifact-sync-deployment.md @@ -13,8 +13,10 @@ New application writers stamp every head change, grant, rejoin, removal, redaction, and move source. Changes to channel listing also require recovery. Old application updates do not maintain all these stamps, so mixed-version serving cannot advertise incremental correctness. -For a populated database, first build the revision lookup index outside a -transaction, before migration `0076`: +Relay startup migration and `buzz-admin migrate` build the indexes below +themselves within the migrations' budgets, which is enough for a small +database. For a database too large for that, first build the revision lookup +index outside a transaction, before migration `0076`: ```sql CREATE INDEX CONCURRENTLY idx_artifact_revisions_artifact diff --git a/docs/database.md b/docs/database.md index 971495d3064..d09bab62a6b 100644 --- a/docs/database.md +++ b/docs/database.md @@ -410,17 +410,29 @@ parity assertion on the table for that window. `SET LOCAL statement_timeout` when a statement can scan. A migration that cannot get its lock fails and is retried. It MUST NOT queue behind traffic on a hot table. -- In `migrations/`, an index on an existing populated table MUST be built - with `CREATE INDEX CONCURRENTLY`, because auto-migrating deployments have no - online schema change tool to build it for them. CONCURRENTLY cannot run - inside the migration transaction, so follow the prebuild-then-verify - procedure in [thread-window-deployment.md](thread-window-deployment.md). - When the index is missing, the migration builds it inline under its lock - and statement budgets, so a small populated database still upgrades by - auto-migration and a large one fails closed until an operator prebuilds it. - A migration MUST NOT refuse a table only because it has rows. Migrations - 0076 and 0078 did, so the startup runner builds their indexes under the - same budgets before running them. +- Startup auto-migration is how most deployments upgrade, so every migration + MUST apply its own change on a database that holds data. A migration MUST + NOT refuse to run because a table has rows: every auto-migrating relay with + data would crash-loop on that version until someone intervened by hand. +- In `migrations/`, a new index on an existing table is built by the + migration itself. Skip `CREATE INDEX` when `to_regclass` finds the index, + otherwise build it inline under the migration's lock and statement budgets, + then verify its definition and flags, as `0049` does. A small populated + database upgrades like an empty one. A database too large for the budget + fails with a timeout, which is retried; its operator prebuilds the exact + index with `CREATE INDEX CONCURRENTLY` (procedure in + [thread-window-deployment.md](thread-window-deployment.md)) and the + migration then verifies it. The prebuild is an option for large or + out-of-band deployments, never a precondition. Frozen migrations + `0076`, `0078` and `0081` predate this rule; the startup runner builds their + indexes first (`INLINE_INDEX_BUILDS` in + `crates/buzz-db/src/runtime/migration.rs`), and a source test rejects any + other migration that refuses a populated table. +- Test every migration against a populated database that auto-migrated + through the earlier migrations, not only a fresh one: migrate to the + previous version, seed the tables the migration reads or changes, then run + `run_migrations` to head and assert it succeeds. CI's fresh databases have + empty tables, so a migration that only fails on data passes there. Partitioned parents need per-partition concurrent builds attached to an `ON ONLY` parent index. - The declarative schema source declares indexes with plain `CREATE INDEX`. @@ -462,8 +474,8 @@ description: test for each changed table and index. - [ ] That the currently deployed relay works against the new schema, and how that was checked. -- [ ] Lock and statement timeouts, concurrent index builds in `migrations/`, - and any operator prebuild step. +- [ ] Lock and statement timeouts, how each new index is built inline, and the + test that upgrades a populated database through the migration. ## Related documents diff --git a/docs/profile-search.md b/docs/profile-search.md index 2e30ae6ac00..0ec7f6db0b1 100644 --- a/docs/profile-search.md +++ b/docs/profile-search.md @@ -65,10 +65,12 @@ never matches and never makes the search fail ## Deployment -Verified-name matching reads `idx_nip_fi_profile_heads_names`. Migration -`0081` builds it only while `nip_fi_profile_heads` is empty; on a populated -database it fails until the index exists. Build it first, outside a -transaction: +Verified-name matching reads `idx_nip_fi_profile_heads_names`. Relay startup +migration and `buzz-admin migrate` build it before migration `0081` under a +one-second lock and five-second statement budget, which is enough for a small +database. Applied directly, `0081` builds it only while `nip_fi_profile_heads` +is empty. For a database too large for the budget, or before applying `0081` +directly, build it first, outside a transaction: ```sql CREATE INDEX CONCURRENTLY idx_nip_fi_profile_heads_names diff --git a/migrations/AGENTS.md b/migrations/AGENTS.md index d3770150718..b0bc887e8a7 100644 --- a/migrations/AGENTS.md +++ b/migrations/AGENTS.md @@ -25,5 +25,7 @@ Before changing anything in this directory, read name the writer release that upgrades must not skip. - Bound every lock (`SET LOCAL lock_timeout`). - Build a missing index inline under the lock and statement budgets, as - `0049` does. Never refuse a table only because it has rows: that breaks + `0049` does. Never refuse a table only because it has rows: that crash-loops every auto-migrating deployment with data, however small. +- Test the migration against a populated database that auto-migrated through + the earlier migrations, not only a fresh one (docs/database.md Rule 6). From 6c3768b54253332cd367d9d12a41943e1d54b125 Mon Sep 17 00:00:00 2001 From: Luke Tornquist Date: Wed, 4 Nov 2026 17:05:34 -0500 Subject: [PATCH 4/6] docs(db): limit operator prebuilds to out-of-band migration With auto-migration on, migrations are the whole upgrade procedure, so an operator prebuild is only for deployments that apply migrations out of band. Drop the separate populated-upgrade testing rule; the populated boot test and the prebuild-guard source test cover it. Co-authored-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz> Co-Authored-By: Claude Opus 5.5 Signed-off-by: Luke Tornquist --- AGENTS.md | 5 ++--- docs/artifact-sync-deployment.md | 7 +++--- docs/database.md | 38 ++++++++++++++------------------ docs/profile-search.md | 8 +++---- migrations/AGENTS.md | 2 -- 5 files changed, 26 insertions(+), 34 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 4a02fcc6c0d..62cedb2c955 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -18,9 +18,8 @@ Before planning or reviewing a non-trivial change: queries, read [docs/database.md](docs/database.md): no new triggers, functions, or foreign keys; no data backfills in migrations; update every schema location; keep every migration compatible with the relay that - is currently deployed; never make a migration refuse a populated table; - and test it against a populated database that auto-migrated through the - earlier migrations, not only a fresh one. + is currently deployed; and never make a migration refuse a populated + table. 5. Check that the proposed design advances, or at least does not contradict, that product intent. Call out any intentional tension explicitly. diff --git a/docs/artifact-sync-deployment.md b/docs/artifact-sync-deployment.md index 78461169191..bf9ccbfd99e 100644 --- a/docs/artifact-sync-deployment.md +++ b/docs/artifact-sync-deployment.md @@ -14,9 +14,10 @@ redaction, and move source. Changes to channel listing also require recovery. Ol stamps, so mixed-version serving cannot advertise incremental correctness. Relay startup migration and `buzz-admin migrate` build the indexes below -themselves within the migrations' budgets, which is enough for a small -database. For a database too large for that, first build the revision lookup -index outside a transaction, before migration `0076`: +themselves within the migrations' budgets. A deployment too large for those +budgets runs with auto-migration off; before applying the migrations out of +band, first build the revision lookup index outside a transaction, before +migration `0076`: ```sql CREATE INDEX CONCURRENTLY idx_artifact_revisions_artifact diff --git a/docs/database.md b/docs/database.md index d09bab62a6b..5728bf8ebe7 100644 --- a/docs/database.md +++ b/docs/database.md @@ -410,29 +410,23 @@ parity assertion on the table for that window. `SET LOCAL statement_timeout` when a statement can scan. A migration that cannot get its lock fails and is retried. It MUST NOT queue behind traffic on a hot table. -- Startup auto-migration is how most deployments upgrade, so every migration - MUST apply its own change on a database that holds data. A migration MUST - NOT refuse to run because a table has rows: every auto-migrating relay with - data would crash-loop on that version until someone intervened by hand. +- With `BUZZ_AUTO_MIGRATE` on, migrations are the whole upgrade procedure, so + every migration MUST apply its own change on a database that holds data. A + migration MUST NOT refuse to run because a table has rows: every + auto-migrating relay with data would crash-loop on that version until + someone intervened by hand. - In `migrations/`, a new index on an existing table is built by the migration itself. Skip `CREATE INDEX` when `to_regclass` finds the index, otherwise build it inline under the migration's lock and statement budgets, - then verify its definition and flags, as `0049` does. A small populated - database upgrades like an empty one. A database too large for the budget - fails with a timeout, which is retried; its operator prebuilds the exact - index with `CREATE INDEX CONCURRENTLY` (procedure in - [thread-window-deployment.md](thread-window-deployment.md)) and the - migration then verifies it. The prebuild is an option for large or - out-of-band deployments, never a precondition. Frozen migrations - `0076`, `0078` and `0081` predate this rule; the startup runner builds their - indexes first (`INLINE_INDEX_BUILDS` in - `crates/buzz-db/src/runtime/migration.rs`), and a source test rejects any - other migration that refuses a populated table. -- Test every migration against a populated database that auto-migrated - through the earlier migrations, not only a fresh one: migrate to the - previous version, seed the tables the migration reads or changes, then run - `run_migrations` to head and assert it succeeds. CI's fresh databases have - empty tables, so a migration that only fails on data passes there. + then verify its definition and flags, as `0049` does. A deployment too + large for those budgets runs with auto-migration off and applies migrations + out of band; only such a deployment prebuilds the exact index with + `CREATE INDEX CONCURRENTLY` (procedure in + [thread-window-deployment.md](thread-window-deployment.md)) before applying + the migration, which then verifies it. Frozen migrations `0076`, `0078` and + `0081` predate this rule; the startup runner builds their indexes first + (`INLINE_INDEX_BUILDS` in `crates/buzz-db/src/runtime/migration.rs`), and a + source test rejects any other migration that refuses a populated table. Partitioned parents need per-partition concurrent builds attached to an `ON ONLY` parent index. - The declarative schema source declares indexes with plain `CREATE INDEX`. @@ -474,8 +468,8 @@ description: test for each changed table and index. - [ ] That the currently deployed relay works against the new schema, and how that was checked. -- [ ] Lock and statement timeouts, how each new index is built inline, and the - test that upgrades a populated database through the migration. +- [ ] Lock and statement timeouts, how each new index is built inline, and + any prebuild step for deployments that apply migrations out of band. ## Related documents diff --git a/docs/profile-search.md b/docs/profile-search.md index 0ec7f6db0b1..5d70db4883a 100644 --- a/docs/profile-search.md +++ b/docs/profile-search.md @@ -67,10 +67,10 @@ never matches and never makes the search fail Verified-name matching reads `idx_nip_fi_profile_heads_names`. Relay startup migration and `buzz-admin migrate` build it before migration `0081` under a -one-second lock and five-second statement budget, which is enough for a small -database. Applied directly, `0081` builds it only while `nip_fi_profile_heads` -is empty. For a database too large for the budget, or before applying `0081` -directly, build it first, outside a transaction: +one-second lock and five-second statement budget. Applied directly as SQL, +`0081` builds it only while `nip_fi_profile_heads` is empty. A deployment that +applies migrations out of band (auto-migration off) builds it first, outside a +transaction: ```sql CREATE INDEX CONCURRENTLY idx_nip_fi_profile_heads_names diff --git a/migrations/AGENTS.md b/migrations/AGENTS.md index b0bc887e8a7..34cafd615f7 100644 --- a/migrations/AGENTS.md +++ b/migrations/AGENTS.md @@ -27,5 +27,3 @@ Before changing anything in this directory, read - Build a missing index inline under the lock and statement budgets, as `0049` does. Never refuse a table only because it has rows: that crash-loops every auto-migrating deployment with data, however small. -- Test the migration against a populated database that auto-migrated through - the earlier migrations, not only a fresh one (docs/database.md Rule 6). From 5ca55f9cb3c8f38767a90effc5d43f24b00b4260 Mon Sep 17 00:00:00 2001 From: Luke Tornquist Date: Wed, 4 Nov 2026 17:07:41 -0500 Subject: [PATCH 5/6] test(db): boot verified-profile databases through 0081 separately The artifact test's run_migrations_through(80) bypasses the startup runner's inline builds, so 0078 refused its seeded rows before 0081 was reached. Seed verified profiles in their own test from 0080 instead. Co-authored-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz> Co-Authored-By: Claude Opus 5.5 Signed-off-by: Luke Tornquist --- crates/buzz-db/src/runtime/migration.rs | 78 ++++++++++++++++++------- 1 file changed, 57 insertions(+), 21 deletions(-) diff --git a/crates/buzz-db/src/runtime/migration.rs b/crates/buzz-db/src/runtime/migration.rs index 33cc9ec62b7..dce1dc609ac 100644 --- a/crates/buzz-db/src/runtime/migration.rs +++ b/crates/buzz-db/src/runtime/migration.rs @@ -3207,15 +3207,15 @@ mod postgres_tests { .expect("second post-apply reconciliation"); } - /// Migrations 0076, 0078 and 0081 refuse a populated table whose index was - /// not prebuilt concurrently. A relay that auto-migrated through earlier - /// releases and holds data must still boot through every migration. When a + /// Migrations 0076 and 0078 refuse a populated table whose index was not + /// prebuilt concurrently. A relay that auto-migrated through earlier + /// releases and holds artifact data must still boot through every migration. When a /// bounded inline build cannot take its lock it must fail closed without /// recording that migration, keeping earlier migrations and each index /// already built in its own transaction. #[tokio::test] #[ignore = "requires Postgres"] - async fn populated_database_auto_migrates_through_every_migration() { + async fn populated_artifact_tables_auto_migrate_through_every_migration() { let pool = connect_test_pool().await; reset_public_schema(&pool).await; MIGRATOR @@ -3326,16 +3326,43 @@ mod postgres_tests { ); holder.rollback().await.expect("release third writer lock"); - // Seed a verified profile head before 0081, as on any relay that has - // published NIP-FI verified profiles. - run_migrations_through(&pool, 80) + run_migrations(&pool) .await - .expect("apply migrations through 80"); + .expect("populated small database auto-migrates"); + assert_boots_through_every_migration(&pool).await; + assert_eq!( + valid_indexes( + &pool, + &[ + "idx_artifact_revisions_artifact", + "artifact_heads_modified", + "artifact_heads_channel", + "channel_members_sync", + "artifact_revisions_moves", + ] + ) + .await, + 5 + ); + } + + /// Migration 0081 refuses a populated `nip_fi_profile_heads`. A relay that + /// has published NIP-FI verified profiles must still boot through every + /// migration. + #[tokio::test] + #[ignore = "requires Postgres"] + async fn populated_verified_profiles_auto_migrate_through_every_migration() { + let pool = connect_test_pool().await; + reset_public_schema(&pool).await; + MIGRATOR + .run_to(80, &pool) + .await + .expect("apply migrations 1-80"); sqlx::query( "INSERT INTO nip_fi_profile_heads (community_id, pubkey, issuer, issued_at, event) \ VALUES ($1, $2, $3, 1, $4)", ) - .bind(community_id) + .bind(uuid::Uuid::new_v4()) .bind("a".repeat(64)) .bind("b".repeat(64)) .bind(serde_json::json!({"tags": [["display_name", "Wes Johnson"]]})) @@ -3345,24 +3372,32 @@ mod postgres_tests { run_migrations(&pool) .await - .expect("populated small database auto-migrates"); - let versions = applied_versions(&pool).await; + .expect("database with verified profiles auto-migrates"); + assert_boots_through_every_migration(&pool).await; + assert_eq!( + valid_indexes(&pool, &["idx_nip_fi_profile_heads_names"]).await, + 1 + ); + } + + async fn assert_boots_through_every_migration(pool: &PgPool) { let embedded: Vec = MIGRATOR.iter().map(|migration| migration.version).collect(); assert_eq!( - versions, embedded, + applied_versions(pool).await, + embedded, "a populated database must boot through every embedded migration" ); - let valid: i64 = sqlx::query_scalar( + } + + async fn valid_indexes(pool: &PgPool, names: &[&str]) -> i64 { + sqlx::query_scalar( "SELECT count(*) FROM pg_index i JOIN pg_class c ON c.oid = i.indexrelid \ - WHERE c.relname IN ('idx_artifact_revisions_artifact', 'artifact_heads_modified', \ - 'artifact_heads_channel', 'channel_members_sync', 'artifact_revisions_moves', \ - 'idx_nip_fi_profile_heads_names') \ - AND i.indisvalid AND i.indisready AND i.indislive", + WHERE c.relname = ANY($1) AND i.indisvalid AND i.indisready AND i.indislive", ) - .fetch_one(&pool) + .bind(names) + .fetch_one(pool) .await - .expect("read index flags"); - assert_eq!(valid, 6); + .expect("read index flags") } /// A migration that refuses a populated table crash-loops every @@ -3378,7 +3413,8 @@ mod postgres_tests { let name = path.file_name()?.to_str()?.to_owned(); let version: i64 = name.split('_').next()?.parse().ok()?; let sql = std::fs::read_to_string(&path).expect("read migration"); - sql.contains("RAISE EXCEPTION 'prebuild ").then_some(version) + sql.contains("RAISE EXCEPTION 'prebuild ") + .then_some(version) }) .collect(); guarded.sort_unstable(); From a481b5bbd5c157d49cd8f78d60e43014272e9c44 Mon Sep 17 00:00:00 2001 From: Luke Tornquist Date: Wed, 4 Nov 2026 17:33:24 -0500 Subject: [PATCH 6/6] docs(db): reconcile Rule 3 with inline index builds Rule 3 forbade any write-blocking scan, contradicting Rule 6's bounded inline CREATE INDEX. Limit it to large tables and allow the budgeted build. State that the prebuild-guard lint matches only its literal wording, and that 0078 refuses populated tables only when applied as SQL. Co-authored-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz> Co-Authored-By: Claude Opus 5.5 Signed-off-by: Luke Tornquist --- docs/artifact-sync-deployment.md | 3 ++- docs/database.md | 12 +++++++----- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/docs/artifact-sync-deployment.md b/docs/artifact-sync-deployment.md index bf9ccbfd99e..14148e7920a 100644 --- a/docs/artifact-sync-deployment.md +++ b/docs/artifact-sync-deployment.md @@ -42,7 +42,8 @@ CREATE INDEX CONCURRENTLY artifact_revisions_moves ``` Migration `0078` verifies each index definition and its valid, ready, and live -flags. It refuses a populated table whose index is missing. A fresh empty +flags. Applied directly as SQL, it refuses a populated table whose index is +missing. A fresh empty database can build the indexes under the migration's short budgets. Migration `0076` also verifies the exact definition and all three flags. Remove an invalid index with `DROP INDEX CONCURRENTLY` before a new build. diff --git a/docs/database.md b/docs/database.md index 5728bf8ebe7..b96c0c6876a 100644 --- a/docs/database.md +++ b/docs/database.md @@ -205,9 +205,10 @@ deletes to ship first, then the constraint drop as a separate contract step. ### 3. Backfills are not migrations A migration MUST contain schema changes only. It MUST NOT `UPDATE`, `INSERT`, -or `DELETE` existing rows, and MUST NOT scan or rewrite a table while holding a -write-blocking lock. Scans that do not block writes, such as -`VALIDATE CONSTRAINT` or a bounded concurrent index build, are allowed (Rule 6). +or `DELETE` existing rows, and MUST NOT scan or rewrite a large table while +holding a write-blocking lock. Scans that do not block writes, such as +`VALIDATE CONSTRAINT` or a concurrent index build, are allowed, as is a plain +`CREATE INDEX` under Rule 6's lock and statement budgets (Rule 6). - Historical data repair MUST be a backfill definition that conforms to [data-backfills.md](data-backfills.md): stable ID, finite upper bound, @@ -425,8 +426,9 @@ parity assertion on the table for that window. [thread-window-deployment.md](thread-window-deployment.md)) before applying the migration, which then verifies it. Frozen migrations `0076`, `0078` and `0081` predate this rule; the startup runner builds their indexes first - (`INLINE_INDEX_BUILDS` in `crates/buzz-db/src/runtime/migration.rs`), and a - source test rejects any other migration that refuses a populated table. + (`INLINE_INDEX_BUILDS` in `crates/buzz-db/src/runtime/migration.rs`). A + source test rejects any other migration that raises `'prebuild …'`; it + matches that wording only, so review must catch other refusals. Partitioned parents need per-partition concurrent builds attached to an `ON ONLY` parent index. - The declarative schema source declares indexes with plain `CREATE INDEX`.