From 7e6f936756d9024071b82b074c2414c81638a439 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Wed, 12 Aug 2026 01:02:57 -0700 Subject: [PATCH] perf(db): drop 0287's two zero-row scans from the ACCESS EXCLUSIVE hold MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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`. --- .../migrations/0287_workflow_blocks_error_enabled.sql | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/packages/db/migrations/0287_workflow_blocks_error_enabled.sql b/packages/db/migrations/0287_workflow_blocks_error_enabled.sql index d6839cc66f9..951ad0cb8f2 100644 --- a/packages/db/migrations/0287_workflow_blocks_error_enabled.sql +++ b/packages/db/migrations/0287_workflow_blocks_error_enabled.sql @@ -11,12 +11,4 @@ ALTER TABLE "workflow_blocks" ADD COLUMN "error_enabled" boolean DEFAULT false N UPDATE "workflow_blocks" AS b SET "error_enabled" = true FROM "workflow_edges" AS e -WHERE e."source_block_id" = b."id" AND e."source_handle" = 'error';--> statement-breakpoint --- The flag also briefly persisted inside `data` on this unmerged branch and never --- reached production. These move any row a developer created onto the column and --- leave the value one home, so a block saved before the change stops differing --- from one saved after it. Both match zero rows on a database that never saw the key. --- migration-safe: idempotent (re-running sets the same value and removes an already-absent key); the key is written by no released version, so no concurrent writer can reintroduce it. -UPDATE "workflow_blocks" SET "error_enabled" = true WHERE "data" ->> 'errorEnabled' = 'true';--> statement-breakpoint --- migration-safe: idempotent (re-running removes an already-absent key); the key is written by no released version, so no concurrent writer can reintroduce it. -UPDATE "workflow_blocks" SET "data" = "data" - 'errorEnabled' WHERE "data" ->> 'errorEnabled' IS NOT NULL; +WHERE e."source_block_id" = b."id" AND e."source_handle" = 'error';