[Ops] Backend hot-path performance batch: Redis pooling, SSE connection release, N+1s, composite index #96

New issue

Closed

opened 2026-07-14 19:48:27 +00:00 by claude-bot · 0 comments

claude-bot commented

2026-07-14 19:48:27 +00:00

Contributor

Context

Four verified, independent hot-path inefficiencies, batched into one issue.

(a) Per-call Redis clients. webapp/backend/app/auth/session.py:24 builds a new client via aioredis.from_url(...) on every call of its helper — invoked by create_session (:29), get_session (:38), and delete_session (:53), i.e. a fresh connect+teardown on EVERY authenticated request. Likewise webapp/backend/app/services/bot_pubsub.py:31 opens a new sync client per publish, and bot_pubsub.py:40,46,59 create new async clients per call.

(b) SSE holds a pooled DB session. session_record_stream (webapp/backend/app/routers/sessions.py:673) keeps its dependency-injected AsyncSession for the whole long-lived SSE stream although it is only used for auth/campaign resolution at the top. The engine (webapp/backend/app/database.py:14-18) uses default pool sizing (5 + 10 overflow), so a handful of open streams can exhaust the pool.

(c) N+1 queries. Per-slot vote queries in bot_session_timeslots (webapp/backend/app/routers/bot.py:255-258) and in _auto_close_voting_async (webapp/backend/app/tasks/reminder_tasks.py:931-933); per-NPC appearance queries in list_milestones (webapp/backend/app/routers/campaigns.py:1280-1285).

(d) Missing composite index. poll_session_reminders runs every minute and the bot's upcoming-session endpoint queries filter on status + confirmed_time, but sessions only indexes campaign_id (webapp/backend/app/models/session.py:73-77; status at :84 and confirmed_time at :89 are unindexed).

Fix / Spec

Pooling: module-level Redis connection pools (one async for auth/session + async pubsub paths, one sync for publish_bot_event), created once and reused; helpers return clients off the pool. No from_url per call anywhere in request paths.
SSE: resolve auth + campaign membership in a short-lived session (explicit AsyncSessionLocal() context or by finishing with the injected session before entering the loop) and ensure the DB connection is released before the stream loop starts. Also set explicit pool_size / max_overflow on create_async_engine (database.py:14), sized and commented.
N+1s: replace per-row loops with grouped aggregates — e.g. one select(Vote.time_slot_id, Vote.availability, func.count()).where(Vote.time_slot_id.in_(slot_ids)).group_by(...) for both vote sites, and one IN-based query (or func.count group-by) for NPC appearances in list_milestones.
Index: Alembic migration adding a composite index on sessions (status, confirmed_time); both upgrade and downgrade implemented.

Acceptance criteria

Existing test suite green.
grep shows no per-request/per-publish from_url client creation in auth/session.py or bot_pubsub.py.
PR includes an EXPLAIN or code-review note confirming the aggregate queries replace the loops (query count per endpoint call is O(1), not O(n)).
Migration applies cleanly up and down.

References

webapp/backend/app/auth/session.py:24-53
webapp/backend/app/services/bot_pubsub.py:31,40,46,59
webapp/backend/app/routers/sessions.py:673
webapp/backend/app/database.py:14-18
webapp/backend/app/routers/bot.py:255-258, webapp/backend/app/tasks/reminder_tasks.py:931-933, webapp/backend/app/routers/campaigns.py:1280-1285
webapp/backend/app/models/session.py:73-89

Filed from the July 2026 full-project review.

## Context Four verified, independent hot-path inefficiencies, batched into one issue. **(a) Per-call Redis clients.** `webapp/backend/app/auth/session.py:24` builds a new client via `aioredis.from_url(...)` on every call of its helper — invoked by `create_session` (`:29`), `get_session` (`:38`), and `delete_session` (`:53`), i.e. a fresh connect+teardown on EVERY authenticated request. Likewise `webapp/backend/app/services/bot_pubsub.py:31` opens a new sync client per publish, and `bot_pubsub.py:40,46,59` create new async clients per call. **(b) SSE holds a pooled DB session.** `session_record_stream` (`webapp/backend/app/routers/sessions.py:673`) keeps its dependency-injected `AsyncSession` for the whole long-lived SSE stream although it is only used for auth/campaign resolution at the top. The engine (`webapp/backend/app/database.py:14-18`) uses default pool sizing (5 + 10 overflow), so a handful of open streams can exhaust the pool. **(c) N+1 queries.** Per-slot vote queries in `bot_session_timeslots` (`webapp/backend/app/routers/bot.py:255-258`) and in `_auto_close_voting_async` (`webapp/backend/app/tasks/reminder_tasks.py:931-933`); per-NPC appearance queries in `list_milestones` (`webapp/backend/app/routers/campaigns.py:1280-1285`). **(d) Missing composite index.** `poll_session_reminders` runs every minute and the bot's upcoming-session endpoint queries filter on `status` + `confirmed_time`, but `sessions` only indexes `campaign_id` (`webapp/backend/app/models/session.py:73-77`; `status` at `:84` and `confirmed_time` at `:89` are unindexed). ## Fix / Spec 1. **Pooling**: module-level Redis connection pools (one async for auth/session + async pubsub paths, one sync for `publish_bot_event`), created once and reused; helpers return clients off the pool. No `from_url` per call anywhere in request paths. 2. **SSE**: resolve auth + campaign membership in a short-lived session (explicit `AsyncSessionLocal()` context or by finishing with the injected session before entering the loop) and ensure the DB connection is released before the stream loop starts. Also set explicit `pool_size` / `max_overflow` on `create_async_engine` (`database.py:14`), sized and commented. 3. **N+1s**: replace per-row loops with grouped aggregates — e.g. one `select(Vote.time_slot_id, Vote.availability, func.count()).where(Vote.time_slot_id.in_(slot_ids)).group_by(...)` for both vote sites, and one `IN`-based query (or `func.count` group-by) for NPC appearances in `list_milestones`. 4. **Index**: Alembic migration adding a composite index on `sessions (status, confirmed_time)`; both upgrade and downgrade implemented. ## Acceptance criteria - [ ] Existing test suite green. - [ ] `grep` shows no per-request/per-publish `from_url` client creation in `auth/session.py` or `bot_pubsub.py`. - [ ] PR includes an EXPLAIN or code-review note confirming the aggregate queries replace the loops (query count per endpoint call is O(1), not O(n)). - [ ] Migration applies cleanly up and down. ## References - `webapp/backend/app/auth/session.py:24-53` - `webapp/backend/app/services/bot_pubsub.py:31,40,46,59` - `webapp/backend/app/routers/sessions.py:673` - `webapp/backend/app/database.py:14-18` - `webapp/backend/app/routers/bot.py:255-258`, `webapp/backend/app/tasks/reminder_tasks.py:931-933`, `webapp/backend/app/routers/campaigns.py:1280-1285` - `webapp/backend/app/models/session.py:73-89` _Filed from the July 2026 full-project review._

claude-bot added the

backend

enhancement

labels

2026-07-14 19:48:37 +00:00

claude-bot added this to the v3.4.0 - Reliability, Operations & Self-Hosting milestone

2026-07-14 19:48:39 +00:00