PRD 0002: Async DB session migration — retire get_db / NullPool sync-in-async

Source of truth for this epic. Tracker: epic issue. The epic links here; this file is not embedded in the issue body. Seed: issue #175.

Problem

PRD 0001 hardened DB session/engine lifecycle across epic #158, but explicitly descoped its root cause as a human-gated design slice (#166): get_db (ccat_ops_db_api/dependencies/__init__.py:27-69) yields a sync SQLAlchemy Session with null_pool=True — a fresh connection per request. This is a band-aid over the real bug: three async def auth dependencies in ccat_ops_db_api/auth/unified_auth.py (get_current_user, get_current_user_optional, get_service_user) take this sync session via Depends(get_db) and call sync DB-querying helpers directly, blocking the event loop on every authenticated request. Because FastAPI caches a dependency’s result per callable per request, auth’s get_db() call and any endpoint’s own Depends(get_db) resolve to the same session, held for the whole request — including any awaited async work afterward.

Depends(get_db) appears 269 times across 47 files (ui/ 101/14, obs/ 58/9, pipeline/ 38/7, plan/ 37/8, evaluation/ 4/1, top-level routers 27, unified_auth.py 3, scheduler/ 0). Two overlapping open issues address adjacent slices of this surface: #77 (convert ~67 broken async def-with-sync-query endpoints to plain def) and #78 (the full ORM migration to AsyncSession). This epic fully absorbs both rather than leaving three overlapping efforts in flight, and retires #166 via the ADR it produces.

The actual blocking surface is larger than the sync-DB-call-site count suggests. Starlette decides sync-vs-async dispatch by inspecting the decorator wrapper, not the original function: require_permissions() (unified_auth.py:435) wraps every decorated endpoint in an async def that calls the wrapped function inline (result = func(*args, **kwargs), line 513), so a @require_permissions-decorated plain-def endpoint is dispatched as async and runs its entire sync DB body on the event loop — FastAPI’s threadpool protection never applies. An AST scan puts this at 142 of the 222 plain-def router endpoints (out of 290 total router endpoints: 68 async def, 222 plain def; 155 @require_permissions uses in routers). The real blast radius is closer to 210 of 290 endpoints, not just the auth dependencies plus the 68 genuinely async def endpoints. (require_roles, unified_auth.py:428, has a related but separate bug: an unconditional await func(...) that would 500 on a plain-def endpoint — latent today because its 5 current uses are all async def, but worth fixing in the same pass.)

