From 13fdfab44697a96da355dc3e1739456ba9643c81 Mon Sep 17 00:00:00 2001 From: Remon Oldenbeuving Date: Wed, 5 Aug 2026 16:49:05 +0200 Subject: [PATCH] fix(migrations): make deploy-time DDL lose the lock race, not user traffic Migration 0204 deadlocked against live traffic on five consecutive production deploys, killing 13 user requests across /api/users/notifications and /api/webhooks/github before it landed. The cause was relation-level lock inversion. DROP TABLE takes ACCESS EXCLUSIVE on the target and on every table it references by foreign key, because dropping the constraint removes the referential-integrity trigger on the parent. 0204 dropped 12 tables holding 21 foreign keys into kilocode_users, organizations, and microdollar_usage, so it needed ACCESS EXCLUSIVE on two of the busiest tables in the schema at once. Ordinary queries touching both in the other order completed the cycle, and the deadlock detector aborts whichever party notices it -- in practice user requests rather than the migration. Production has lock_timeout = 0, so the migration sat in the lock queue indefinitely and stayed exposed to the detector. Measured locally, a conflicting statement waits 2.60s without lock_timeout versus failing at 0.63s with it, against a deadlock_timeout of 1000ms. Apply migrations with a runner that sets lock_timeout below deadlock_timeout, so the migration is the only casualty, and retries lock failures so a transient collision costs seconds instead of a deploy. A custom runner is required because drizzle-kit builds its own pg.Pool and zod-strips unknown dbCredentials keys, so lock_timeout cannot be injected through drizzle.config.ts. It also discarded every error field: the failed deploy printed only "applying migrations...undefined". The runner delegates to the same drizzle-orm migrator, so migration semantics are unchanged. It uses a single pg.Client rather than a Pool because the migrator runs the whole batch in one transaction on one connection, which keeps SET lock_timeout in force with no pool checkout to race against. Behaviour worth knowing: - It refuses to start when lock_timeout is 0 or >= deadlock_timeout, asserted against pg_settings rather than assumed. - Only 40P01, 55P03, and 40001 are retried. Real migration bugs still fail on the first attempt. - 11 existing migrations break out of the transaction with a bare COMMIT; for CREATE INDEX CONCURRENTLY. Replaying those would leave earlier statements applied but unrecorded, so they are detected and run with retries disabled. Only the production and staging deploy jobs are switched over. `pnpm drizzle migrate` is untouched, so local development, test:db, verify-bootstrap, chromatic, setup-smoke, and kiloclaw dev-start behave exactly as before. Documentation records the rules this incident exposed: the FK-parent locking rule and the per-constraint decomposition that avoids it, the single-transaction batching hazard, and the folderMillis skip hazard (the migrator compares each journal entry against max(created_at), so an entry older than the newest applied migration is skipped permanently and silently). --- .github/workflows/deploy-production.yml | 2 +- .github/workflows/deploy-staging.yml | 2 +- .kilo/skills/database-migrations/SKILL.md | 45 +++- DEVELOPMENT.md | 1 + REVIEW.md | 2 + package.json | 1 + packages/db/AGENTS.md | 38 +++ packages/db/src/migrate-cli.ts | 15 ++ packages/db/src/migrate.test.ts | 207 +++++++++++++++ packages/db/src/migrate.ts | 299 ++++++++++++++++++++++ 10 files changed, 604 insertions(+), 8 deletions(-) create mode 100644 packages/db/src/migrate-cli.ts create mode 100644 packages/db/src/migrate.test.ts create mode 100644 packages/db/src/migrate.ts diff --git a/.github/workflows/deploy-production.yml b/.github/workflows/deploy-production.yml index 89cdcecccf..a0d77777aa 100644 --- a/.github/workflows/deploy-production.yml +++ b/.github/workflows/deploy-production.yml @@ -75,7 +75,7 @@ jobs: run: pnpm install --frozen-lockfile - name: Run Drizzle migrations - run: NODE_ENV=production pnpm run drizzle migrate + run: NODE_ENV=production pnpm run drizzle:migrate-safely stage-app: uses: ./.github/workflows/stage-vercel-deployment.yml diff --git a/.github/workflows/deploy-staging.yml b/.github/workflows/deploy-staging.yml index 2ac477e00e..2d04a346d2 100644 --- a/.github/workflows/deploy-staging.yml +++ b/.github/workflows/deploy-staging.yml @@ -71,7 +71,7 @@ jobs: run: pnpm install --frozen-lockfile - name: Run Drizzle migrations - run: NODE_ENV=production pnpm run drizzle migrate + run: NODE_ENV=production pnpm run drizzle:migrate-safely deploy-app: needs: [check-staging-db-startup, run-migrations] diff --git a/.kilo/skills/database-migrations/SKILL.md b/.kilo/skills/database-migrations/SKILL.md index 0b39b80b2f..f5f776a0d8 100644 --- a/.kilo/skills/database-migrations/SKILL.md +++ b/.kilo/skills/database-migrations/SKILL.md @@ -18,11 +18,44 @@ This skill covers shared PostgreSQL migrations only. It does not govern Durable broad, correct the schema and regenerate. Do not hand-edit generated DDL, snapshots, or journal entries. 6. Append only intentional `UPDATE` or `INSERT` data backfills after generated DDL, separated with `--> statement-breakpoint`. -7. Apply migrations with `pnpm drizzle migrate` or run `pnpm drizzle:verify-bootstrap` when relevant. -8. Run `pnpm format` and targeted schema or migration checks. -9. Prefer one generated migration per unshipped feature branch. To squash - migrations before shipping, remove the branch-local migration SQL, snapshots, - and journal entries, then regenerate once from the current schema. Re-append - intentional backfills afterward. +7. Check lock safety against live traffic before shipping DDL; see the section below. +8. Apply migrations with `pnpm drizzle migrate` locally, or `pnpm drizzle:migrate-safely` to rehearse the deploy path. Run `pnpm drizzle:verify-bootstrap` when relevant. +9. Run `pnpm format` and targeted schema or migration checks. +10. Prefer one generated migration per unshipped feature branch. To squash + migrations before shipping, remove the branch-local migration SQL, snapshots, + and journal entries, then regenerate once from the current schema. Re-append + intentional backfills afterward. Keep generated artifacts generated. `packages/db/AGENTS.md` also covers shared-schema PII requirements and DB-backed timestamp serialization. + +## Lock safety review + +`packages/db/AGENTS.md` is canonical; this is the checklist. + +- Identify every table the DDL will lock with `ACCESS EXCLUSIVE`. That includes + the target table **and every table it references by foreign key**, since + dropping a constraint removes the referential-integrity trigger on the parent. + A `DROP TABLE ... CASCADE` also locks the children whose constraints it drops. +- Treat `kilocode_users`, `organizations`, and `microdollar_usage` as always + busy. A statement that needs `ACCESS EXCLUSIVE` on two of them at once will + deadlock with ordinary traffic that touches both in the other order. +- Split such work so each statement locks at most one busy table: drop the + foreign keys one statement at a time, then drop the table. +- Confirm the new journal `when` is the newest in `meta/_journal.json`, or the + migrator will skip the migration silently. +- Verify with `SHOW deadlock_timeout` before changing `MIGRATION_LOCK_TIMEOUT`; + the timeout must stay below it. + +Query the locks a statement will take before shipping anything non-trivial: + +```sql +select + c.conname, + src.relname as from_table, + tgt.relname as to_table +from pg_constraint c +join pg_class src on src.oid = c.conrelid +join pg_class tgt on tgt.oid = c.confrelid +where c.contype = 'f' + and 'YOUR_TABLE' in (src.relname, tgt.relname); +``` diff --git a/DEVELOPMENT.md b/DEVELOPMENT.md index 6877e16f06..486d46a9f7 100644 --- a/DEVELOPMENT.md +++ b/DEVELOPMENT.md @@ -251,6 +251,7 @@ should pass against the local PostgreSQL database. | `pnpm format:changed` | Format only files changed since `main` | | `pnpm validate` | Run the root typecheck, lint, and test scripts | | `pnpm drizzle migrate` | Apply pending database migrations | +| `pnpm drizzle:migrate-safely` | Apply pending migrations the way deploys do: `lock_timeout` below `deadlock_timeout`, retries on lock failures, and full PostgreSQL error detail on failure | | `pnpm drizzle generate` | Generate a new migration after schema changes | | `pnpm drizzle:verify-bootstrap` | Create a temporary empty database and verify `pnpm drizzle migrate` bootstraps it cleanly | | `pnpm dev:db:reset` | Drop all app-owned schemas in the local dev database, recreate `public`, and leave the DB truly empty before re-migrating | diff --git a/REVIEW.md b/REVIEW.md index 758a99e35c..e07236bc9b 100644 --- a/REVIEW.md +++ b/REVIEW.md @@ -25,6 +25,8 @@ - Dropping columns/tables that may still be read by running application code - Large backfills or data transforms without batching - Missing partial index opportunities (e.g. `WHERE col IS NOT NULL`) +- A single statement needing `ACCESS EXCLUSIVE` on more than one busy table - `DROP TABLE`/`ALTER TABLE` also locks every table the target references by foreign key, so dropping a table with FKs to both `kilocode_users` and `organizations` deadlocks against normal traffic. Flag it and ask for the foreign keys to be dropped one statement at a time before the table +- A new migration whose journal `when` is not the newest in `meta/_journal.json` - the migrator compares against `max(created_at)` and skips older entries silently # COMMENT FORMAT diff --git a/package.json b/package.json index defa160d32..4fc0dec890 100644 --- a/package.json +++ b/package.json @@ -19,6 +19,7 @@ "format:changed": "git diff --name-only $(git merge-base origin/main HEAD) --diff-filter=ACMR -- '**/*.js' '**/*.jsx' '**/*.ts' '**/*.tsx' '**/*.json' '**/*.css' '**/*.md' | xargs -r oxfmt --no-error-on-unmatched-pattern", "validate": "pnpm run typecheck && pnpm run lint && pnpm run test", "drizzle": "pnpm --filter @kilocode/db exec drizzle-kit", + "drizzle:migrate-safely": "tsx packages/db/src/migrate-cli.ts", "drizzle:verify-bootstrap": "bash scripts/verify-drizzle-bootstrap.sh", "test:e2e": "pnpm --filter web run test:e2e", "dependency-cycle-check": "pnpm --filter web run dependency-cycle-check", diff --git a/packages/db/AGENTS.md b/packages/db/AGENTS.md index c34bf481b9..93761b0c64 100644 --- a/packages/db/AGENTS.md +++ b/packages/db/AGENTS.md @@ -30,6 +30,44 @@ Prefer one generated migration per unshipped feature branch. Load `database-migrations` for shared PostgreSQL migration work. Load `git-rebase` for migration conflicts during a rebase. +## Lock safety + +Deploys apply migrations against a database that is serving traffic, so DDL +competes with live queries for relation-level locks. + +`DROP TABLE` and `ALTER TABLE` take `ACCESS EXCLUSIVE` on the target table **and +on every table the target references by foreign key**, because dropping the +constraint removes the referential-integrity trigger that lives on the parent. +A migration touching a table with foreign keys to `kilocode_users` and +`organizations` therefore locks both of those, and any concurrent query that +holds one while waiting for the other deadlocks. PostgreSQL then aborts whichever +party detects the cycle, which in practice is user requests rather than the +migration. + +Two consequences when writing migrations against hot parent tables: + +- Prefer dropping each foreign key in its own statement over one `DROP TABLE + ... CASCADE`, so a single statement never needs `ACCESS EXCLUSIVE` on more + than one busy table at a time. Once the constraints are gone, the table drop + needs no lock on any parent. +- Migrations are applied by `pnpm drizzle:migrate-safely`, which sets + `lock_timeout` below the server's `deadlock_timeout` so the migration loses + the lock race instead of user queries, and retries lock failures. A migration + that breaks out of the migrator's transaction with a bare `COMMIT;` (the + `CREATE INDEX CONCURRENTLY` workaround) cannot be replayed, so it runs with + retries disabled — keep those migrations small and expect no retry safety net. + +The migrator applies every pending migration in one transaction, so locks taken +by the first statement are held until the last migration commits. Prefer one +migration per deploy over letting a batch accumulate. + +The migrator decides what is pending by comparing each journal entry's `when` +against `max(created_at)` in `drizzle.__drizzle_migrations`, not by checking each +entry individually. A migration whose journal timestamp predates an +already-applied migration is skipped permanently and silently, which is a real +risk after a rebase or a long-lived branch. Confirm a new migration's `when` is +the newest in `meta/_journal.json` before merging. + When adding user or account PII to shared PostgreSQL, update `softDeleteUser` in `apps/web/src/lib/user/index.ts` to delete or anonymize it and add corresponding coverage in `apps/web/src/lib/user/index.test.ts`. diff --git a/packages/db/src/migrate-cli.ts b/packages/db/src/migrate-cli.ts new file mode 100644 index 0000000000..58b7ca52bc --- /dev/null +++ b/packages/db/src/migrate-cli.ts @@ -0,0 +1,15 @@ +#!/usr/bin/env tsx +/** + * Entry point for `pnpm drizzle:migrate-safely`. + * + * Kept separate from `migrate.ts` so importing the runner (in tests) never + * connects to a database or applies migrations as an import side effect. + */ +import process from 'node:process'; + +import { reportFailure, runMigrations } from './migrate'; + +runMigrations().catch((error: unknown) => { + reportFailure(error); + process.exitCode = 1; +}); diff --git a/packages/db/src/migrate.test.ts b/packages/db/src/migrate.test.ts new file mode 100644 index 0000000000..3dc9b6a7db --- /dev/null +++ b/packages/db/src/migrate.test.ts @@ -0,0 +1,207 @@ +import { + applyWithRetries, + findFailingQuery, + findPostgresError, + findTransactionBreakingMigrations, + MigrationSafetyError, + reportFailure, +} from './migrate'; + +/** Shaped like a `pg` error: the code lives on the error object itself. */ +function postgresError(code: string, message = 'boom'): Error { + return Object.assign(new Error(message), { code }); +} + +/** Shaped like DrizzleQueryError: the pg error is the `cause`. */ +function wrappedPostgresError(code: string, query: string): Error { + return Object.assign(new Error(`Failed query: ${query}`), { + query, + cause: postgresError(code, 'deadlock detected'), + }); +} + +const noSleep = async () => {}; + +describe('findPostgresError', () => { + it('reads the code off a bare pg error', () => { + expect(findPostgresError(postgresError('40P01'))?.code).toBe('40P01'); + }); + + it('unwraps the cause chain that drizzle wraps the pg error in', () => { + const error = wrappedPostgresError('40P01', 'drop table "x"'); + + expect(findPostgresError(error)?.code).toBe('40P01'); + expect(findPostgresError(error)?.message).toBe('deadlock detected'); + }); + + it('returns undefined when nothing in the chain carries a code', () => { + expect(findPostgresError(new Error('plain'))).toBeUndefined(); + expect(findPostgresError('not an error')).toBeUndefined(); + }); + + it('surfaces the fields drizzle-kit discards', () => { + const error = Object.assign(new Error('deadlock detected'), { + code: '40P01', + detail: 'Process 123 waits for AccessExclusiveLock on relation 456', + hint: 'See server log for query details.', + table: 'organizations', + }); + + expect(findPostgresError(error)).toMatchObject({ + code: '40P01', + detail: 'Process 123 waits for AccessExclusiveLock on relation 456', + hint: 'See server log for query details.', + table: 'organizations', + }); + }); +}); + +describe('findFailingQuery', () => { + it('reads the query drizzle attaches to the wrapper', () => { + expect(findFailingQuery(wrappedPostgresError('55P03', 'drop table "y"'))).toBe( + 'drop table "y"' + ); + }); + + it('returns undefined when no wrapper carries a query', () => { + expect(findFailingQuery(postgresError('55P03'))).toBeUndefined(); + }); +}); + +describe('findTransactionBreakingMigrations', () => { + it('flags a migration that commits mid-file for CONCURRENTLY', () => { + const pending = [ + { + tag: '0193_concurrent_index', + sql: [ + 'ALTER TABLE "a" DROP CONSTRAINT "b";', + 'COMMIT;', + 'CREATE INDEX CONCURRENTLY "idx" ON "a" ("c");', + 'BEGIN;', + ].join('--> statement-breakpoint'), + }, + ]; + + expect(findTransactionBreakingMigrations(pending)).toEqual(['0193_concurrent_index']); + }); + + it('ignores ordinary DDL', () => { + const pending = [ + { tag: '0204_drop_tables', sql: 'DROP TABLE IF EXISTS "x" CASCADE;' }, + { tag: '0205_add_column', sql: 'ALTER TABLE "y" ADD COLUMN "z" text;' }, + ]; + + expect(findTransactionBreakingMigrations(pending)).toEqual([]); + }); + + it('does not mistake COMMIT inside an identifier or comment for a statement', () => { + const pending = [ + { tag: '0206_commit_column', sql: 'ALTER TABLE "kiloclaw" ADD COLUMN "commit_sha" text;' }, + ]; + + expect(findTransactionBreakingMigrations(pending)).toEqual([]); + }); +}); + +describe('reportFailure', () => { + let errors: string[]; + let spy: jest.SpyInstance; + + beforeEach(() => { + errors = []; + spy = jest.spyOn(console, 'error').mockImplementation((...args: unknown[]) => { + errors.push(args.map(String).join(' ')); + }); + }); + + afterEach(() => spy.mockRestore()); + + it('reports a safety refusal without a stack trace', () => { + reportFailure(new MigrationSafetyError('lock_timeout is disabled')); + + expect(errors).toEqual(['[migrate] refusing to run: lock_timeout is disabled']); + }); + + it('reports the pg fields drizzle-kit discards', () => { + reportFailure( + Object.assign(new Error('Failed query: drop table "x"'), { + query: 'drop table "x"', + cause: Object.assign(new Error('deadlock detected'), { + code: '40P01', + detail: 'Process 1 waits for AccessExclusiveLock', + }), + }) + ); + + expect(errors.join('\n')).toContain('[migrate] migration failed: 40P01'); + expect(errors.join('\n')).toContain('detail: Process 1 waits for AccessExclusiveLock'); + expect(errors.join('\n')).toContain('failing statement: drop table "x"'); + }); +}); + +describe('applyWithRetries', () => { + it('returns the attempt count on first success', async () => { + const apply = jest.fn(async () => {}); + + await expect(applyWithRetries(apply, 5, noSleep)).resolves.toBe(1); + expect(apply).toHaveBeenCalledTimes(1); + }); + + it.each(['40P01', '55P03', '40001'])('retries %s until it succeeds', async code => { + let calls = 0; + const apply = jest.fn(async () => { + calls += 1; + if (calls < 3) throw postgresError(code); + }); + + await expect(applyWithRetries(apply, 5, noSleep)).resolves.toBe(3); + expect(apply).toHaveBeenCalledTimes(3); + }); + + it('retries a lock failure that drizzle wrapped', async () => { + let calls = 0; + const apply = jest.fn(async () => { + calls += 1; + if (calls < 2) throw wrappedPostgresError('55P03', 'drop table "x"'); + }); + + await expect(applyWithRetries(apply, 3, noSleep)).resolves.toBe(2); + }); + + it('gives up after maxAttempts and rethrows the lock failure', async () => { + const apply = jest.fn(async () => { + throw postgresError('55P03'); + }); + + await expect(applyWithRetries(apply, 3, noSleep)).rejects.toMatchObject({ code: '55P03' }); + expect(apply).toHaveBeenCalledTimes(3); + }); + + it('does not retry a real migration bug', async () => { + // 42P01 undefined_table: replaying this would never help. + const apply = jest.fn(async () => { + throw postgresError('42P01', 'relation "gone" does not exist'); + }); + + await expect(applyWithRetries(apply, 5, noSleep)).rejects.toMatchObject({ code: '42P01' }); + expect(apply).toHaveBeenCalledTimes(1); + }); + + it('does not retry an error with no pg code', async () => { + const apply = jest.fn(async () => { + throw new Error('connection terminated'); + }); + + await expect(applyWithRetries(apply, 5, noSleep)).rejects.toThrow('connection terminated'); + expect(apply).toHaveBeenCalledTimes(1); + }); + + it('honours maxAttempts of 1 for migrations that cannot be replayed', async () => { + const apply = jest.fn(async () => { + throw postgresError('40P01'); + }); + + await expect(applyWithRetries(apply, 1, noSleep)).rejects.toMatchObject({ code: '40P01' }); + expect(apply).toHaveBeenCalledTimes(1); + }); +}); diff --git a/packages/db/src/migrate.ts b/packages/db/src/migrate.ts new file mode 100644 index 0000000000..d7ac499690 --- /dev/null +++ b/packages/db/src/migrate.ts @@ -0,0 +1,299 @@ +/** + * Applies pending Drizzle migrations with lock safety. + * + * `drizzle-kit migrate` cannot do this: it builds its own `pg.Pool` and + * zod-strips unknown `dbCredentials` keys, so `lock_timeout` cannot be injected + * through `drizzle.config.ts`. It also discards failures — a deadlocked + * migration prints `applying migrations...undefined` and nothing else. + * + * Two behaviours matter against a database that is serving traffic: + * + * 1. `lock_timeout` is set below the server's `deadlock_timeout`. DDL needing + * `ACCESS EXCLUSIVE` on a busy table would otherwise sit in the lock queue + * until PostgreSQL's deadlock detector fires, and the detector aborts + * whichever party notices the cycle — in practice user requests rather than + * the migration. Timing out first makes the migration the only casualty. + * 2. A lock failure is retried with backoff, so a transient collision costs + * seconds instead of failing the deploy and re-firing on every later merge. + * + * Run it with `pnpm drizzle:migrate-safely` (entry point: `migrate-cli.ts`). + */ +import { existsSync, readFileSync } from 'node:fs'; +import { dirname, resolve } from 'node:path'; +import process from 'node:process'; +import { fileURLToPath } from 'node:url'; + +import { drizzle } from 'drizzle-orm/node-postgres'; +import { migrate } from 'drizzle-orm/node-postgres/migrator'; +import pg from 'pg'; +import * as z from 'zod'; + +import { computeDatabaseUrl, getDatabaseClientConfig } from './database-url'; + +const SCRIPT_DIR = dirname(fileURLToPath(import.meta.url)); +const MIGRATIONS_FOLDER = resolve(SCRIPT_DIR, 'migrations'); +const REPO_ROOT = resolve(SCRIPT_DIR, '../../..'); + +const DEFAULT_LOCK_TIMEOUT = '500ms'; +const DEFAULT_MAX_ATTEMPTS = 10; +const MAX_BACKOFF_MS = 15_000; +const MAX_LOGGED_TAGS = 10; + +/** deadlock_detected, lock_not_available, serialization_failure. */ +export const RETRYABLE_ERROR_CODES = new Set(['40P01', '55P03', '40001']); + +const journalSchema = z.object({ + entries: z.array(z.object({ tag: z.string(), when: z.number() })), +}); + +const ledgerRowSchema = z.object({ max_created_at: z.string().nullable() }); + +const lockSettingsSchema = z.object({ + lock_timeout_ms: z.number(), + deadlock_timeout_ms: z.number(), +}); + +const postgresErrorSchema = z.object({ + code: z.string(), + detail: z.string().optional(), + hint: z.string().optional(), + where: z.string().optional(), + table: z.string().optional(), + constraint: z.string().optional(), + message: z.string().optional(), +}); + +export type PostgresErrorFields = z.infer; +export type PendingMigration = { tag: string; sql: string }; +type SqlRunner = (sql: string) => Promise<{ rows: unknown[] }>; + +/** A refusal to start, rather than a migration failure. Reported without a stack. */ +export class MigrationSafetyError extends Error { + constructor(message: string) { + super(message); + this.name = 'MigrationSafetyError'; + } +} + +function loadEnvFile(path: string): void { + if (!existsSync(path)) return; + process.loadEnvFile(path); +} + +/** Unwraps DrizzleQueryError to reach the underlying pg error fields. */ +export function findPostgresError(error: unknown): PostgresErrorFields | undefined { + let current: unknown = error; + while (current instanceof Error) { + const parsed = postgresErrorSchema.safeParse(current); + if (parsed.success) return parsed.data; + current = current.cause; + } + return undefined; +} + +/** Reads the failing SQL that DrizzleQueryError carries alongside the cause. */ +export function findFailingQuery(error: unknown): string | undefined { + let current: unknown = error; + while (current instanceof Error) { + if ('query' in current && typeof current.query === 'string') return current.query; + current = current.cause; + } + return undefined; +} + +/** + * Migrations that break out of the migrator's transaction with a bare `COMMIT;` + * (the workaround for `CREATE INDEX CONCURRENTLY`) cannot be replayed: a + * failure after that commit leaves earlier statements applied but unrecorded. + */ +export function findTransactionBreakingMigrations(pending: PendingMigration[]): string[] { + return pending + .filter(migration => + migration.sql + .split('--> statement-breakpoint') + .some(statement => /^\s*COMMIT\s*;?\s*$/im.test(statement)) + ) + .map(migration => migration.tag); +} + +export function backoffMs(attempt: number): number { + return Math.min(1_000 * attempt, MAX_BACKOFF_MS) + Math.floor(Math.random() * 500); +} + +export function reportFailure(error: unknown): void { + if (error instanceof MigrationSafetyError) { + console.error(`[migrate] refusing to run: ${error.message}`); + return; + } + + const postgresError = findPostgresError(error); + if (!postgresError) { + console.error('[migrate] migration failed'); + console.error(error); + return; + } + + // drizzle-kit discards every field below, which is why a deadlocked deploy + // only ever printed "applying migrations...undefined". + console.error(`[migrate] migration failed: ${postgresError.code}`); + for (const [label, value] of [ + ['message', postgresError.message], + ['detail', postgresError.detail], + ['hint', postgresError.hint], + ['where', postgresError.where], + ['table', postgresError.table], + ['constraint', postgresError.constraint], + ] as const) { + if (value) console.error(`[migrate] ${label}: ${value}`); + } + + const failingQuery = findFailingQuery(error); + if (failingQuery) console.error(`[migrate] failing statement: ${failingQuery}`); +} + +/** + * Retries only lock-acquisition failures. Everything else is a real migration + * bug and must fail on the first attempt. + */ +export async function applyWithRetries( + apply: () => Promise, + maxAttempts: number, + sleep: (ms: number) => Promise = ms => new Promise(done => setTimeout(done, ms)) +): Promise { + for (let attempt = 1; ; attempt++) { + try { + await apply(); + return attempt; + } catch (error) { + const code = findPostgresError(error)?.code; + const isRetryable = code !== undefined && RETRYABLE_ERROR_CODES.has(code); + + if (!isRetryable) throw error; + if (attempt >= maxAttempts) { + console.error( + `[migrate] gave up after ${attempt} attempt(s); a conflicting lock was held every time` + ); + throw error; + } + + const delay = backoffMs(attempt); + console.warn( + `[migrate] attempt ${attempt}/${maxAttempts} hit ${code}, retrying in ${delay}ms` + ); + await sleep(delay); + } + } +} + +/** + * Fails loudly when the session cannot lose the lock race, because that is the + * property this runner exists to guarantee. + */ +async function assertLockTimeoutIsSafe(run: SqlRunner): Promise { + const result = await run(` + select + (select setting::int from pg_settings where name = 'lock_timeout') as lock_timeout_ms, + (select setting::int from pg_settings where name = 'deadlock_timeout') as deadlock_timeout_ms + `); + const settings = lockSettingsSchema.parse(result.rows[0]); + + if (settings.lock_timeout_ms === 0) { + throw new MigrationSafetyError( + 'lock_timeout is disabled on the migration session, so DDL would wait in the lock queue until the deadlock detector aborts a user query' + ); + } + if (settings.lock_timeout_ms >= settings.deadlock_timeout_ms) { + throw new MigrationSafetyError( + `lock_timeout (${settings.lock_timeout_ms}ms) must be below deadlock_timeout ` + + `(${settings.deadlock_timeout_ms}ms), otherwise the deadlock detector can abort user queries first` + ); + } + + console.log( + `[migrate] lock_timeout=${settings.lock_timeout_ms}ms deadlock_timeout=${settings.deadlock_timeout_ms}ms` + ); +} + +/** + * Mirrors the migrator's own pending check: it compares each journal entry + * against `max(created_at)`, so an entry older than the newest applied + * migration is skipped even when it is absent from the ledger. + */ +async function readPendingMigrations(run: SqlRunner): Promise { + const journalPath = resolve(MIGRATIONS_FOLDER, 'meta/_journal.json'); + const journal = journalSchema.parse(JSON.parse(readFileSync(journalPath, 'utf8'))); + + let appliedThrough = 0; + try { + const result = await run( + 'select max(created_at)::text as max_created_at from drizzle.__drizzle_migrations' + ); + const row = ledgerRowSchema.safeParse(result.rows[0]); + appliedThrough = row.success ? Number(row.data.max_created_at ?? 0) : 0; + } catch { + // No ledger yet: this is a fresh database and everything is pending. + } + + return journal.entries + .filter(entry => entry.when > appliedThrough) + .map(entry => ({ + tag: entry.tag, + sql: readFileSync(resolve(MIGRATIONS_FOLDER, `${entry.tag}.sql`), 'utf8'), + })); +} + +function describeTags(tags: string[]): string { + if (tags.length <= MAX_LOGGED_TAGS) return tags.join(', '); + return `${tags.slice(0, MAX_LOGGED_TAGS).join(', ')} (+${tags.length - MAX_LOGGED_TAGS} more)`; +} + +export async function runMigrations(): Promise { + loadEnvFile(resolve(REPO_ROOT, '.env.local')); + loadEnvFile(resolve(REPO_ROOT, '.env')); + + const lockTimeout = process.env.MIGRATION_LOCK_TIMEOUT || DEFAULT_LOCK_TIMEOUT; + const requestedAttempts = Number(process.env.MIGRATION_MAX_ATTEMPTS); + const configuredMaxAttempts = + Number.isInteger(requestedAttempts) && requestedAttempts > 0 + ? requestedAttempts + : DEFAULT_MAX_ATTEMPTS; + + // A single Client rather than a Pool: drizzle's migrator runs the whole batch + // in one transaction on one connection, so `SET lock_timeout` applies to every + // statement with no pool checkout to race against. + const client = new pg.Client({ + ...getDatabaseClientConfig(computeDatabaseUrl()), + connectionTimeoutMillis: 30_000, + }); + await client.connect(); + + try { + await client.query(`SET lock_timeout = '${lockTimeout}'`); + await assertLockTimeoutIsSafe(sql => client.query(sql)); + + const pending = await readPendingMigrations(sql => client.query(sql)); + if (pending.length === 0) { + console.log('[migrate] no pending migrations'); + return; + } + console.log( + `[migrate] ${pending.length} pending: ${describeTags(pending.map(migration => migration.tag))}` + ); + + const transactionBreaking = findTransactionBreakingMigrations(pending); + if (transactionBreaking.length > 0) { + console.warn( + `[migrate] retries disabled: ${describeTags(transactionBreaking)} commit mid-migration and cannot be replayed safely` + ); + } + + const db = drizzle(client); + const attempts = await applyWithRetries( + () => migrate(db, { migrationsFolder: MIGRATIONS_FOLDER }), + transactionBreaking.length > 0 ? 1 : configuredMaxAttempts + ); + console.log(`[migrate] applied ${pending.length} migration(s) on attempt ${attempts}`); + } finally { + await client.end(); + } +}