fix(migrations): bound the lock wait of 0475 and 0480, and of every file after

0475 and 0480 added their columns with a bare ALTER TABLE, which waits
for its table lock behind every open transaction while every write to
the table queues behind it. Both now run in a DO block with a five
second lock_timeout and carry the boot's retry marker; the effect is
the same and both stay safe to run again.

Neither has shipped in a release. A database that already applied them
is unaffected: prisma migrate deploy and migrate status ignore the
checksum of an applied migration (checked against a database migrated
with the old files), so nothing is re-run and nothing is reported.

The lock-timeout guard now refuses, from 0475 on, any statement that
takes a table lock outside such a block.
This commit is contained in:
Marc-André Bombeck committed 2026-10-25 02:42:09 +01:00
1 parent 205d8f257c
commit b2c99e5c8d
3 files changed
+91 -3

No files matched your search

@@ -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$;
@@ -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$;
@@ -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([]);
});
});