Skip to content

fix(server): hold migration advisory lock across the whole migration (PERF-027) - #1998

Draft
serenakeyitan wants to merge 1 commit into
mainfrom
fix/perf-027-migration-advisory-lock
Draft

fix(server): hold migration advisory lock across the whole migration (PERF-027)#1998
serenakeyitan wants to merge 1 commit into
mainfrom
fix/perf-027-migration-advisory-lock

Conversation

@serenakeyitan

Copy link
Copy Markdown
Contributor

Closes #1690 — [PERF-027][High] Startup migrations are not serialized across replicas.

Problem

runMigrations (packages/server/src/db/migrate.ts) only probed the advisory lock: it acquired pg_try_advisory_lock(hashtext('drizzle_migrations')) and released it immediately, then ran drizzle migrate() on a separate, unlocked connection. drizzle-orm@0.44.7 takes no advisory lock of its own, so during a multi-replica rollout every replica could pass the preflight simultaneously and execute DDL/backfills in parallel — duplicate-object errors, duplicated backfill work, or failed boot replicas.

Approach

Hold one session-level advisory lock on a single dedicated connection for the entire migration, per the issue's recommended direction:

  • runMigrations opens one max: 1 postgres.js client (one physical session), polls pg_try_advisory_lock (unchanged 1s interval / 15s default timeout), and on success keeps the lock held.
  • drizzle migrate() runs on that same connection; session-level advisory locks survive the transactions drizzle opens, so journal read + DDL/backfills + journal insert are all covered.
  • The post-migration table count reuses the same connection (3 connections → 1).
  • client.end() in finally closes the session, which releases the lock server-side on both success and error paths — a failed migration cannot strand the lock, and no explicit unlock can mask an in-flight error on a dead connection.
  • Non-owner replicas wait by polling; when the owner releases, the next replica acquires the lock, sees the advanced journal, and no-ops. On timeout the existing migration lock contention error still fails the boot fast for the orchestrator to retry.

Lock key, timeout defaults, error message, and the 20s runMigrations stage budget in bootstrap-server.ts are all unchanged, so existing monitoring/behavior contracts hold. bootstrap-server.ts gets a comment update only.

Verification

  • pnpm check, pnpm typecheck, pnpm build: pass (the one pre-existing biome error in packages/client/src/__tests__/assistant-text.test.ts is on main and untouched by this PR).
  • pnpm test: full suite green locally (server: 245 files / 2672 tests).
  • bootstrap-migration-lock.test.ts extended from 2 to 4 cases:
    1. contention (pre-existing): external holder → clear migration lock contention error after timeout;
    2. waiting (new): holder releases after 1.5s → a waiter with 10s timeout completes normally (journal no-op), pinning the wait-then-proceed rollout semantics;
    3. full-span locking (new): with drizzle's migrator mocked to probe from a second session, the advisory lock must be unavailable while migrate() executes — this test fails against the old preflight-then-unlock code — and grabbable again after runMigrations returns;
    4. free lock (pre-existing): no holder → migration succeeds.

QA note

This touches the boot path. Deterministic serialization behavior is covered by the product tests above; for release-candidate validation the existing case packages/qa/cases/cross-surface/release-boot-health.md (production image boots + self-runs migrations + health surfaces) already covers this surface, so formal QA can reuse it if requested.

The old preflight acquired pg_try_advisory_lock(hashtext('drizzle_migrations'))
and released it immediately, then ran drizzle migrate() on a separate,
unlocked connection. Under multi-replica rollout every replica could pass
preflight simultaneously and execute DDL/backfills in parallel, causing
duplicate-object errors, duplicated backfill work, or failed boot replicas.

runMigrations now opens one max:1 connection, acquires the session-level
advisory lock on it (same key, same 15s poll/timeout, same contention error
message), and keeps it held across the drizzle migrate() call and the table
count; closing the session in finally releases the lock on both success and
error paths. Non-owner replicas keep polling; once the owner releases, they
acquire the lock, see the advanced journal, and no-op.

Closes #1690
@serenakeyitan serenakeyitan added fire_wip GoF: draft PR in progress fire_submitted GoF: PR submitted for maintainer review (stays draft) and removed fire_wip GoF: draft PR in progress labels Jul 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fire_submitted GoF: PR submitted for maintainer review (stays draft)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[PERF-027][High] Startup migrations are not serialized across replicas

1 participant