From cba2c246126e1afd40cea6034d6871c4c75620df Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Wed, 21 Oct 2026 19:15:20 -0400 Subject: [PATCH] feat(544): shipped compose files run on the orchestrator's defaults; the app waits for its database itself Brings the three compose files in line with rule 198 (milestone 544, step 6203, at the operator's request): - no healthcheck on the app or the database, so a slow pg_isready on stalled storage can no longer get a working database killed; - no deploy.restart_policy in the Swarm file, so Swarm's default (condition: any) restarts clean exits too, instead of on-failure with max_attempts leaving the app down; - depends_on is plain start order; restart: unless-stopped stays on the plain-Compose files (Swarm ignores it), and the dev app gains it. The orchestrator used to be what retried a container that started before Postgres was ready. That is now the app's job: alembic/env.py calls services/db_ready.wait_for_database before migrating. It retries refused connections, unresolved hosts, connect timeouts and "the database system is starting up" with 1-2-4-...-10s spacing for up to 300s, then exits non-zero; a wrong password or missing database fails at once. Per-request recovery is the engine's pool_pre_ping. Also fixes the dev compose database image: postgres:16-alpine has no vector extension, so migrations could not run against it. It now uses pgvector/pgvector:pg17 like the other two files. Co-Authored-By: Claude Opus 5.5 --- alembic/env.py | 2 + docker-compose.prod.yml | 24 -------- docker-compose.quickstart.yml | 17 +----- docker-compose.yml | 21 ++----- src/scribe/services/db_ready.py | 77 ++++++++++++++++++++++++ tests/test_db_ready.py | 100 ++++++++++++++++++++++++++++++++ 6 files changed, 184 insertions(+), 57 deletions(-) create mode 100644 src/scribe/services/db_ready.py create mode 100644 tests/test_db_ready.py diff --git a/alembic/env.py b/alembic/env.py index a7af4f88..0ba83fd2 100644 --- a/alembic/env.py +++ b/alembic/env.py @@ -6,6 +6,7 @@ from sqlalchemy.ext.asyncio import create_async_engine from scribe.config import Config from scribe.models import Base +from scribe.services.db_ready import wait_for_database config = context.config if config.config_file_name is not None: @@ -32,6 +33,7 @@ def do_run_migrations(connection): async def run_async_migrations() -> None: + await wait_for_database(Config.DATABASE_URL) engine = create_async_engine(Config.DATABASE_URL) async with engine.connect() as connection: await connection.run_sync(do_run_migrations) diff --git a/docker-compose.prod.yml b/docker-compose.prod.yml index 15bf444b..578c4564 100644 --- a/docker-compose.prod.yml +++ b/docker-compose.prod.yml @@ -9,16 +9,6 @@ services: SECURE_COOKIES: "true" networks: - scribe_backend - healthcheck: - test: ["CMD", "python", "-c", "import urllib.request; urllib.request.urlopen('http://localhost:5000/api/health')"] - interval: 30s - timeout: 10s - retries: 3 - start_period: 30s - deploy: - restart_policy: - condition: on-failure - max_attempts: 5 db: # Postgres 17 with the vector extension Scribe needs. @@ -32,20 +22,6 @@ services: POSTGRES_DB: scribe networks: - scribe_backend - # Lenient on purpose: health only orders app startup, so a stalled check - # must not get the database killed. - healthcheck: - test: ["CMD-SHELL", "pg_isready -U scribe"] - interval: 30s - timeout: 10s - retries: 10 - start_period: 180s - deploy: - restart_policy: - condition: on-failure - delay: 10s - max_attempts: 0 - window: 120s volumes: pgdata: diff --git a/docker-compose.quickstart.yml b/docker-compose.quickstart.yml index dc79f5eb..05a8bbe1 100644 --- a/docker-compose.quickstart.yml +++ b/docker-compose.quickstart.yml @@ -16,15 +16,8 @@ services: volumes: - app_data:/data depends_on: - db: - condition: service_healthy + - db restart: unless-stopped - healthcheck: - test: ["CMD", "python", "-c", "import urllib.request; urllib.request.urlopen('http://localhost:5000/api/health')"] - interval: 30s - timeout: 10s - retries: 3 - start_period: 30s db: # Postgres 17 with the vector extension Scribe needs. @@ -36,14 +29,6 @@ services: POSTGRES_USER: scribe POSTGRES_PASSWORD: scribe POSTGRES_DB: scribe - # Lenient on purpose: health only orders app startup, so a stalled check - # must not get the database killed. - healthcheck: - test: ["CMD-SHELL", "pg_isready -U scribe"] - interval: 30s - timeout: 10s - retries: 10 - start_period: 180s restart: unless-stopped volumes: diff --git a/docker-compose.yml b/docker-compose.yml index a074ceca..6da3ce1a 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -4,8 +4,8 @@ services: ports: - "5000:5000" depends_on: - db: - condition: service_healthy + - db + restart: unless-stopped volumes: - app_data:/data # Or a bind mount, for direct host access to the app's data: @@ -15,15 +15,10 @@ services: SECRET_KEY: "${SECRET_KEY:-dev-secret-change-me}" # Optional: a SearXNG instance to list as the web-search backend in Integrations. # SEARXNG_URL: "http://searxng:8080" - healthcheck: - test: ["CMD", "python", "-c", "import urllib.request; urllib.request.urlopen('http://localhost:5000/api/health')"] - interval: 10s - timeout: 5s - retries: 5 - start_period: 30s db: - image: postgres:16-alpine + # Postgres 17 with the vector extension Scribe needs. + image: pgvector/pgvector:pg17 stop_grace_period: 120s restart: unless-stopped volumes: @@ -32,14 +27,6 @@ services: POSTGRES_USER: ${POSTGRES_USER:-scribe} POSTGRES_PASSWORD: ${POSTGRES_PASSWORD:-scribe} POSTGRES_DB: ${POSTGRES_DB:-scribe} - # Lenient on purpose: health only orders app startup, so a stalled check - # must not get the database killed. - healthcheck: - test: ["CMD-SHELL", "pg_isready -U ${POSTGRES_USER:-scribe}"] - interval: 30s - timeout: 10s - retries: 10 - start_period: 180s volumes: pgdata: diff --git a/src/scribe/services/db_ready.py b/src/scribe/services/db_ready.py new file mode 100644 index 00000000..2460e8f4 --- /dev/null +++ b/src/scribe/services/db_ready.py @@ -0,0 +1,77 @@ +"""Wait for the database before the app starts. + +The container runs migrations first, and the database may still be starting +(or its host may not resolve yet) when it does. Waiting here, with a deadline, +lets the app start on an orchestrator's defaults instead of relying on a +healthcheck or a restart policy to retry it. +""" + +import asyncio +import logging +import time +from collections.abc import Awaitable, Callable + +import asyncpg +from sqlalchemy.engine import URL, make_url + +logger = logging.getLogger(__name__) + +# Long enough for a first boot that initialises the data directory on slow +# storage; past it the process exits non-zero and the orchestrator restarts it. +DEFAULT_DEADLINE_S = 300.0 + +# Errors that mean "not up yet". Anything else (a wrong password, a missing +# database) fails at once, because waiting would not change the answer. +_TRANSIENT = ( + OSError, # refused, unreachable, DNS not resolving yet, connect timeout + asyncpg.CannotConnectNowError, # "the database system is starting up" + asyncpg.TooManyConnectionsError, +) + + +def asyncpg_dsn(database_url: str | URL) -> str: + """The SQLAlchemy URL as a plain DSN asyncpg accepts.""" + url = make_url(database_url).set(drivername="postgresql") + return url.render_as_string(hide_password=False) + + +async def wait_for_database( + database_url: str | URL, + deadline_s: float = DEFAULT_DEADLINE_S, + *, + connect: Callable[..., Awaitable] = asyncpg.connect, + sleep: Callable[[float], Awaitable] = asyncio.sleep, + clock: Callable[[], float] = time.monotonic, +) -> None: + """Return once a connection succeeds; raise the last error after the deadline. + + Retries are spaced 1, 2, 4 … seconds apart, capped at 10, so a database + that is down for minutes sees a handful of attempts rather than a flood. + """ + dsn = asyncpg_dsn(database_url) + started = clock() + delay = 1.0 + attempt = 0 + while True: + attempt += 1 + try: + conn = await connect(dsn, timeout=10) + except _TRANSIENT as exc: + waited = clock() - started + if waited + delay > deadline_s: + logger.error( + "Database still unreachable after %.0fs (%d attempts): %s", + waited, attempt, exc, + ) + raise + logger.warning( + "Waiting for the database (attempt %d, %.0fs so far): %s", + attempt, waited, exc, + ) + await sleep(delay) + delay = min(delay * 2, 10.0) + continue + await conn.close() + if attempt > 1: + logger.info("Database reachable after %d attempts", attempt) + return diff --git a/tests/test_db_ready.py b/tests/test_db_ready.py new file mode 100644 index 00000000..93ee36fe --- /dev/null +++ b/tests/test_db_ready.py @@ -0,0 +1,100 @@ +"""The startup wait for the database: retries what clears up, fails what won't.""" + +import asyncpg +import pytest + +from scribe.services.db_ready import asyncpg_dsn, wait_for_database + +URL = "postgresql+asyncpg://scribe:p%40ss@db:5432/scribe" + + +class _Clock: + """A fake monotonic clock that only moves when the fake sleep is awaited.""" + + def __init__(self): + self.now = 0.0 + self.sleeps: list[float] = [] + + def __call__(self) -> float: + return self.now + + async def sleep(self, seconds: float) -> None: + self.sleeps.append(seconds) + self.now += seconds + + +class _Conn: + closed = False + + async def close(self): + self.closed = True + + +def _connect_failing(errors): + """A connect that raises each error in turn, then succeeds.""" + calls = [] + conn = _Conn() + + async def connect(dsn, timeout): + calls.append(dsn) + if len(calls) <= len(errors): + raise errors[len(calls) - 1] + return conn + + return connect, calls, conn + + +def test_dsn_drops_the_driver_and_keeps_the_escaped_password(): + assert asyncpg_dsn(URL) == "postgresql://scribe:p%40ss@db:5432/scribe" + + +async def test_returns_at_once_when_the_database_is_up(): + clock = _Clock() + connect, calls, conn = _connect_failing([]) + await wait_for_database(URL, connect=connect, sleep=clock.sleep, clock=clock) + assert len(calls) == 1 + assert clock.sleeps == [] + assert conn.closed + + +async def test_waits_through_a_database_that_is_still_starting(): + clock = _Clock() + connect, calls, conn = _connect_failing([ + ConnectionRefusedError("refused"), + OSError("Name or service not known"), + asyncpg.CannotConnectNowError("the database system is starting up"), + ]) + await wait_for_database(URL, connect=connect, sleep=clock.sleep, clock=clock) + assert len(calls) == 4 + assert clock.sleeps == [1.0, 2.0, 4.0] + assert conn.closed + + +async def test_backoff_is_capped_at_ten_seconds(): + clock = _Clock() + connect, _, _ = _connect_failing([ConnectionRefusedError()] * 6) + await wait_for_database(URL, connect=connect, sleep=clock.sleep, clock=clock) + assert clock.sleeps == [1.0, 2.0, 4.0, 8.0, 10.0, 10.0] + + +async def test_gives_up_with_the_last_error_once_the_deadline_passes(): + clock = _Clock() + connect, calls, _ = _connect_failing([ConnectionRefusedError("still down")] * 100) + with pytest.raises(ConnectionRefusedError, match="still down"): + await wait_for_database( + URL, deadline_s=30, connect=connect, sleep=clock.sleep, clock=clock, + ) + # 25s waited; another 10s would pass the deadline, so it raises instead. + assert clock.sleeps == [1.0, 2.0, 4.0, 8.0, 10.0] + assert len(calls) == len(clock.sleeps) + 1 + + +async def test_a_wrong_password_fails_at_once(): + clock = _Clock() + connect, calls, _ = _connect_failing([ + asyncpg.InvalidPasswordError("password authentication failed"), + ]) + with pytest.raises(asyncpg.InvalidPasswordError): + await wait_for_database(URL, connect=connect, sleep=clock.sleep, clock=clock) + assert len(calls) == 1 + assert clock.sleeps == []