mirror of
https://github.com/block/buzz.git
synced 2026-11-06 09:21:45 +00:00
fix(db): auto-migrate prebuild-guarded indexes on small databases (#8802)
## Problem Migrations `0076`, `0078` and `0081` raise an exception on any table with at least one row unless its index was prebuilt with `CREATE INDEX CONCURRENTLY`. A relay that runs migrations at startup (`BUZZ_AUTO_MIGRATE`, on by default in the Helm chart) and has data in those tables therefore crash-loops on upgrade: ``` migration error: while executing migration 81: error returned from database: prebuild idx_nip_fi_profile_heads_names concurrently; see docs/profile-search.md ``` `0081` (#9019) hits every auto-migrating relay that has published NIP-FI verified profiles. `0076`/`0078` (#8523) hit any relay with artifact data or channel members. Each of these tables is small enough on such relays that a plain index build takes milliseconds. The root cause is `docs/database.md` Rule 6. It told authors that an index on a populated table MUST be built concurrently by an operator ahead of time, so migrations were written to refuse populated tables. With auto-migration that refusal is a crash loop, not a safe stop. ## Fix The migrations are already applied on some databases, so they can't be edited: sqlx would reject the changed checksums. Instead, `run_migrations_locked` builds their missing indexes first. For each entry in `INLINE_INDEX_BUILDS` (`0076`, `0078`, `0081`), it runs migrations through the version before it, then builds any missing index. Each build is a plain `CREATE INDEX` in its own transaction, under the same 1s `lock_timeout` and 5s `statement_timeout` the migrations use. That is the bounded inline build `0049` already does: - **Small populated databases** upgrade the same way empty ones do. - **Large databases** still fail closed. Only a lock or statement timeout (`55P03` / `57014`) gets the "prebuild concurrently" hint, pointing at that migration's own doc. Other errors pass through unchanged. The blocked migration is not recorded. - **Prebuilt indexes** are found with `to_regclass` and skipped before `CREATE INDEX`, so deployments that prebuilt concurrently never request the writer-conflicting lock. - **Invalid prebuilt indexes** are left for the migration to reject, as before. `CONCURRENTLY` is not used in this path. The runner holds the migration advisory lock, and another replica blocked on that lock holds a snapshot that a concurrent build would wait on. ### Rules - `docs/database.md` Rule 6: with `BUZZ_AUTO_MIGRATE` on, every migration must apply its own change on a populated database and must never refuse one because it has rows. A new index is built inline under the migration's budgets. Only deployments that apply migrations out of band (auto-migration off) prebuild concurrently. - Root `AGENTS.md` (step 4) and `migrations/AGENTS.md` repeat the never-refuse rule. - `docs/artifact-sync-deployment.md` and `docs/profile-search.md` describe the startup behavior first and the concurrent prebuild as the out-of-band path. ## Tests - `populated_database_auto_migrates_through_every_migration` (Postgres) migrates to `0075`, seeds artifact rows, and checks four phases: 1. **Lock held:** while a writer lock is held, the `0076` build fails closed. 2. **Index prebuilt:** with the `0076` index prebuilt, the build is skipped. 3. **Partial `0078`:** a blocked `0078` build keeps the builds before it committed. 4. **Verified profile seeded:** with a verified profile head inserted before `0081`, a final startup succeeds. It asserts that the applied versions equal every embedded migration and that all six indexes are valid, ready and live. - `frozen_prebuild_guards_have_inline_builds` (no database) scans `migrations/`. It fails if any migration refuses a populated table (`RAISE EXCEPTION 'prebuild …`) without an inline build, and if a new guard is added. - `migration_execution_cannot_bypass_schema_destruction_lock` allows exactly one production `run_to`, inside `run_migrations_locked`. ## What got simpler - Auto-migrating relays no longer need a manual, operator-only prebuild step for small deployments. That restores the chart README's contract that `helm upgrade` is the whole upgrade procedure. - Rule 6 no longer steers authors toward refusing populated tables, and a test enforces that. - The change adds one table-driven step for three frozen migrations and no new configuration. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Signed-off-by: Luke Tornquist <tornquist@squareup.com> Co-authored-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -17,8 +17,9 @@ 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; 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.
|
||||
|
||||
|
||||
@@ -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, doc, indexes) in INLINE_INDEX_BUILDS {
|
||||
MIGRATOR.run_to(version - 1, &mut *conn).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
|
||||
// `created_at` floor trigger from migration 0021 — correctly shaped — on
|
||||
@@ -65,6 +69,122 @@ 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, with the document that describes
|
||||
/// its concurrent prebuild, as `(name, definition)`.
|
||||
///
|
||||
/// 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 \
|
||||
ON public.artifact_revisions (community_id, artifact_id)",
|
||||
)],
|
||||
),
|
||||
(
|
||||
78,
|
||||
"docs/artifact-sync-deployment.md",
|
||||
&[
|
||||
(
|
||||
"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",
|
||||
),
|
||||
],
|
||||
),
|
||||
(
|
||||
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
|
||||
/// 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,
|
||||
doc: &str,
|
||||
indexes: InlineIndexBuilds,
|
||||
) -> 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 {
|
||||
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 *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?;
|
||||
}
|
||||
}
|
||||
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 {doc}, then retry"
|
||||
))
|
||||
})
|
||||
}
|
||||
|
||||
/// Run `op` while holding the exclusive schema/destruction session lock.
|
||||
///
|
||||
/// `op` receives ownership of the detached connection that owns the advisory
|
||||
@@ -2074,9 +2194,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;
|
||||
@@ -2113,9 +2233,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
|
||||
@@ -3107,6 +3234,226 @@ mod postgres_tests {
|
||||
.expect("second post-apply reconciliation");
|
||||
}
|
||||
|
||||
/// 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_artifact_tables_auto_migrate_through_every_migration() {
|
||||
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");
|
||||
|
||||
// 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");
|
||||
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(uuid::Uuid::new_v4())
|
||||
.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("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<i64> = MIGRATOR.iter().map(|migration| migration.version).collect();
|
||||
assert_eq!(
|
||||
applied_versions(pool).await,
|
||||
embedded,
|
||||
"a populated database must boot through every embedded migration"
|
||||
);
|
||||
}
|
||||
|
||||
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 = ANY($1) AND i.indisvalid AND i.indisready AND i.indislive",
|
||||
)
|
||||
.bind(names)
|
||||
.fetch_one(pool)
|
||||
.await
|
||||
.expect("read index flags")
|
||||
}
|
||||
|
||||
/// 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<i64> = 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<i64> = 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]
|
||||
#[ignore = "requires Postgres"]
|
||||
async fn pre_0007_ambiguous_nip_rs_data_blocks_without_mutation_and_allows_retry() {
|
||||
|
||||
@@ -13,8 +13,11 @@ 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. 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
|
||||
@@ -39,11 +42,20 @@ 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.
|
||||
|
||||
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. 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`,
|
||||
then verifies and records `0078`. Direct SQL application does not record the
|
||||
|
||||
@@ -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,
|
||||
@@ -410,11 +411,24 @@ 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).
|
||||
- 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 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`). 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`.
|
||||
@@ -456,8 +470,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
|
||||
any prebuild step for deployments that apply migrations out of band.
|
||||
|
||||
## Related documents
|
||||
|
||||
|
||||
@@ -65,9 +65,11 @@ 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
|
||||
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. 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
|
||||
|
||||
@@ -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 crash-loops
|
||||
every auto-migrating deployment with data, however small.
|
||||
|
||||
Reference in New Issue
Block a user