---
title: Async DB session migration — retire get_db / NullPool sync-in-async
status: draft
epic: https://github.com/ccatobs/ops-db-api/issues/187
repos:
  - ccatobs/ops-db-api
---

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

> Source of truth for this epic. Tracker: [epic issue](https://github.com/ccatobs/ops-db-api/issues/187).
> The epic links here; this file is not embedded in the issue body.
> Seed: [issue #175](https://github.com/ccatobs/ops-db-api/issues/175).

## Problem

[PRD 0001](0001-db-session-lifecycle-hardening.md) 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](../adr/0005-async-session-and-eager-loading.md) (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 `selectinload`s 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.