From 78bc7183bd6de60b9eb6b11f6e50878443d31455 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Sun, 25 Oct 2026 08:04:27 +0100 Subject: [PATCH] fix(migrations): leave an index another instance is still building alone A concurrent build that is still running is INVALID with its migration not yet finished, exactly like a failed one. With the web and worker roles in two containers, the second boot dropped the index the first was building, which cancelled the first one's migration and left it failed until someone resolved it by hand. The check and the drop now run in one transaction that first takes Prisma's migration lock without waiting and does nothing when another instance holds it; an index Postgres reports as being built is never dropped. One transaction also keeps lock_timeout on the statement it bounds behind a transaction pooler, and the applied-migrations lookup quotes the schema, so a mixed-case schema no longer reads as having none applied. --- docs/ops/deploy.md | 6 ++ scripts/migrate-deploy.mjs | 63 +++++++++++--- .../migrate-deploy-invalid-index.test.ts | 82 +++++++++++++++++++ 3 files changed, 139 insertions(+), 12 deletions(-) diff --git a/docs/ops/deploy.md b/docs/ops/deploy.md index ce8c89dbc..032c55730 100644 --- a/docs/ops/deploy.md +++ b/docs/ops/deploy.md @@ -119,6 +119,12 @@ boot drops that INVALID index before it migrates, logs `dropped the invalid index …`, and the re-run builds it again. Without that step the re-run would find the index by name and skip it. +A build that is still running looks the same, INVALID with its migration not +yet finished. The boot therefore leaves every index alone while another +instance is migrating (it checks Prisma's migration lock first) and never +drops an index Postgres reports as being built, so a second container that +starts during the first one's migration cannot cancel it. + An INVALID index whose migration is already recorded applied is not touched at boot. The boot names it on stderr with `WARNING — index … is INVALID`; rebuild it with `REINDEX INDEX CONCURRENTLY "";` when the database is quiet. diff --git a/scripts/migrate-deploy.mjs b/scripts/migrate-deploy.mjs index d7467f8d6..800547a63 100644 --- a/scripts/migrate-deploy.mjs +++ b/scripts/migrate-deploy.mjs @@ -197,12 +197,32 @@ function concurrentIndexMap(dir) { return builtBy; } +/** + * Prisma's migration lock: `migrate deploy` holds this advisory lock (in the + * schema engine, `pg_advisory_lock(72707369)`) for as long as it migrates. + */ +export const PRISMA_MIGRATE_LOCK = 72707369; + /** * Drop the INVALID indexes a migration still to run will build, and name the * others. Best effort: a database that cannot be read here is left to * `migrate deploy`, which reports its own errors. + * + * A concurrent build that is still running looks exactly like a failed one, + * INVALID with its migration not yet finished. With the web and worker roles + * in two containers, both run this script, and dropping the index the other + * one is building cancels its migration and leaves it failed. So the check + * and the drop run in one transaction that first takes Prisma's migration + * lock without waiting: when another instance is migrating, nothing here is + * touched, and while this transaction holds it, no migration starts. An + * index Postgres reports as being built is never dropped either. One + * transaction also keeps `lock_timeout` on the statement it bounds behind a + * transaction pooler. */ -async function clearInvalidConcurrentIndexes(connectionString, migrationsDir) { +export async function clearInvalidConcurrentIndexes( + connectionString, + migrationsDir, +) { if (!connectionString) return; const builtBy = concurrentIndexMap(migrationsDir); let client; @@ -216,18 +236,36 @@ async function clearInvalidConcurrentIndexes(connectionString, migrationsDir) { connectionTimeoutMillis: 5_000, }); await client.connect(); + const schemaSql = client.escapeIdentifier(schema); + await client.query("BEGIN"); + const locked = await client.query( + "SELECT pg_try_advisory_xact_lock($1) AS ok", + [PRISMA_MIGRATE_LOCK], + ); + if (!locked.rows[0]?.ok) { + await client.query("ROLLBACK"); + return; + } + // Dropping an index takes its table's lock for a moment; never wait on + // it for long at boot. + await client.query("SET LOCAL lock_timeout = '5s'"); const invalid = await client.query( `SELECT c.relname AS name FROM pg_index i JOIN pg_class c ON c.oid = i.indexrelid JOIN pg_namespace n ON n.oid = c.relnamespace - WHERE NOT i.indisvalid AND n.nspname = $1`, + WHERE NOT i.indisvalid AND n.nspname = $1 + AND NOT EXISTS ( + SELECT 1 FROM pg_stat_progress_create_index p + WHERE p.index_relid = i.indexrelid + )`, [schema], ); - if (invalid.rows.length === 0) return; - const table = `${client.escapeIdentifier(schema)}._prisma_migrations`; - const exists = await client.query("SELECT to_regclass($1) AS t", [ - `${schema}._prisma_migrations`, - ]); + if (invalid.rows.length === 0) { + await client.query("ROLLBACK"); + return; + } + const table = `${schemaSql}._prisma_migrations`; + const exists = await client.query("SELECT to_regclass($1) AS t", [table]); const applied = new Set( exists.rows[0]?.t ? ( @@ -243,13 +281,13 @@ async function clearInvalidConcurrentIndexes(connectionString, migrationsDir) { builtBy, applied, }); - // Dropping an index takes its table's lock for a moment; never wait on - // it for long at boot. - await client.query("SET lock_timeout = '5s'"); - for (const { index, migration } of plan.drop) { + for (const { index } of plan.drop) { await client.query( - `DROP INDEX IF EXISTS ${client.escapeIdentifier(schema)}.${client.escapeIdentifier(index)}`, + `DROP INDEX IF EXISTS ${schemaSql}.${client.escapeIdentifier(index)}`, ); + } + await client.query("COMMIT"); + for (const { index, migration } of plan.drop) { console.log( `HealthLog: dropped the invalid index ${index} a failed concurrent ` + `build left behind; ${migration} builds it again.`, @@ -264,6 +302,7 @@ async function clearInvalidConcurrentIndexes(connectionString, migrationsDir) { ); } } catch (error) { + await client?.query("ROLLBACK").catch(() => {}); console.error( `HealthLog: could not check for invalid indexes before migrating: ${ error instanceof Error ? error.message : String(error) diff --git a/tests/integration/migrate-deploy-invalid-index.test.ts b/tests/integration/migrate-deploy-invalid-index.test.ts index 25956de43..ee672661a 100644 --- a/tests/integration/migrate-deploy-invalid-index.test.ts +++ b/tests/integration/migrate-deploy-invalid-index.test.ts @@ -16,6 +16,11 @@ import { join } from "node:path"; import { Client } from "pg"; import { afterAll, beforeEach, describe, expect, it } from "vitest"; +import { + clearInvalidConcurrentIndexes, + PRISMA_MIGRATE_LOCK, +} from "../../scripts/migrate-deploy.mjs"; + const SCHEMA = "invalid_index_probe"; const NAME = "0002_probe_value_index"; const INDEX = "probe_value_key"; @@ -171,3 +176,80 @@ describe("a concurrent index build that failed", () => { } }); }); + +/** + * A build still running is INVALID too, and with the web and worker roles in + * two containers the second one's boot runs the cleanup while the first one + * migrates. Dropping that index cancels the first one's migration and leaves + * it failed, so the cleanup must leave a running build alone. + */ +describe("a concurrent index build still running", () => { + async function slowBuildFixture(db: Client): Promise { + await db.query(`CREATE SCHEMA ${SCHEMA}`); + await db.query( + `CREATE TABLE ${SCHEMA}.probe (id integer PRIMARY KEY, value integer)`, + ); + await db.query( + `INSERT INTO ${SCHEMA}.probe SELECT g, g FROM generate_series(1, 400) g`, + ); + // Slow on purpose, so the build is caught half way. + await db.query( + `CREATE FUNCTION ${SCHEMA}.probe_slow(v integer) RETURNS integer + LANGUAGE plpgsql IMMUTABLE AS $$ + BEGIN PERFORM pg_sleep(0.005); RETURN v; END $$`, + ); + const dir = mkdtempSync(join(tmpdir(), "hl-running-index-")); + dirs.push(dir); + currentDir = join(dir, "migrations"); + write( + dir, + NAME, + `CREATE INDEX CONCURRENTLY IF NOT EXISTS "${INDEX}" ON "probe" (probe_slow("value"));`, + ); + } + + async function caughtHalfWay( + holdMigrateLock: boolean, + ): Promise<{ builder: Client; build: Promise }> { + const builder = await connect(); + if (holdMigrateLock) { + await builder.query("SELECT pg_advisory_lock($1)", [PRISMA_MIGRATE_LOCK]); + } + const build = builder.query( + `CREATE INDEX CONCURRENTLY "${INDEX}" ON ${SCHEMA}.probe (${SCHEMA}.probe_slow(value))`, + ); + const db = await connect(); + try { + const until = Date.now() + 10_000; + while ((await indexState(db)) !== false && Date.now() < until) { + await new Promise((r) => setTimeout(r, 50)); + } + expect(await indexState(db)).toBe(false); + } finally { + await db.end(); + } + return { builder, build }; + } + + for (const [label, holdMigrateLock] of [ + ["another instance migrating", true], + ["a build outside any migration", false], + ] as const) { + it(`leaves the index of ${label} alone`, async () => { + const db = await connect(); + try { + await slowBuildFixture(db); + const { builder, build } = await caughtHalfWay(holdMigrateLock); + try { + await clearInvalidConcurrentIndexes(probeUrl(), currentDir); + await expect(build).resolves.toBeDefined(); + expect(await indexState(db)).toBe(true); + } finally { + await builder.end(); + } + } finally { + await db.end(); + } + }, 60_000); + } +});