diff --git a/backend/accounts/db.py b/backend/accounts/db.py index 8b97ff8..b22d9e5 100644 --- a/backend/accounts/db.py +++ b/backend/accounts/db.py @@ -62,11 +62,21 @@ def _apply_sqlite_pragmas(dbapi_conn, _rec): # # pool_size=1/max_overflow=1 (the original setting on both) meant a 3rd # concurrent request per worker had to wait out pool_timeout (~30s default) for a -# slot — easy to hit with 4 uvicorn workers x real traffic. 5+5 gives each async -# engine headroom for a burst of concurrent account requests without coming close -# to Postgres's own connection ceiling (two async engines here plus the sync -# engine below, times N workers, all share that budget). -_ASYNC_POOL_KWARGS = dict(pool_size=5, max_overflow=5, pool_pre_ping=True, +# slot — easy to hit with 4 uvicorn workers x real traffic. pool_size=5 keeps that +# headroom for the steady state. +# +# max_overflow is deliberately *tighter* than pool_size (2, not 5), because the +# overflow is the part nobody budgets for and it is what saturated Postgres on +# 2026-07-26. The connection ceiling is shared far more widely than this process +# knows: two async engines here plus the sync engine below, times N uvicorn +# workers, times the replica count (web autoscales), and prod and beta are two +# databases on ONE Postgres instance. At 5+5 the configured worst case across all +# of that exceeded the server's max_connections; at 5+2 prod web's ceiling is +# 4x14 rather than 4x20. Steady-state behaviour is unchanged — only the burst +# ceiling narrows, and a burst that would have opened a 6th..10th connection now +# waits on pool_timeout instead of being refused outright by the server (which is +# an error, not a wait). See infra/deploy/db/init/20-tuning.sh for the server side. +_ASYNC_POOL_KWARGS = dict(pool_size=5, max_overflow=2, pool_pre_ping=True, pool_recycle=1800, future=True) # Smaller than the async engines: only the notifier leader (one process # cluster-wide, see core/singleton.py) uses the sync engine, driving its periodic @@ -74,7 +84,8 @@ _ASYNC_POOL_KWARGS = dict(pool_size=5, max_overflow=5, pool_pre_ping=True, # statement_timeout (via psycopg's `options`) bound how long a wedged Postgres # can hang the notifier — it runs for the lifetime of that process's leader # flock, so an indefinite hang here would silently stop every subscription -# notification until the process is restarted. +# notification until the process is restarted. Left at 2+3: one process holds it, +# so it is not multiplied by the worker or replica count. _SYNC_POOL_KWARGS = dict(pool_size=2, max_overflow=3, pool_pre_ping=True, pool_recycle=1800, future=True, connect_args={"connect_timeout": 5, diff --git a/backend/tests/accounts/test_db_engines.py b/backend/tests/accounts/test_db_engines.py index 29454d5..cbe6e24 100644 --- a/backend/tests/accounts/test_db_engines.py +++ b/backend/tests/accounts/test_db_engines.py @@ -9,6 +9,28 @@ from sqlalchemy import select from accounts import db +# The server-side ceiling these pools spend from, and the deployment shape that +# spends it. Kept here so a pool change that stops fitting is a failing test +# rather than a 2am "sorry, too many clients already". Mirrors max_connections in +# infra/deploy/db/init/20-tuning.sh. ONE Postgres instance serves prod and beta. +MAX_CONNECTIONS = 200 +PROD_WEB_WORKERS = 4 # uvicorn workers per prod web replica +BETA_WEB_WORKERS = 2 +ASYNC_ENGINES_PER_PROCESS = 2 # RW + RO, both built from _ASYNC_POOL_KWARGS + + +def _async_ceiling_per_process() -> int: + """Connections one backend process can hold from its async pools at once.""" + k = db._ASYNC_POOL_KWARGS + return ASYNC_ENGINES_PER_PROCESS * (k["pool_size"] + k["max_overflow"]) + + +def _sync_ceiling() -> int: + """The notifier's sync pool. One process holds it cluster-wide (leader flock), + so it is NOT multiplied by workers or replicas.""" + k = db._SYNC_POOL_KWARGS + return k["pool_size"] + k["max_overflow"] + def test_defaults_to_sqlite_when_no_database_url(): assert db.IS_POSTGRES is False @@ -32,13 +54,18 @@ def test_postgres_pool_sizing_and_sync_timeouts(): """The Postgres engine-construction kwargs, defined at module scope so they're testable without a real Postgres connection (the engines built from them are only actually constructed when IS_POSTGRES, exercised by the docker-compose - stack -- see the module docstring). pool_size=1/max_overflow=1 (the old + stack -- see the module docstring). pool_size=1/max_overflow=1 (the original setting) let a 3rd concurrent request per worker wait out pool_timeout for a - slot; both async engines need real headroom, and the sync engine (the + slot, so the steady-state pool needs real headroom; the sync engine (the notifier's, driven off-event-loop) must bound connect + statement time so a wedged Postgres can't hang it forever under its leader flock.""" assert db._ASYNC_POOL_KWARGS["pool_size"] >= 5 - assert db._ASYNC_POOL_KWARGS["max_overflow"] >= 5 + + # The overflow is the part nobody budgets for -- it is multiplied by two async + # engines x uvicorn workers x replicas, against a ceiling shared with beta. + # Prod saturated max_connections on 2026-07-26 with this at 5. Keep it well + # under pool_size; the budget test below is the real constraint. + assert db._ASYNC_POOL_KWARGS["max_overflow"] <= 2 assert db._SYNC_POOL_KWARGS["pool_size"] < db._ASYNC_POOL_KWARGS["pool_size"] connect_args = db._SYNC_POOL_KWARGS["connect_args"] @@ -53,6 +80,27 @@ def test_postgres_pool_sizing_and_sync_timeouts(): engine.dispose() +def test_pool_budget_fits_under_max_connections(): + """At today's replica counts the whole estate's configured worst case must fit + inside the server's max_connections, with margin -- prod web + prod worker + + beta web + beta worker all draw on ONE Postgres instance. This is the test + that would have failed before 2026-07-26, when the configured worst case was + 170 against a ceiling of 100. + + Note this counts *configured maxima*: every pool at full overflow at the same + instant. Prod web autoscaling to its max of 3 replicas is deliberately NOT in + this sum -- that peak still does not fit, and closing it needs a connection + pooler or a lower autoscale ceiling rather than a bigger number here.""" + per_process = _async_ceiling_per_process() + prod = per_process * PROD_WEB_WORKERS + (per_process + _sync_ceiling()) + beta = per_process * BETA_WEB_WORKERS + (per_process + _sync_ceiling()) + + assert prod + beta <= MAX_CONNECTIONS * 0.8, ( + f"configured worst case {prod + beta} leaves too little room under " + f"max_connections={MAX_CONNECTIONS}" + ) + + def test_read_session_yields_a_working_session(): # On SQLite the read session is fully usable (no read-only enforcement); this # just confirms the dependency wiring resolves to a live AsyncSession. diff --git a/infra/deploy/db/init/20-tuning.sh b/infra/deploy/db/init/20-tuning.sh index 4791d10..78f8161 100755 --- a/infra/deploy/db/init/20-tuning.sh +++ b/infra/deploy/db/init/20-tuning.sh @@ -8,10 +8,10 @@ # 10-timescaledb.sql enables timescaledb. Settings are written via ALTER SYSTEM # (persisted to postgresql.auto.conf, which the timescaledb image's own # timescaledb-tune postgresql.conf defers to); the container's post-init restart brings -# restart-only settings (shared_buffers, …) into effect. NB: never ALTER SYSTEM SET -# shared_preload_libraries here — that would land in auto.conf and override the image's -# `timescaledb` preload. Init scripts do NOT re-run on an existing volume — to -# re-tune later, set DB_MEMORY and run this by hand, then restart: +# restart-only settings (shared_buffers, max_connections, …) into effect. NB: never +# ALTER SYSTEM SET shared_preload_libraries here — that would land in auto.conf and +# override the image's `timescaledb` preload. Init scripts do NOT re-run on an existing +# volume — to re-tune later, set DB_MEMORY and run this by hand, then restart: # docker compose exec -e DB_MEMORY=16g db bash /docker-entrypoint-initdb.d/20-tuning.sh # docker compose restart db set -euo pipefail @@ -33,13 +33,30 @@ if [ "$mb" -lt 1024 ]; then mb=8192; fi # maintenance_work_mem 512 MB) and scale linearly on a bigger box. shared_buffers=$((mb / 4)) # 25% — the shared page cache effective_cache=$((mb * 3 / 4)) # 75% — planner's view of total cache (PG + OS) -work_mem=$((mb / 128)) # ~64 MB at 8 GB (per-operation; kept modest) +maint_mem=$((mb / 16)) # 512 MB at 8 GB — index builds / VACUUM + +# work_mem is per *sort operation*, not per connection, so its real cost is +# work_mem x concurrent sorts x max_connections (200, below) — and the db container +# has a hard memory limit. Left uncapped the ratio gives 128 MB at prod's 16g +# budget, which is more than a 200-connection instance should carry: a burst of +# heavy sorts can walk into the container limit, and an OOM-killed Postgres is a +# far worse day than a refused connection. Cap at 64 MB (dev's 8g is unaffected). +work_mem=$((mb / 128)) +if [ "$work_mem" -gt 64 ]; then work_mem=64; fi if [ "$work_mem" -lt 16 ]; then work_mem=16; fi -maint_mem=$((mb / 16)) # 512 MB at 8 GB — index builds / VACUUM + +# Connection ceiling. NOT derived from DB_MEMORY: it is bounded by how many pools +# the app opens, not by RAM. One instance serves prod and beta, and every backend +# process opens three pools (two async engines + the notifier's sync engine, see +# backend/accounts/db.py), so the image default of 100 was below the configured +# worst case — prod saturated it on 2026-07-26 and started refusing clients with +# "sorry, too many clients already". 200 covers today's replica counts including +# web's autoscale maximum. ~+1 GB of per-connection overhead at 200. +max_conn=200 echo "[tuning] DB_MEMORY=${budget} -> ${mb}MB: shared_buffers=${shared_buffers}MB" \ "effective_cache_size=${effective_cache}MB work_mem=${work_mem}MB" \ - "maintenance_work_mem=${maint_mem}MB" + "maintenance_work_mem=${maint_mem}MB max_connections=${max_conn}" psql -v ON_ERROR_STOP=1 --username "$POSTGRES_USER" --dbname "$POSTGRES_DB" <