Commit d96f3f9
authored
perf(db): drop 0287's two zero-row scans from the ACCESS EXCLUSIVE hold (#6609)
drizzle-orm 0.45.2 runs every pending migration inside ONE transaction
(node_modules/drizzle-orm/pg-core/dialect.js:60-71), and migrate.ts:212
sets `statement_timeout = 0`. So the ACCESS EXCLUSIVE that 0287:2's
ADD COLUMN takes on `workflow_blocks` is held, unbounded, through 0288
and 0289 to COMMIT — `lock_timeout` at migrate.ts:213 bounds acquisition
only, exactly as that file's own TSDoc at :76-78 says. Every editor load,
workflow save, executor block read, and realtime canvas op queues behind
it platform-wide, and migrations run before image promotion
(ci.yml:113-133), so the stall lands on 100% old-version traffic.
Two of the three statements inside that hold did nothing. `data.errorEnabled`
never existed in a released version — `git log origin/main -S errorEnabled`
returns zero commits across main's entire history — so both statements
filtered on it match zero rows, and the file's own comment said as much.
There is no index on the `data` expression, so each was a full sequential
scan of the whole table.
Measured on PostgreSQL 17.9 against a 328 MB / 200k-row fixture built to
the same bytes-per-row shape as the reported production table:
0287:2 ADD COLUMN 0.5 ms metadata-only, takes AccessExclusiveLock
0287:11 edge backfill 21 ms Nested Loop -> Index Scan on the PK
0287:20 (deleted) 47 ms Seq Scan, 200,000 rows removed, 0 matched
0287:22 (deleted) 47 ms Seq Scan, 200,000 rows removed, 0 matched
A concurrent primary-key SELECT started 50 ms into the transaction was
blocked 53-219 ms before and 22-25 ms after — the latter indistinguishable
from the 21-33 ms control with no migration running at all. The two deleted
statements accounted for 80,520 buffer accesses (~629 MB of in-lock I/O on
that fixture) and 81% of the transaction's work.
Editing an already-merged migration in place is safe here specifically
because drizzle writes `hash` but never reads it back: the skip test at
dialect.js:56-63 compares `created_at` against `folderMillis` only. The
edit is therefore a no-op on every database that already applied 0287
(staging, dev, branch DBs) and takes effect only where it has not run.
`meta/_journal.json` and the snapshot prevId chain are untouched.
The only casualty is a stale `data.errorEnabled` key on branch databases a
developer created it on. Nothing reads it: save.ts:50 and load.ts:89 both
read the `error_enabled` COLUMN, and load.ts passes `data` through
untouched.
Alternatives rejected, each checked against the code rather than assumed:
- An embedded `COMMIT;` to end the transaction early. 0289's four
CREATE TYPE, its CREATE TABLE, and its three CREATE INDEX all lack
IF NOT EXISTS, so a mid-batch failure in autocommit leaves them
applied-but-unjournaled and the replay dies on 42710 — which
migrate.ts:219 only retries for 55P03. That turns a transient stall
into a wedged deploy.
- Moving the statements to a new file after 0289. All pending files share
one transaction, so it buys exactly zero lock reduction.
- Rewriting the surviving backfill as
`WHERE id IN (SELECT source_block_id FROM workflow_edges WHERE ...)`.
Measured both: planner-equivalent. `source_handle` is unindexed, so both
forms seq-scan `workflow_edges` (7.5 ms, identical) and then index-scan
`workflow_blocks` on the primary key — it never scans that table. The
IN form adds a HashAggregate and 1,170 more buffers, so it is marginally
worse. Left as shipped.
- Promoting the `data-backfill` lint from warn to annotate. `readAnnotation`
only requires a non-empty reason and 0287 already supplies one per
statement, so the rule would fire zero findings.
0288's nullable `retry` column is correct as-is and unchanged. Both
delete-and-reinsert save paths on the deployed version (save.ts:30-63 and
the realtime REPLACE_STATE handler) reset `error_enabled` to false and
`retry` to NULL for a workflow saved by an old replica during the rollout;
the ordinary realtime block upsert does not, because its `set` clause
omits both columns. That residue is tolerable: everything the surviving
backfill writes is re-derived from the edge set at
workflow-block.tsx:754 and lib/workflows/persistence/utils.ts:196-201, and
`retry` ships in this same release so it has no installed base.
`bun run check:migrations origin/main` drops from three data-backfill
warnings to one. The real migrator applied all 289 journal entries to a
fresh PostgreSQL 17.9 database with the edited file, producing
`error_enabled boolean not null default false` and `retry jsonb`.1 parent d1bc99a commit d96f3f9
1 file changed
Lines changed: 1 addition & 9 deletions
Lines changed: 1 addition & 9 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
11 | 11 | | |
12 | 12 | | |
13 | 13 | | |
14 | | - | |
15 | | - | |
16 | | - | |
17 | | - | |
18 | | - | |
19 | | - | |
20 | | - | |
21 | | - | |
22 | | - | |
| 14 | + | |
0 commit comments