fix(db): index large-value workflow_id FKs so workflow deletes don't seq-scan - #6136
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
PR SummaryMedium Risk Overview Indexes are built Reviewed by Cursor Bugbot for commit 8746c4a. Bugbot is set up for automated code reviews on this repo. Configure here. |
Greptile SummaryAdds concurrent indexes for the nullable workflow foreign keys used by large execution-value tables.
Confidence Score: 4/5This needs the concurrent-index replay path fixed before merging because a failed build can leave the migration complete without a usable index. A partial CREATE INDEX CONCURRENTLY can leave an INVALID relation, and the subsequent IF NOT EXISTS replay skips it while the runner only warns, preserving the workflow-delete sequential scans this change is intended to eliminate. Files Needing Attention: packages/db/migrations/0271_watery_scarlet_spider.sql
|
| Filename | Overview |
|---|---|
| packages/db/migrations/0271_watery_scarlet_spider.sql | Adds the intended concurrent indexes, but replay after a partial build can accept an INVALID index and leave the performance defect unresolved. |
| packages/db/schema.ts | Declares both workflow_id indexes with names and columns matching the migration. |
| packages/db/migrations/meta/0271_snapshot.json | Records both indexes consistently with the schema and follows the previous snapshot ancestry. |
| packages/db/migrations/meta/_journal.json | Correctly appends migration 0271 after migration 0270. |
Reviews (1): Last reviewed commit: "fix(db): index large-value workflow_id F..." | Re-trigger Greptile
| CREATE INDEX CONCURRENTLY IF NOT EXISTS "execution_large_value_references_workflow_id_idx" ON "execution_large_value_references" USING btree ("workflow_id");--> statement-breakpoint | ||
| CREATE INDEX CONCURRENTLY IF NOT EXISTS "execution_large_values_workflow_id_idx" ON "execution_large_values" USING btree ("workflow_id");--> statement-breakpoint |
There was a problem hiding this comment.
Invalid indexes survive migration replay
When either concurrent build fails after creating its catalog entry, the replayed IF NOT EXISTS statement skips the resulting INVALID index, allowing the migration to complete while workflow deletions continue sequentially scanning the corresponding large-value table.
Knowledge Base Used: Database schema and migrations (packages/db)
28fdf04 to
8746c4a
Compare
Summary
execution_large_values.workflow_idandexecution_large_value_references.workflow_idareON DELETE SET NULLFKs (since 0212) with no index leading on that columnUPDATE ... SET workflow_id = NULL WHERE workflow_id = $1once per deleted parent row — so everyDELETE FROM workflowseq-scans both tables per workflow0278adds an index on both columns, builtCONCURRENTLYbelow an embeddedCOMMITper the runner convention inpackages/db/scripts/migrate.tsType of Change
Testing
Applied the migration against a local DB: both indexes build and report
indisvalid = t, and re-running the file is a no-op (already exists, skipping) — required since everything after the embeddedCOMMITruns in autocommit and replays on failure.EXPLAINon the trigger's predicate now showsIndex Scan using execution_large_values_workflow_id_idx(forced withenable_seqscan = off; the local table only has ~2.4k rows).bun run check:migrations origin/staging,bun run lint, and the full audit suite (check:boundaries,check:api-validation:strict,check:utils,check:zustand-v5,check:react-query,check:client-boundary,check:bare-icons,check:icon-paths,check:realtime-prune,skills:check,agent-stream-docs:check) all pass.bun run type-checkexits 0.Note for deploy:
CONCURRENTLYdoes two table passes and waits out in-flight transactions, so the build will run for a while on prod-sized tables — worth checkingindisvalidafter, since a failed concurrent build leaves an INVALID index that the nextIF NOT EXISTSsilently skips.Checklist