[TheSentinel454]

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

[github-actions]

🔐 Codex Security Review

Note: This is an automated, security-focused review generated by Codex.
Use it as a supplement to human review; false positives are possible.

Scope

• Exact PR diff: ec21753e1b4ca3cd0f208b5fb42cb22a1e29a433...a481b5bbd5c157d49cd8f78d60e43014272e9c44
• Model: gpt-6.1-sol

💡 Click "edited" above to see earlier reviews for this PR.

---

Review Summary

Overall Risk: NONE

No concrete security, correctness, or reliability issues found in the authorized PR range.

Findings

No concrete security, correctness, or reliability findings were identified.

Notes

• Review used read-only source and history inspection. Builds and tests were not run.

---

_{Generated by Codex Security Review |
Requested by: @TheSentinel454 |
Workflow run}

[wpfleger96]

🤖 Automated review at exact head 538cda70edc88e4d5778a7475ea35b30d5a3537e (base f3a7bab098a530ba12b909cb7121b4fa9ce70731). No blocking findings.

The staged runner holds up. With sqlx 0.9, run_to(target) only limits which pending migrations get applied. On a database already at 76–78 the extra run_to calls just validate checksums, and the _sqlx_migrations check skips the builds. A dirty database or one with unknown migrations fails before any build runs. All five definitions in INLINE_INDEX_BUILDS produce exactly the pg_get_indexdef strings 0076 and 0078 verify, including the WHERE (source_channel_id IS NOT NULL) predicate. Each build commits on its own on the connection that keeps holding the schema/destruction lock, and a failed build rolls back without leaving an invalid index behind. buzz-admin migrate goes through db.migrate() → run_migrations, so the deployment doc is right that it builds the indexes too.

We also ran it against a throwaway Postgres 17. Both new tests pass. A real upgrade works too: apply the base's SQL migrations 1–75, seed artifact heads, revisions and a membership, then run the head's run_migrations. It reaches 78 with all five indexes valid, ready and live, and existing rows are unchanged. A repeat startup and two simultaneous migration calls both succeed. A same-named index with the wrong definition fails closed at 75 with the documented error, and the retry succeeds after dropping it. That upgrade also covers the path the new test doesn't: its final phase prebuilds the 0076 index, so the test never shows a successful inline build of that one.

MINOR (docs): docs/database.md now contradicts itself. Rule 6 still says an index on a populated table MUST be built CONCURRENTLY, and the next sentence says the migration builds a missing one inline. Rule 3 says a migration MUST NOT scan a table while holding a write-blocking lock, and a plain CREATE INDEX does exactly that. A future author can't satisfy all three. It'd be clearer to make the bounded inline build an explicit exception in both rules, keep the concurrent prebuild for tables too big for the budget, and move the "0076 and 0078 did" history into docs/artifact-sync-deployment.md. migrations/AGENTS.md should get the same wording.

MINOR (docs): docs/artifact-sync-deployment.md still opens with "For a populated database, first build the revision lookup index outside a transaction" and still says 0078 "refuses a populated table whose index is missing". Both are now true only for direct SQL application. Saying up front that startup migration handles small databases, and the concurrent prebuild is for large ones, would keep operators from doing a manual step they no longer need.

MINOR: build_indexes_before_migration maps every sqlx::Error to "migration N blocked … prebuild them concurrently". That advice is wrong for a dropped connection or a permission error. The underlying error is included, so this isn't misleading for long. Still, adding the prebuild hint only for lock or statement timeouts (SQLSTATE 55P03 / 57014) and passing other errors through unchanged would be more accurate.

[ShapedTime]

Data point from a self-hosted relay that hit this on main-20261102025339-d342cf1 (chart 0.1.11, Postgres 18.6, BUZZ_AUTO_MIGRATE=true). Two things that may affect how urgent this is:

It doesn't take any artifact data. The description scopes this to databases with artifact rows, but 0078 also covers channel_members_sync. Both of our artifact tables were empty, so 0076 passed, and then 0078 stopped on channel_members, which had 291 rows:

migration error: while executing migration 78: error returned from database:
prebuild channel_members_sync concurrently; see docs/artifact-sync-deployment.md

