fix(db): ride out a brief database outage mid-request; explain it if it persists
The operator's 500 was NOT the startup gap I attributed it to. They found it by logging in and getting Steward's own error page -- which means create_app had completed and the app was serving, so the gaierror came from a request handler acquiring a connection, not from boot. Neither prior fix covers that: the startup retry never runs, and pool_pre_ping only helps once the database is back, since its replacement connect fails too while the container is gone. Adds a before_request gate that acquires a pooled connection with a short bounded retry (~1.75s over 4 tries) so a database restart is ridden out invisibly, and renders a distinct 503 "database unavailable" page when the budget is exhausted. The request budget is deliberately far shorter than the startup one: nobody watches a container boot, but somebody is watching this page load, and a page that hangs is worse than one that says what is wrong. /health and static are exempt -- a liveness probe must stay answerable while the database is down, or a dependency outage triggers a restart loop. Credential and missing-database errors still propagate rather than being reported as "unavailable", which would send the operator chasing the wrong problem entirely. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -1,8 +1,9 @@
|
||||
"""Startup DB readiness retry + connection-pool resilience.
|
||||
"""DB connection resilience: pool, startup readiness, and request-time outage.
|
||||
|
||||
Covers the two recovery gaps that let a database restart take the app down:
|
||||
the engine handing out stale pooled connections, and startup assuming the DB
|
||||
is reachable and ready the instant it asks.
|
||||
Covers the three recovery gaps that let a database restart take the app down:
|
||||
the engine handing out stale pooled connections (#2626), startup assuming the
|
||||
DB is reachable and ready the instant it asks (#2627), and a request finding
|
||||
the database gone and failing with an opaque 500 (#2635).
|
||||
"""
|
||||
import types
|
||||
import pytest
|
||||
@@ -213,3 +214,94 @@ def test_gives_up_after_budget_with_actionable_message(monkeypatch, no_sleep):
|
||||
assert "did not become available" in message
|
||||
# The operator needs the underlying cause, not just "timed out".
|
||||
assert "Name or service not known" in message
|
||||
|
||||
|
||||
# ── ensure_database_reachable (issue #2635) ──────────────────────────────────
|
||||
|
||||
|
||||
class _FakeEngine:
|
||||
"""Minimal stand-in for AsyncEngine.connect() as an async context manager."""
|
||||
|
||||
def __init__(self, *errors, then_succeed: bool = True):
|
||||
self.errors = list(errors)
|
||||
self.then_succeed = then_succeed
|
||||
self.calls = 0
|
||||
|
||||
def connect(self):
|
||||
engine = self
|
||||
|
||||
class _Ctx:
|
||||
async def __aenter__(self):
|
||||
index = engine.calls
|
||||
engine.calls += 1
|
||||
if index < len(engine.errors):
|
||||
raise engine.errors[index]
|
||||
if not engine.then_succeed:
|
||||
raise engine.errors[-1]
|
||||
return object()
|
||||
|
||||
async def __aexit__(self, *exc_info):
|
||||
return False
|
||||
|
||||
return _Ctx()
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def no_async_sleep(monkeypatch):
|
||||
"""Record awaited retry delays without spending them."""
|
||||
slept: list[float] = []
|
||||
|
||||
async def fake_sleep(seconds: float) -> None:
|
||||
slept.append(seconds)
|
||||
|
||||
monkeypatch.setattr(database.asyncio, "sleep", fake_sleep)
|
||||
return slept
|
||||
|
||||
|
||||
async def test_reachable_on_first_try_costs_no_retries(no_async_sleep):
|
||||
engine = _FakeEngine()
|
||||
await database.ensure_database_reachable(engine)
|
||||
assert engine.calls == 1
|
||||
assert no_async_sleep == []
|
||||
|
||||
|
||||
async def test_rides_out_a_brief_outage(no_async_sleep):
|
||||
# The reported shape: DNS gone while the db container restarts.
|
||||
engine = _FakeEngine(OSError(-2, "Name or service not known"))
|
||||
await database.ensure_database_reachable(engine)
|
||||
assert engine.calls == 2
|
||||
assert no_async_sleep == [0.25]
|
||||
|
||||
|
||||
async def test_uses_the_full_retry_schedule_before_giving_up(no_async_sleep):
|
||||
engine = _FakeEngine(OSError("refused"), then_succeed=False)
|
||||
with pytest.raises(database.DatabaseUnavailable):
|
||||
await database.ensure_database_reachable(engine)
|
||||
|
||||
# One attempt per delay, plus a final attempt after the last wait.
|
||||
assert engine.calls == len(database._REQUEST_RETRY_DELAYS) + 1
|
||||
assert no_async_sleep == list(database._REQUEST_RETRY_DELAYS)
|
||||
|
||||
|
||||
async def test_request_budget_stays_short(no_async_sleep):
|
||||
# A person is waiting on this; guard against the schedule growing into a hang.
|
||||
assert sum(database._REQUEST_RETRY_DELAYS) <= 3.0
|
||||
|
||||
|
||||
async def test_unavailable_error_carries_the_underlying_cause(no_async_sleep):
|
||||
engine = _FakeEngine(
|
||||
OSError("Name or service not known"), then_succeed=False)
|
||||
with pytest.raises(database.DatabaseUnavailable) as excinfo:
|
||||
await database.ensure_database_reachable(engine)
|
||||
assert "Name or service not known" in str(excinfo.value)
|
||||
|
||||
|
||||
async def test_bad_credentials_are_not_masked_as_unavailable(no_async_sleep):
|
||||
# Retrying a wrong password would be pointless, and reporting it as
|
||||
# "database unavailable" would send the operator chasing the wrong problem.
|
||||
engine = _FakeEngine(
|
||||
_PGError("password authentication failed", "28P01"), then_succeed=False)
|
||||
with pytest.raises(_PGError):
|
||||
await database.ensure_database_reachable(engine)
|
||||
assert engine.calls == 1
|
||||
assert no_async_sleep == []
|
||||
|
||||
Reference in New Issue
Block a user