From b57e280ad6989585fb538f5c289c1b35745264af Mon Sep 17 00:00:00 2001 From: emi Date: Sun, 26 Jul 2026 18:49:42 +0000 Subject: [PATCH 1/3] frontend: load bookmarks-ui.js from the Go SSR home template too (#120) --- frontend/server/internal/render/templates/home.html.tmpl | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/frontend/server/internal/render/templates/home.html.tmpl b/frontend/server/internal/render/templates/home.html.tmpl index 2ce3918..7a03313 100644 --- a/frontend/server/internal/render/templates/home.html.tmpl +++ b/frontend/server/internal/render/templates/home.html.tmpl @@ -175,7 +175,10 @@

Browse all cities →

{{template "base_body_end" .}} - {{/* The records strip renders real temperatures server-side as .temp spans, so + {{/* Bookmarked locations: a star toggle in the results share row + a "Saved" + dropdown by the Find button. Loaded as its own module alongside app.js + (not merged into it) — see bookmarks-ui.js's header comment for why. */}} + {{/* The records strip renders real temperatures server-side as .temp spans, so it needs the same converter the SEO pages use or they'd stay in °F while the toggle says °C. Both import units.js, which the module loader dedupes. */}} From 3ec72ca75b3de9652473c81de6b96e11554bcd5e Mon Sep 17 00:00:00 2001 From: emi Date: Sun, 26 Jul 2026 18:51:02 +0000 Subject: [PATCH 2/3] bookmarks: don't fabricate a local save when the server rejects it (#121) --- frontend/static/bookmarks-ui.js | 4 ++++ frontend/static/bookmarks.js | 23 ++++++++++++++++++++--- 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/frontend/static/bookmarks-ui.js b/frontend/static/bookmarks-ui.js index 82877ed..2f3b6a7 100644 --- a/frontend/static/bookmarks-ui.js +++ b/frontend/static/bookmarks-ui.js @@ -111,6 +111,10 @@ function buildStar() { try { if (existing) await bookmarks.remove(existing.id); else await bookmarks.add(loc.lat, loc.lon, currentLabel()); + } catch (e) { + // A real server rejection (e.g. the per-user cap) — bookmarks.add() no + // longer fakes a local save for this, so say why nothing changed. + alert(e.message || "Couldn't save this location."); } finally { btn.disabled = false; } diff --git a/frontend/static/bookmarks.js b/frontend/static/bookmarks.js index 361adfa..fd64a25 100644 --- a/frontend/static/bookmarks.js +++ b/frontend/static/bookmarks.js @@ -97,17 +97,34 @@ async function syncFromServer(preSyncLocal) { notify(); } -/** Save (or re-save with a new label) the given spot. Upserts either way. */ +/** Save (or re-save with a new label) the given spot. Upserts either way. + * + * Throws if signed in and the server explicitly rejects the request (e.g. the + * 409 per-user cap, a 422 on bad input) — that is a real failure, not the + * "offline" case below, and must not be papered over with a local-only + * bookmark that doesn't actually exist server-side and would just vanish, + * unexplained, on the next sync. A genuine network failure (no response at + * all) still falls through to the local-only save so the star visibly + * sticks even offline. */ export async function add(lat, lon, label) { const id = cellId(lat, lon); if (signedIn) { + let res = null; try { - const res = await apiFetch(uv("bookmarks"), { method: "POST", json: { lat, lon, label: label || undefined } }); + res = await apiFetch(uv("bookmarks"), { method: "POST", json: { lat, lon, label: label || undefined } }); + } catch (e) { + // offline / network failure: fall through to the local-only path below. + } + if (res) { if (res.ok) { const rows = await fetchServerList(); if (rows != null) { cache = rows.map(fromServerRow); writeLocal(cache); notify(); return find(lat, lon); } + } else { + let msg = `Couldn't save this location (${res.status}).`; + try { const d = await res.json(); if (typeof d.detail === "string") msg = d.detail; } catch (e) { /* no body */ } + throw new Error(msg); } - } catch (e) { /* fall through so the star still visibly sticks, even offline */ } + } } let b = cache.find((x) => cellId(x.lat, x.lon) === id); if (b) { if (label) b.label = label; } From 0617a3b067601d37e2922b1073bbb586612200ec Mon Sep 17 00:00:00 2001 From: emi Date: Sun, 26 Jul 2026 19:48:50 +0000 Subject: [PATCH 3/3] db: raise max_connections to 200, cap work_mem at 64MB, trim async pool overflow (#124) --- backend/accounts/db.py | 23 +++++++--- backend/tests/accounts/test_db_engines.py | 54 +++++++++++++++++++++-- infra/deploy/db/init/20-tuning.sh | 35 ++++++++++++--- 3 files changed, 96 insertions(+), 16 deletions(-) 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" <