diff --git a/prisma/migrations/0475_mental_health_assessment_wording/migration.sql b/prisma/migrations/0475_mental_health_assessment_wording/migration.sql index 6591fa1e1..a3a64cc4d 100644 --- a/prisma/migrations/0475_mental_health_assessment_wording/migration.sql +++ b/prisma/migrations/0475_mental_health_assessment_wording/migration.sql @@ -10,5 +10,16 @@ -- -- Reversibility: dropping the column loses only that marker; the scores and -- the encrypted answers do not depend on it. +-- +-- The lock wait is bounded inside a DO block and the file is safe to run +-- again, so the boot retries it on a lock timeout. +-- +-- @retry-on-lock-timeout -ALTER TABLE "mental_health_assessments" ADD COLUMN IF NOT EXISTS "wording" TEXT; +DO $migration$ +BEGIN + PERFORM set_config('lock_timeout', '5s', true); + ALTER TABLE "mental_health_assessments" + ADD COLUMN IF NOT EXISTS "wording" TEXT; +END +$migration$; diff --git a/prisma/migrations/0480_status_note_failure_streak/migration.sql b/prisma/migrations/0480_status_note_failure_streak/migration.sql index ddc0b0fdb..45ce64bdb 100644 --- a/prisma/migrations/0480_status_note_failure_streak/migration.sql +++ b/prisma/migrations/0480_status_note_failure_streak/migration.sql @@ -7,5 +7,16 @@ -- up to two hours. A written or re-dated note resets it to zero. -- -- Existing rows start at zero, which is the five-minute window they had. -ALTER TABLE "insight_status_caches" - ADD COLUMN IF NOT EXISTS "failure_streak" INTEGER NOT NULL DEFAULT 0; +-- +-- The lock wait is bounded inside a DO block and the file is safe to run +-- again, so the boot retries it on a lock timeout. +-- +-- @retry-on-lock-timeout + +DO $migration$ +BEGIN + PERFORM set_config('lock_timeout', '5s', true); + ALTER TABLE "insight_status_caches" + ADD COLUMN IF NOT EXISTS "failure_streak" INTEGER NOT NULL DEFAULT 0; +END +$migration$; diff --git a/src/__tests__/migration-lock-timeout-guard.test.ts b/src/__tests__/migration-lock-timeout-guard.test.ts index 919e7661f..5f276e195 100644 --- a/src/__tests__/migration-lock-timeout-guard.test.ts +++ b/src/__tests__/migration-lock-timeout-guard.test.ts @@ -234,3 +234,69 @@ describe("migration lock waits", () => { ).toEqual([]); }); }); + +/** + * From 0475 on, a statement that takes a table lock runs inside a `DO` block + * that bounds its wait. A bare `ALTER TABLE` waits behind every open + * transaction on the table, and every write to it queues behind the waiting + * `ALTER`, for as long as the longest of them runs: under a backfill or a + * pg_dump that is the boot's start period gone, and the half-applied file is + * left on record. Older files are history; they ran and are not run again. + */ +const LOCK_BOUND_FROM = 475; + +/** Top-level statements that take a table lock and wait for it. */ +const TABLE_LOCK = + /^(ALTER TABLE|CREATE (UNIQUE )?INDEX (?!CONCURRENTLY)|DROP (TABLE|INDEX) (?!CONCURRENTLY)|LOCK )/i; + +function unboundedLockViolations(name: string, sql: string): string[] { + const number = Number.parseInt(name.slice(0, 4), 10); + if (!(number >= LOCK_BOUND_FROM)) return []; + return topLevelStatements(sql) + .filter((statement) => TABLE_LOCK.test(statement)) + .map( + (statement) => + `${name}: "${statement.slice(0, 60)}" waits for its table lock without bound; run it in a DO block with set_config('lock_timeout', …, true)`, + ); +} + +describe("table locks since 0475", () => { + it("are taken inside a block that bounds the wait", () => { + const checked = migrations().filter( + ({ name }) => Number.parseInt(name.slice(0, 4), 10) >= LOCK_BOUND_FROM, + ); + expect(checked.length).toBeGreaterThan(5); + const found = checked.flatMap(({ name, sql }) => + unboundedLockViolations(name, sql), + ); + expect(found, found.join("\n")).toEqual([]); + }); + + it("catches each shape it exists to refuse", () => { + expect( + unboundedLockViolations( + "0999_a", + 'ALTER TABLE "t" ADD COLUMN IF NOT EXISTS "c" INTEGER;', + ), + ).toHaveLength(1); + expect( + unboundedLockViolations("0999_b", 'CREATE INDEX "i" ON "t" ("c");'), + ).toHaveLength(1); + expect( + unboundedLockViolations( + "0999_c", + "DO $m$ BEGIN PERFORM set_config('lock_timeout', '5s', true); ALTER TABLE \"t\" ADD COLUMN \"c\" INTEGER; END $m$;", + ), + ).toEqual([]); + expect( + unboundedLockViolations( + "0999_d", + 'CREATE INDEX CONCURRENTLY IF NOT EXISTS "i" ON "t" ("c");', + ), + ).toEqual([]); + // History is not re-run. + expect( + unboundedLockViolations("0100_e", 'ALTER TABLE "t" ADD COLUMN "c" INT;'), + ).toEqual([]); + }); +});