There is already a parallel async stack to build on: AsyncDatabaseResources / build_async_database_resources() (dependencies/__init__.py:277-301), lifespan-owned, async_sessionmaker-based, built for the transaction-buffering subsystem (#162). A get_async_db() dependency exists in the same file but is wired into zero routers — dead scaffolding this epic promotes into the canonical request-scoped async dependency. The working async query idiom already lives in ccat_ops_db_api/transaction_buffering/ (transaction_executor.py, smart_query_manager.py, lsn_tracker.py): select(...) + await session.execute(...) against async_sessionmaker factories directly. That subsystem is fully async and decoupled from get_db() already — it needs no changes.

Solution

A full migration of all sync DB session usage in the request path to AsyncSession, landed as an epic branch collecting several sub-issue PRs (mirroring #158’s pattern), with the final epic-branch → develop PR held and tested against a local checkout before merging:

Landing strategy — decided: epic-branch-hold. One long-lived epic branch collects every slice, held and tested locally, single final PR to develop — mirroring #158. A reviewer counter-proposal argued for shipping each domain slice as an ordinary PR straight to develop once Slices 0+1 land, to avoid branch drift and get per-slice staging validation. Rejected: that proposal’s core worry — a long-lived branch drifting against develop over the epic’s duration — assumes human-paced sequential implementation. With LLM-orchestrated implementation across the slices, this isn’t the months-long-branch problem #158’s pattern was designed around; the branch’s lifetime is short enough that drift risk doesn’t dominate the decision. No partial-migration state reaches develop at any point during the epic.

• Slice 0 — foundation, no endpoints touched. Promote get_async_db to the canonical request-scoped session; fix secondary-site autoflush/ read-only parity on the async local session (see Implementation decisions); establish the selectinload eager-loading convention; stand up a real-DB async integration test harness (sqlite+aiosqlite or a Postgres testcontainer) — the current suite’s mocked DB sessions (tests/conftest.py) cannot surface the failure modes this migration introduces.

• Slice 1 — auth. Convert unified_auth.py, token_manager.py, admin.py, github_auth.py, and api_tokens.py to async with selectinload(User.roles) at the query sites. Fix require_permissions()/require_roles() dispatch in the same pass: await run_in_threadpool(func, ...) for a wrapped plain-def endpoint, await func(...) for a wrapped async def one. This single change restores FastAPI’s threadpool protection for every not-yet-migrated decorated router immediately — it converts the epic from “the API stays partially broken until each domain’s slice lands” into “the API is loop-safe from Slice 1 onward, and each later domain slice is a scalability upgrade rather than an urgent bugfix.” (Longer-term, converting require_permissions/require_roles from decorators to Depends-based guards would make this bug class structurally impossible — worth doing opportunistically per domain during the later slices, not as a separate effort.) This slice is independently implementable and testable as its own PR — the actual root-cause fix, because get_db and get_async_db are separate cached FastAPI dependencies, so auth can go async while business-logic endpoints are still migrating. It still lands on the epic branch like every other slice and does not ship to develop ahead of the rest: per the epic-branch-hold workflow above, nothing merges until the full migration is tested together on a local checkout. (Shipping auth’s fix to develop early would require an explicit exception to the hold-until-complete workflow — not assumed here.) Implemented as sub-issue #178; the async-auth conversion and the decorator-dispatch fix are recorded in ADR 0005 (decision 5). As built, the low-frequency token-management CRUD (token_manager.py, the api_tokens.py token endpoints) stays on the sync get_db session — it is not the per-request auth hot path and its authenticated-user .roles reads are already safe via eager loading — so the async conversion in this slice covers the auth dependency chain and github_auth.py’s login/role-mutation path rather than the token CRUD layer.

• Slices 2..N — one router/schema domain per PR (obs → plan → ui/* → evaluation → pipeline), converting endpoints and CRUD to await session.execute(select(...)), auditing every existing joinedload collection use for the .unique() requirement (see below), and auditing each response_model’s serialized relationships for eager-load coverage. scheduler/ has zero Depends(get_db) call sites (Redis-only) and needs no slice.

• Residual top-level routers slice — auth.py (2 sites), user_preferences.py (5), admin_diagnose.py (2), and admin_live_check.py (1) are not covered by Slice 1’s auth-module scope and need their own pass before the final slice can delete get_db (every remaining call site must be gone first).

• Special slice — routers/ui/transfer.py’s WebSocket handler (lines ~65-133), which drives blocking sync DB calls inside an async loop. It cannot use plain def (WebSocket routes can’t be sync) and needs a bespoke fix — genuine async DB access or run_in_threadpool.

• Final slice — delete get_db() and the sync request-path engine once zero Depends(get_db) call sites remain; set a short, explicit fail-fast pool_timeout on the async engine (_build_async_engine(), which currently has none and defaults to SQLAlchemy’s 30s) plus pool_pre_ping=True — this replaces the originally proposed NullPool→QueuePool revert entirely, since there is no surviving sync pool to revert. The ADR must write down the per-worker connection arithmetic — workers × (pool_size + max_overflow) per engine (main and local at the secondary site) — against Postgres max_connections headroom (~60 free of 100 today, per the connection budget above), since this is the number that changes if uvicorn workers are ever increased from today’s 1-per-process deployment.

Implementation decisions

• No surviving sync connection pool in the request path. get_db() and the sync engine are deleted outright once the migration completes; there is no bounded QueuePool fallback. The one legitimate sync residual is the dev-token-seed at FastAPI lifespan startup (main.py:~124-158) — one-shot, dev-only, outside the request path, calling the upstream ccat_ops_db package’s sync-only init_ccat_ops_db(). ccat_ops_db has no async engine or session helpers — no create_async_engine, AsyncSession, or async_sessionmaker anywhere in the package (a vestigial async_driver URL flag exists but is unreachable — get_engine() never passes it) — so adding async support there is out of scope; the dev-seed keeps its existing sync call.

• #77 is closed as superseded, not implemented. A migrated endpoint correctly stays async def once it gets real async DB access — #77’s “convert to plain def” is the opposite direction and never becomes the right answer for any endpoint in this epic’s scope. #77 could not have fixed the root cause even on its own terms: auth’s event-loop blocking comes from async def dependencies calling sync DB helpers while also awaiting other async work (verify_api_token, buffer_transaction) — the fix has to be auth’s session, not the endpoint’s def/async def choice. #77’s only real appeal — a cheap win shipped to develop while the full migration is still in progress — doesn’t apply here either, since the epic-branch-hold workflow means nothing in this epic reaches develop before the whole migration is tested together. It was also never fully implementable as written: roughly 20 endpoints mix genuine await work (the httpx OAuth exchange in github_auth.py, Redis pub/sub broadcasts in ui/transfer.py’s CRUD path, SmartQueryManager buffered reads in obs/raw_data_files_obs.py) with sync DB calls in the same function body — those could never have cleanly converted to plain def in the first place.

• ccat-api CLI is out of scope. Confirmed to be Redis/site-config only (ccat_ops_db_api/cli/) with zero DB access — no async migration surface there.

• Commit/rollback contract for get_async_db. The promoted dependency wraps its yield in try / except Exception: await session.rollback(); raise / finally: await session.close(). Write endpoints call await session.commit() explicitly; the dependency guarantees cleanup but never commits on an endpoint’s behalf. This applies uniformly across every migrated write endpoint in Slices 1 through N, not just auth.

• selectinload(User.roles), not joinedload, at the auth query sites (verify_jwt_token, verify_api_token — which is also the query get_service_user relies on for its user fetch). User.roles is a many-to-many collection (ccat_ops_db/models.py:1120, default lazy="select"); joinedload on a collection would force .unique() handling on the Result for no benefit. This one change covers all ~16 downstream lazy-read sites (unified_auth.py 421/522/591/596/605/613/647, dependencies/admin.py:29, routers/api_tokens.py 108/131/434) for the life of the request.

• github_auth.py’s role-mutation path becomes async. assign_roles_from_teams/get_or_create_user (currently sync) mutate user.roles via .append()/.remove() (lines 198/249/285); the collection must be eager-loaded before mutation, and the call chain becomes async def with an explicit await session.commit().

• Secondary-site read-only enforcement is new hardening, not restored parity — and it’s a hard requirement, not an optimization. get_db()’s read_only flag only gates DDL: init_ccat_ops_db skips create_schema when read_only=True and does nothing else. The only runtime guard get_db() gives the secondary today is autoflush=False (dependencies/__init__.py:65), which does not stop an explicit add()/commit() from succeeding. The async factories (build_async_database_resources) set neither autoflush=False nor any connection-level guard. This epic must add both to the async local session (autoflush=False plus an enforced guard, e.g. a connect-event running SET default_transaction_read_only) — this is genuinely new protection, and turning it on is a deliberate behavior change: it can surface secondary-site direct writes that currently succeed silently. Land it deliberately, with a clear rollout note, not as a drop-in “restore what was there.” This is the mechanism that keeps transaction-buffering’s outage fail-and-recover behavior intact going forward — the buffering subsystem’s own factories (transaction_executor, smart_query_manager, lsn_tracker) are otherwise untouched by this epic.

• expire_on_commit=False (already set on the existing async factories) must be preserved. Flipping it reintroduces MissingGreenlet on post-commit attribute reads (e.g. github_auth.py’s post-commit user.username read).

• Audit User.api_tokens’s cascade. cascade="all, delete-orphan" means session.delete(user) needs api_tokens eager-loaded or the cascade lazy-load faults under async. Bulk delete()/update() bypass ORM cascades entirely and need their own review where used.

• Connection budget confirmed comfortable. Primary Postgres runs ~40/100 configured worst-case connections today across all known consumers (ops-db-api, data-transfer, workflow-manager); removing the sync engine only reduces risk further, since NullPool was already uncapped and load-dependent rather than a fixed baseline. data-transfer’s Celery workers running with no -c concurrency flag (worst case scales with host CPU count) is a separate, pre-existing risk unrelated to this epic — noted for awareness, not addressed here. During the migration window, a request whose auth has migrated but whose endpoint hasn’t will transiently open both an async session (auth) and a sync NullPool connection (the endpoint) — expected, and immaterial at ~40/100, but named here since this bullet would otherwise read as airtight.

• This epic produces the first ADR touching sessions/pooling/async (docs/adr/ currently has none relevant) — it also formally retires #166.

Testing decisions

The current suite’s mocked DB sessions (tests/conftest.py, 100% MagicMock) cannot surface the failure modes this migration introduces — MissingGreenlet from an unloaded lazy relationship, a missing .unique() on a converted joinedload collection (181 joinedload uses vs. 3 selectinloads in routers today — this really is the highest-frequency mechanical trap), or a gap in the new secondary-site read-only guard. A real async integration-test harness is a Slice 0 deliverable, not an afterthought — split across two backends, not either/or: sqlite+aiosqlite for the broad per-router migration tests (ops-db’s models are already sqlite-compatible — JSONB is wrapped as JSON().with_variant(JSONB(), "postgresql"), UUID(as_uuid=True) emulates cleanly, no ARRAY type is used), and a small real-Postgres job (testcontainer or CI service container) for the two things sqlite cannot validate: the secondary-site read-only guard (SET default_transaction_read_only is a Postgres-only connect-event) and any JSONB-specific query behavior. Don’t let the convenient sqlite harness silently become the only one. At minimum, tests should:

• exercise the auth path end-to-end against a real async engine, confirming User.roles loads without MissingGreenlet and downstream require_roles/require_permissions checks work;

• cover at least one representative eager-load + response_model endpoint per migrated domain, confirming serialization doesn’t trigger a lazy load;

• attach a do_orm_execute event applying raiseload("*") to every ORM query on the test session factory only (do not change model lazy= defaults globally — sync consumers like data-transfer and workflow-manager legitimately rely on lazy loading). This turns any endpoint whose response serialization would lazy-load in production into a deterministic local test failure instead of a production MissingGreenlet — the cheapest available guard against the migration’s dominant failure mode;

• assert the secondary-site async local session is read-only (a write attempt against it fails) and has autoflush=False — this is new protection this epic adds (see Implementation decisions), not a restored guarantee, so write the test and its review as a behavior-change check, not a regression check;

• cover the github_auth.py role-mutation path (append/remove) under an async session;

• regression-test the .unique() requirement on any joinedload collection converted in each domain slice.

Related, non-blocking work (optional)

ccat_ops_db already generates an asyncpg URL (get_database_url(..., async_driver=True)) but has no async engine/session constructors, so this API hand-rolls _build_async_engine() locally. A small, additive ops-db work item — pure get_async_engine() / get_async_session_factory() constructors, deliberately without the module-level caching the sync path has (lifespan-owned lifecycle stays this app’s responsibility, per #158/#162) — would give pool sizing, pool_pre_ping, and connect-event policy (like the read-only guard) one home next to the models, and hand data-transfer/workflow-manager the same building block when they eventually go async. This is additive only and doesn’t touch ccat_ops_db’s existing engine caching or create_schema behavior, so it doesn’t conflict with this PRD’s out-of-scope boundary below — but it’s a cross-repo change and needs its own sign-off. Not a blocker for this epic; best sequenced as an early parallel PR in ops-db, or immediately after Slice 0, so the per-domain slices migrate onto the shared constructor instead of needing a second pass later.

Out of scope

• Adding async support to ccat_ops_db (the upstream ORM package) — it has no async engine/session helpers at all; the dev-token-seed’s one-shot sync call at startup is left as-is.

• PgBouncer / connection-proxy deployment (already out of scope for #158 too).

• data-transfer’s unbounded Celery worker connection consumption — a separate, pre-existing risk, flagged in this PRD for awareness only.

• Re-litigating the session/engine-lifecycle work already landed in #158.