Repository navigation
feat(db): concurrent index builds and safe concurrent index drops - #943
Conversation
An entity index can declare `concurrently: true`; `x db gen` emits it as a non-transactional step (`-- transaction: false`): `create index concurrently if not exists`, alone when the diff is index work only, else as an `<id>_concurrently` follow-up migration after the transactional one. Every index removed from an entity is dropped with `drop index concurrently if exists` in that step (a constraint-backed unique index drops its constraint first, in the transactional step). The runner applies a marked migration statement by statement on the lock-holding session, with no lock_timeout (a concurrent build waits on older transactions by design), writes the ledger row last, and drops an INVALID index left by an interrupted build before building it again. X_MIGRATION_CONCURRENT_IN_TRANSACTION refuses a concurrent statement in an unmarked migration. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
🟡 Reviewed1 actionable comment(s) · grounded on your code ⏱ 32s wall clock · 🤖 developerz.ai — automated review, running on your model and your box. |
There was a problem hiding this comment.
Review summary — 46 file(s), 6 finding(s).
Critical 0 · Major 2 · Minor 4 · Nit 0
[bot] completed: Reviewed concurrent index generation and CLI projection; found constraint-removal rollback gaps, a uniqueness gap during concurrent replacement, and an inaccurate intermediate snapshot. [bot] completed: Reviewed concurrent migration execution and recovery.
| File | Findings |
|---|---|
packages/db/src/concurrent-index.live.test.ts |
1 minor |
packages/db/src/concurrent-index.ts |
1 minor |
packages/db/src/concurrent-plan.ts |
2 major, 1 minor |
wiki/Migrations-And-Backfills.md |
1 minor |
🤖 developerz.ai review — automated, running on your model and your box. What is this? · Reviewed by router/gpt-6.1-sol on box sebastian-1 — 759.9k in / 5.67k out over 19 calls; cost unknown: no price on record for router/gpt-6.1-sol.
|
Address review findings on #943
Queued — waiting for a free box. Sessions: review 1 · review 2 · review 3 · review 4 🤖 developerz.ai, an automated agent, not a human — developerz-ai[bot] |
…y build; REINDEX parsed whole; the lock_timeout test proves the wait - A constraint-backed unique index is unhooked and dropped in the non-transactional step, after every concurrent build (a renamed key's replacement exists first); the transactional step drops no key, so its rollback owes none and its snapshot keeps it. The unhook runs in its own short transaction under SET LOCAL lock_timeout. - REINDEX: the full option list and CONCURRENTLY [boolean] are parsed. - The live test waits until the build is seen blocked, outlives 200 ms, then releases the reader. - Wiki and README: one file, one non-transactional file, or two — only concurrent work splits. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Review summary — 16 file(s), 3 finding(s).
Critical 0 · Major 1 · Minor 2 · Nit 0
[bot] completed — The increment moves constraint removal after replacement builds and strengthens REINDEX detection. Two execution edge cases remain: transaction-forbidden handwritten statements and quoted REINDEX options. [bot] completed: Reviewed the concurrent-step fixes.
2 non-blocking finding(s) below major (no thread on a re-review — only blocking findings open threads)
packages/db/README.md:183minor · docs [bot] The claim that every build precedes every drop is false for a same-name definition change: redefineConcurrently explicitly emits its drop before its build. Qualify the ordering guarantee here and in the wiki Order row to exclude same-name redefinitions. Also at…packages/db/src/concurrent-index.ts:113minor · bug [bot]REINDEX (VERBOSE 'true') INDEX CONCURRENTLY idxis missed because leadingTokens stops at the string literal, so preflight permits it inside BEGIN and PostgreSQL returns 25001. Parse quoted option values without terminating the REINDEX prefix scan and add this regression case.
🤖 developerz.ai review — automated, running on your model and your box. What is this? · Reviewed by router/gpt-6.1-sol on box hv-dedicated-6 — 472.3k in / 2.6k out over 9 calls; cost unknown: no price on record for router/gpt-6.1-sol.
…unded transaction; vacuum and its kin run autocommitted Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Review summary — 2 file(s), 1 finding(s).
Critical 0 · Major 1 · Minor 0 · Nit 0
[bot] Completed review: restores autocommit execution for maintenance statements, but also removes lock-timeout protection from other blocking DDL in marked migrations.
🤖 developerz.ai review — automated, running on your model and your box. What is this? · Reviewed by router/gpt-6.1-sol on box hv-dedicated-dev-din — 79.7k in / 661 out over 4 calls; cost unknown: no price on record for router/gpt-6.1-sol.
… autocommits; every other blocking statement keeps lock_timeout Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Review summary — 2 file(s), 1 finding(s).
Critical 0 · Major 1 · Minor 0 · Nit 0
[bot] completed: Extends lock-timeout protection to non-concurrent statements in marked migrations, but misclassifies table-specific CLUSTER and REINDEX SCHEMA, causing unbounded blocking or migration failure.
🤖 developerz.ai review — automated, running on your model and your box. What is this? · Reviewed by router/gpt-6.1-sol on box hv-dedicated-6 — 57.6k in / 2.02k out over 4 calls; cost unknown: no price on record for router/gpt-6.1-sol.
…he bounded transaction Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Review summary — 2 file(s), 1 finding(s).
Critical 0 · Major 0 · Minor 1 · Nit 0
[bot] completed: The increment bounds table-specific CLUSTER operations while preserving autocommit for bare CLUSTER, but misses the parenthesized option syntax.
🤖 developerz.ai review — automated, running on your model and your box. What is this? · Reviewed by router/gpt-6.1-sol on box sebastian-1 — 52.6k in / 418 out over 2 calls; cost unknown: no price on record for router/gpt-6.1-sol.
Why
Gaps found while running a production app (tesote/bank-integrations,
docs/upstream-issues.md§21–22):x db genwrote every index as a plaincreate indexinside the migration's transaction. That takes a SHARE lock on a hot table and races the migrate role'slock_timeout.create index concurrently.drop index, which takes ACCESS EXCLUSIVE. The app's gate refuses that, so it kept three redundant indexes on its hottest table.What
indexes: [{ on: [...], concurrently: true }], the same shape asusing.-- transaction: false:create index concurrently if not exists …, with a concurrent drop in itsdown.transactional: false. A mixed diff becomes<id>.sql(transactional) plus an<id>_concurrently.sqlfollow-up, each with its own snapshot and hash (GeneratedMigration.followUp,x db gen --json).drop index concurrently if existsin the non-transactional step, and itsdownrebuilds it concurrently.2BP01, measured on PG 18.6) drops its constraint in the transactional step first.lock_timeout. A concurrent build waits on older transactions, which is a lock wait; a live test proves a 200 ms limit fails it with55P03.X_MIGRATION_CONCURRENT_IN_TRANSACTIONrefuses a concurrent statement in an unmarked migration before the transaction opens, instead of letting Postgres fail mid-transaction with25001.-- destructive: true(marker-line.ts).Tests
25001premise, interrupted-build recovery, the no-lock_timeout proof, and the measured2BP01row. 1280 db tests pass.x db geninexamples/dummyanddummy/social-media-clonemakes no new migration.bun run verifyis green (14 of 20; the app-only steps skip at the repo root). Theserve-graphmodule ceilings are raised by 6 with reasons. CHANGELOG[Unreleased], db and entity READMEs, the wiki andllms.txtare updated.🤖 Generated with Claude Code