fix(alembic): lock_timeout on migrations, drop the advisory lock
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) <noreply@anthropic.com>
This commit is contained in:
+23
-20
@@ -1,5 +1,7 @@
|
|||||||
"""Alembic environment — reads DATABASE_URL from app config."""
|
"""Alembic environment — reads DATABASE_URL from app config."""
|
||||||
|
|
||||||
|
import os
|
||||||
|
import re
|
||||||
from logging.config import fileConfig
|
from logging.config import fileConfig
|
||||||
|
|
||||||
from sqlalchemy import engine_from_config, pool, text
|
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.config import get_config
|
||||||
from backend.app.models import Base
|
from backend.app.models import Base
|
||||||
|
|
||||||
# Arbitrary fixed 64-bit key for the session/transaction advisory lock that
|
# Fail a blocked migration FAST instead of hanging forever. Migrations run
|
||||||
# serializes concurrent `alembic upgrade head` runs. Every `web` replica runs
|
# against the live DB while workers hold locks; 0040's `ALTER series_page` queued
|
||||||
# migrations in its entrypoint, so under `docker stack deploy` two replicas can
|
# behind a tag-merge that held a series_page lock for minutes (the merge runs an
|
||||||
# boot at once and race the same DDL — duplicate CREATE TABLE, then a crashed
|
# unindexed full scan over image_record while repointing series_page) and hung
|
||||||
# replica (operator-flagged 2026-06-07: 0040 raced; one backend died with
|
# with no timeout — silent, indefinite (operator-flagged 2026-06-07). With a
|
||||||
# AdminShutdown). The first replica to reach the lock migrates; the rest block,
|
# lock_timeout a blocked DDL errors ("canceling statement due to lock timeout")
|
||||||
# then find the version table already at head and apply nothing.
|
# and the entrypoint's `alembic upgrade head` exits non-zero, so the deploy
|
||||||
_MIGRATION_LOCK_KEY = 0xFCA1E35C
|
# 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
|
config = context.config
|
||||||
|
|
||||||
@@ -47,24 +53,21 @@ def run_migrations_online() -> None:
|
|||||||
poolclass=pool.NullPool,
|
poolclass=pool.NullPool,
|
||||||
)
|
)
|
||||||
with connectable.connect() as connection:
|
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(
|
context.configure(
|
||||||
connection=connection,
|
connection=connection,
|
||||||
target_metadata=target_metadata,
|
target_metadata=target_metadata,
|
||||||
compare_type=True,
|
compare_type=True,
|
||||||
)
|
)
|
||||||
with context.begin_transaction():
|
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()
|
context.run_migrations()
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user