Skip to content

fix(aster-srv): serve /healthz while migrations wait for table locks - #164

Merged
calvin-archastro merged 4 commits into
mainfrom
fix/aster-srv-serve-while-migrating
Oct 18, 2026
Merged

calvin-archastro merged 4 commits into
mainfrom
fix/aster-srv-serve-while-migrating

Conversation

@calvin-archastro

@calvin-archastro calvin-archastro commented Oct 18, 2026 •

Copy link
Copy Markdown
Contributor

Review on ArchCode

Problem

Production incident 2026-10-17 (firstlanding auto_apply_production run 39067438593, rolling aster-srv 0.20.0 to 0.21.0):

new pod starts -> App::start -> db::migrate -> migration 5: ALTER TABLE
  test_results.classifier_state / quarantine   (needs ACCESS EXCLUSIVE)
  blocked by 0.20.0 pods still running their classifier pass (open transaction on those tables)
  5s lock_timeout x 11 attempts -> "canceling statement due to lock timeout"
no listener bound meanwhile -> liveness /healthz fails (10s + 3x20s) -> SIGKILL (137)
after 12 attempts the process exits 1
kubelet restarts it (8 restarts); each restart repeats the wait
Pulumi's 600s Deployment wait times out and the apply fails

It converged once the old pods' passes stopped holding the tables. Invariant broken: the process listened only after migrations finished, so any lock wait longer than the liveness window killed it, and the retry cap turned a transient wait into an exit.

What changed

Before: App::start migrates, then main binds. After: App::connect builds the app with no DB schema work, main binds immediately, and migrations run in a background task that retries every 5s without exiting, then starts the retention, classifier and webhook loops.

  • /healthz is 200 throughout, so the liveness probe does not kill the pod.
  • /readyz and every other route return 503 until App::migrate succeeds (App::schema_ready), so the pod receives no traffic against an old schema.
  • App::start* still migrates before returning, so tests and other callers are unchanged.
  • The retry loop is App::migrate_until_ready in lib.rs (main only wires logging and the loops to start). Table-lock waits and deadlocks (db::is_lock_contention) log a warning until they have lasted 10 minutes, then error. Any other migration error logs at error immediately. It never exits.
  • Pacing: 5s after the first failed pass, 30s after later ones. Each pass is already up to 12 attempts of a 5s lock_timeout, and a waiting ALTER queues other sessions behind it, so the longer pause cuts how often the previous release's readers stall.
  • Unchanged: migration SQL, per-attempt lock_timeout, the advisory lock.

Scope / risk / impact

  • Backend only, crates/aster-srv. Low risk. One behavior change: a persistent migration error (for example a bad SQL migration) now keeps the pod running but not ready and logs migrations not applied yet; retrying, instead of exiting.
  • No user impact. Rolling deploys no longer crash-loop on lock waits.

Testing

  • Canonical end-to-end proof: crates/aster-srv/tests/startup.rs, the_server_stays_alive_while_a_migration_waits_for_a_table_lock. It uses real Postgres 16 and real HTTP. A second connection holds ACCESS SHARE on test_results.quarantine (the old release) while migration 5 runs. It asserts /healthz 200, /readyz 503 and a data route 503, then releases the lock and asserts /readyz 200.
  • startup.rs also has: background_loops_start_only_after_the_schema_is_ready (lock held: warnings only, on_ready not called, then called after release) and a_migration_error_that_is_not_a_lock_wait_logs_an_error_and_stays_unready (no DB listening: error severity on each pass, keeps retrying, unready, loops never start; needs no Postgres).
  • cargo test --locked -p aster-srv with ASTER_SRV_TEST_DATABASE_URL set: all pass. cargo clippy -p aster-srv --all-targets -D warnings and cargo fmt are clean.

Follow-ups

  • The lock holder is the classifier pass, one long transaction touching classifier_state/quarantine. 0.21.0 already shortens it (4977ffb); this PR does not change it.
  • firstlanding infra/services/aster-srv: a startupProbe is optional after this change.

🤖 Generated with Claude Code

Production incident 2026-10-17: release 0.21.0's migration 5 (ALTER TABLE on
classifier_state and quarantine) waited behind the previous release's
classifier pass. It gave up after 12 attempts and exited, and before that the
liveness probe killed the pod (exit 137) because nothing listened during
App::start. The rollout restarted the wait 8 times and the apply timed out.

Listen first, migrate in the background, report 503 on /readyz and every
other route until the schema is applied, and retry migrations without exiting.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@calvin-archastro
calvin-archastro requested a review from a team October 18, 2026 17:21
@calvin-archastro
calvin-archastro marked this pull request as draft October 18, 2026 17:23
Extract the retry-then-start-loops sequence into App::migrate_until_ready.
Table-lock waits warn (error after 10 minutes); any other migration error
logs at error immediately. Pause 5s after the first pass, 30s after later
ones, so the lock queue stalls the previous release's readers less.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@calvin-archastro
calvin-archastro marked this pull request as ready for review October 18, 2026 17:35
@calvin-archastro

calvin-archastro commented Oct 18, 2026 •

Copy link
Copy Markdown
Contributor Author

ArchDev is watching this pull request. State: settled (cycle 2 of 2 for head 243ec42).

when trigger decision result
2026-10-18T17:41:24.242Z review feedback enqueue job_232e5be9be54723d547deb75 published 6f02036
2026-10-18T18:06:02.477Z review feedback enqueue job_23834ec66fee0557eac14b36 published 243ec42

Paused by the no-autofix label or a draft; never resolves reviewer threads. Jobs: job_232e5be9be54723d547deb75, job_23834ec66fee0557eac14b36.

Retries never stop, so a permanent migration failure leaves the pod live
and unready forever. /readyz now says whether it is waiting_for_locks or
failing, and the runbook covers a deploy that never becomes ready.
@calvin-archastro
calvin-archastro merged commit cc96e2d into main Oct 18, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant