From 8e98e7996828ae95f7a30df9fc779f4946b637fc Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Sun, 7 Jun 2026 23:58:14 -0400 Subject: [PATCH] fix(alembic): lock_timeout on migrations, drop the advisory lock MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reverses the advisory-lock approach (7309d1d) — it treated a replica race that wasn't the cause and added a new indefinite-hang mode (a sibling/stale migrator holding the xact lock). Real cause of the 0040 hang (operator-diagnosed 2026-06-07): web has always been a single replica. The migration's ALTER series_page queued behind a concurrent tag-merge that held a series_page lock for minutes — _do_merge repoints series_page then runs _create_protective_aliases, an unindexed full scan of image_record (JSON column, ~59k rows). Migrations ran with no lock_timeout, so the DDL hung indefinitely and silently. Fix: SET lock_timeout (default 30s, env-overridable) on the migration connection before alembic's transaction. A blocked DDL now fails fast with 'canceling statement due to lock timeout'; the entrypoint exits non-zero so the deploy retries / surfaces loudly instead of wedging. General protection for every future migration. (The slow _create_protective_aliases scan — the actual lock holder — is the separate perf fix still under discussion.) Co-Authored-By: Claude Opus 4.8 (1M context) --- alembic/env.py | 43 +++++++++++++++++++++++-------------------- 1 file changed, 23 insertions(+), 20 deletions(-) diff --git a/alembic/env.py b/alembic/env.py index b657465..f9d8164 100644 --- a/alembic/env.py +++ b/alembic/env.py @@ -1,5 +1,7 @@ """Alembic environment — reads DATABASE_URL from app config.""" +import os +import re from logging.config import fileConfig from sqlalchemy import engine_from_config, pool, text @@ -8,14 +10,18 @@ from alembic import context from backend.app.config import get_config from backend.app.models import Base -# Arbitrary fixed 64-bit key for the session/transaction advisory lock that -# serializes concurrent `alembic upgrade head` runs. Every `web` replica runs -# migrations in its entrypoint, so under `docker stack deploy` two replicas can -# boot at once and race the same DDL — duplicate CREATE TABLE, then a crashed -# replica (operator-flagged 2026-06-07: 0040 raced; one backend died with -# AdminShutdown). The first replica to reach the lock migrates; the rest block, -# then find the version table already at head and apply nothing. -_MIGRATION_LOCK_KEY = 0xFCA1E35C +# Fail a blocked migration FAST instead of hanging forever. Migrations run +# against the live DB while workers hold locks; 0040's `ALTER series_page` queued +# behind a tag-merge that held a series_page lock for minutes (the merge runs an +# unindexed full scan over image_record while repointing series_page) and hung +# with no timeout — silent, indefinite (operator-flagged 2026-06-07). With a +# lock_timeout a blocked DDL errors ("canceling statement due to lock timeout") +# and the entrypoint's `alembic upgrade head` exits non-zero, so the deploy +# retries / surfaces loudly rather than wedging. Override via env when a known +# slow-lock window is expected. +_MIGRATION_LOCK_TIMEOUT = os.environ.get("MIGRATION_LOCK_TIMEOUT", "30s") +if not re.fullmatch(r"\d+\s*(ms|s|min)?", _MIGRATION_LOCK_TIMEOUT.strip()): + _MIGRATION_LOCK_TIMEOUT = "30s" # ignore a malformed override config = context.config @@ -47,24 +53,21 @@ def run_migrations_online() -> None: poolclass=pool.NullPool, ) with connectable.connect() as connection: + # Session-level lock_timeout for every DDL statement in this run. Set + # (and commit) before alembic opens its own transaction so the GUC + # persists on this connection regardless of how alembic structures its + # transactions. Value is from our own env, so f-string interpolation is + # safe (and it's been pattern-validated above); SET takes no bind params. + connection.execute( + text(f"SET lock_timeout = '{_MIGRATION_LOCK_TIMEOUT}'") + ) + connection.commit() context.configure( connection=connection, target_metadata=target_metadata, compare_type=True, ) with context.begin_transaction(): - # Serialize concurrent migrators (see _MIGRATION_LOCK_KEY). A - # transaction-scoped advisory lock: the first replica to get here - # holds it for the whole upgrade and is auto-released when this - # transaction ends. A sibling replica blocks on this line, and only - # once the leader commits does it proceed to read the version table - # — now at head — so it runs zero migrations instead of re-applying - # the same DDL. The lock is acquired BEFORE run_migrations() reads - # the current revision, which is what makes the no-op correct. - connection.execute( - text("SELECT pg_advisory_xact_lock(:k)"), - {"k": _MIGRATION_LOCK_KEY}, - ) context.run_migrations()