So a relay with a single channel member hits it, which is effectively every deployment in use.

A failed upgrade can't be rolled back by changing the image. Migrations commit one at a time, so 0052 to 0077 were already recorded when 0078 failed. The image we came from then refuses the database:

migration 52 was previously applied but is missing in the resolved migrations

With a rolling update the old pod keeps serving, so this looks fine until that pod restarts, and reverting the tag doesn't help. We confirmed that message by running the old image's buzz-admin migrate against a copy of the schema, not by restarting the production pod.

Workaround, for anyone landing here from that error: build the four indexes from docs/artifact-sync-deployment.md with psql. The doc then says to run buzz-admin migrate, but a crash-looping pod has nothing to exec into, and it isn't needed: the relay applied 0078 to 0080 itself on its next start and came up Ready.

From reading the diff, indexes prebuilt this way are skipped by the to_regclass check, and a database already at 0078 skips the builds entirely, so the workaround shouldn't conflict with this PR.

[TheSentinel454]

🤖 Thanks Will — all three addressed at a481b5b: Rules 3/6 reconciled (bounded inline build is the explicit exception; CONCURRENTLY prebuild only when auto-migrate is off), artifact-sync doc leads with the startup build, and the prebuild hint is limited to SQLSTATE 55P03/57014 with other errors passed through. This also covers 0081 now.

[wpfleger96]

🤖 Automated review at exact head a481b5bbd5c157d49cd8f78d60e43014272e9c44 (base ec21753e1b4ca3cd0f208b5fb42cb22a1e29a433). No blocking findings.

The three notes from the last review are all addressed. Rules 3 and 6 now agree, the artifact sync doc describes startup behavior first, and the prebuild hint only appears on a lock or statement timeout (55P03 / 57014).

The new 0081 entry in INLINE_INDEX_BUILDS is byte-for-byte the CREATE INDEX … USING gin (%s) string that 0081 checks with pg_get_indexdef. It's a GIN to_tsvector('simple', …) expression index with no predicate or extension dependency. The staged sequence (through 75, build 76, through 77, build 78, through 80, build 81, then the rest) is correct for fresh databases, ones already at 76–81, and dirty ones: with sqlx 0.9, run_to caps which pending migrations run, and dirty history fails before any build. frozen_prebuild_guards_have_inline_builds can't pass on an empty scan, because it also pins the list to [76, 78, 81]. Adding a temporary 0083 with a 'prebuild …' guard makes it fail.

We also ran it against a throwaway Postgres. All three new tests pass. A real upgrade works too: apply the base's migrations through 0080, seed a verified profile head, artifact heads/revisions and a channel member, then run the head's run_migrations. It reaches 0082 with the 0081 index valid, ready and live, and every seeded row unchanged. A restart is a no-op. While a writer lock is held on nip_fi_profile_heads, startup fails after about 1s with the 0081 hint pointing at docs/profile-search.md. 0081 isn't recorded and no partial index is left, and the retry succeeds once the lock is released. With a correct index prebuilt, startup succeeds even while the writer lock is still held, and the index keeps its OID, so the build was skipped. A same-named index with the wrong definition fails closed at 0081.

MINOR (docs): A large relay with auto-migration on that goes over the budget is told to "prebuild them concurrently as described in docs/profile-search.md" (or docs/artifact-sync-deployment.md). Those docs and Rule 6 now present the concurrent prebuild as the step for deployments with auto-migration off. That leaves an operator who hits the error on an auto-migrating relay unsure whether the procedure applies to them. One sentence in each doc saying an auto-migrating relay that fails the budget can prebuild concurrently and restart would fix it. Same gap in docs/thread-window-deployment.md, which Rule 6 links to: lines 12–16 still say all brownfield deployments must prebuild before starting the relay. That intro should get the same small/auto-migrating vs. large/out-of-band split.

MINOR (PR description): The Tests section describes a single four-phase populated_database_auto_migrates_through_every_migration. The code now has two tests: populated_artifact_tables_auto_migrate_through_every_migration and populated_verified_profiles_auto_migrate_through_every_migration.