fix(server): stop leaking sqlx migration locks through the Neon pooler - #713
Conversation
WALM-378: sqlx takes a session-scoped pg_advisory_lock for migrations. Through Neon's transaction-mode pooler that lock (and Apalis's set_config session GUCs leak onto reused backends. The next boot waits 15s, times out, and panics — taking the relayer down until a lucky redeploy. Run sqlx/Apalis migrations on the direct compute endpoint, retry lock contention instead of .expect(), and stop setting session GUCs on the pooled Apalis connections. EOF )
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
A leftover pooled session can still block the first boot after this fix: advisory locks are database-wide, so the direct migrator waits on the same key. On lock timeout, log the holder and pg_terminate_backend only if that backend is idle (the WALM-378 shape). Also include the direct host in migrate connect errors.
lock_timeout aborts the in-flight migrate statement. If sqlx had a transaction open, later pg_locks / pg_terminate_backend queries fail with "current transaction is aborted" and the idle holder is never killed. ROLLBACK first, and treat idle-in-transaction like idle for this lock key only.
Keep the explanation; do not stamp Linear identifiers into runtime strings or source comments.
nikola0x0
left a comment
nikola0x0
left a comment
There was a problem hiding this comment.
Pre-approve. No blocking correctness bugs. I built the branch and ran the new tests locally (cargo test --lib storage::, cargo check --bins) — everything compiles and the 4 new/updated test modules pass. I also independently re-derived sqlx's advisory-lock ID scheme (generate_lock_id in sqlx-postgres) and the pg_locks bit-reconstruction: classid/objid are oid (unsigned), so the (classid::bigint << 32) | objid::bigint reconstruction in release_orphaned_sqlx_migration_lock is correct without a mask — I initially suspected a sign-extension bug there and disproved it by checking the Postgres column types and SET_LOCKTAG_INT64 in the Postgres source. The direct-endpoint rewrite + retry/backoff design correctly addresses the actual leak mechanism (transaction-mode pooler returning a backend to the pool with a session-scoped lock still held).
Majors (details inline)
init_apalis_pool(main.rs:483) — removingafter_connectentirely also dropsstatement_timeout/idle_in_transaction_session_timeoutprotection from Apalis's runtime connections (job processing for the life of the process), not just the migration step that actually caused the incident. Worth confirming that's intentional, sinceset_config(name, value, true)(transaction-local) would close the same pooler-leak vector without giving that up. See inline comment.
Notes for whoever picks up related work
- The retry +
pg_terminate_backendself-healing path (apply_legacy_migrations,release_orphaned_sqlx_migration_lock) has no integration coverage — only the pure helpers (CRC/lock-id, error classification, URL rewrite) are unit-tested. Reasonable given how hard that is to test against a real pooler, just flagging the gap. - The pre-existing
.expect("Failed to initialize legacy security-delete database")at main.rs:782 (outside this diff) still panics if all 6 retry attempts are exhausted (worst case ≈ 45s: 6× up to 5s lock_timeout + ~15.5s of backoff). That's fine for the incident as described (a leaked pooler backend clears well within that), but if Railway's healthcheck/deploy timeout is tighter than that window, a genuine prolonged lock (e.g., two pods migrating concurrently) would still reproduce the original 502 symptom, just far less often. Worth a sentence in the PR description if that's already been checked against Railway's config.
Minor (2) — details inline: unbounded setup_pool.close(), and string-matching vs. structured SQLSTATE for retry classification.
… SQLSTATE SET LOCAL in after_connect is a no-op under autocommit, so restore statement_timeout / idle_in_transaction_session_timeout as session GUCs and omit lock_timeout (the leak that aborted sqlx migrate). Bound setup_pool.close() with the same startup timeout. Classify lock contention on Postgres SQLSTATE (55P03 / 40P01) before mapping into AppError, so a Display reword cannot disable retries.
2 participants
Fixes WALM-378.
What happened
Dev relayer was hard down 07:30–07:44 UTC. It booted, applied main DB migrations, then panicked:
Railway reported SUCCESS/RUNNING (no healthcheck). The process never bound :3001, so every request was 502.
Why it looks random, and why staging/prod stayed up
sqlx migrations take a session-scoped
pg_advisory_lock.LEGACY_DB_URLandDATABASE_URLboth go through Neon’s transaction-mode pooler.LOCKandUNLOCKcan land on different backends; a client disconnect returns the backend to the pool with the lock still held. Apalis then reused that backend for job-queue traffic, so the lock never cleared.On the next boot the migrator waits on that lock. A leaked
set_config('lock_timeout', '15s', false)frominit_apalis_pool(session GUC, also pooler-leaked) cancels the wait after 15s, and.expect()takes the process down.This only fires on restart, and only if a previous boot left the lock on a still-live pooled backend:
Not caused by #711 (7 TypeScript files, not even merged) or #706.
Fix
*-pooler.*→*.). Runtime query pools keep the pooled URL.lock_timeout, 6 attempts, backoff) instead of a single.expect().set_config(..., false)) on the pooled Apalis connections. Boot is still bounded by tokio timeouts. Apalis schema setup, when needed, also uses the direct endpoint.No Railway env change required — the pooler hostname is rewritten in code.
Tests