learningcircuit--local-deep-research/docs/decisions/0004-nullpool-for-sqlcipher.md

# ADR-0004: QueuePool (not NullPool) for SQLCipher databases

**Date:** 2026-04-13
**Status:** Accepted

## Context

Each user has their own SQLCipher-encrypted SQLite database, opened at
login and closed at logout. Background threads (research workers,
metric writers, news scheduler jobs) need database sessions for the
same user concurrently with Flask request handlers.

### Why QueuePool

SQLCipher's `PRAGMA key` adds ~0.2 ms per connection open. With 20–30
queries per page load, NullPool (new connection per checkout) adds a
noticeable 4–6 ms overhead vs QueuePool's ~1.5 ms for pool-resident
connections.

### Why not per-thread NullPool engines

An earlier design maintained a second engine system: one NullPool
engine per `(username, thread_id)` in `_thread_engines`, used by
background threads for metric writes. This was removed in PR #3441
because:

1. **FD leak.** Each SQLCipher + WAL connection holds 3 file
   descriptors (main db + WAL + SHM). Orphaned thread engines — left
   behind when `@thread_cleanup` did not fire — accumulated FDs
   unboundedly, eventually exhausting the 1024 soft limit and crashing
   the server with `OSError: [Errno 24] Too many open files`.

2. **Architectural redundancy.** The per-user QueuePool engine is
   already created with `check_same_thread=False`, making it safe for
   background threads. Routing all work through one bounded pool per
   user keeps FD usage at `pool_size + max_overflow` (currently 60)
   instead of scaling with background-thread count.

### SQLCipher + WAL handle leak workaround

SQLCipher in WAL mode leaks file handles when pooled connections close
out of open-order (a known issue with WAL-mode SQLite engines under
connection pooling). The cleanup scheduler in `connection_cleanup.py`
calls `engine.dispose()` on all per-user engines every 30 minutes,
closing all idle pooled connections and resetting handle state. This is
a workaround, not a root fix — it limits accumulation to a 30-minute
window.

### Current pool sizing

```
pool_size     = 20
max_overflow  = 40
pool_timeout  = 10   # seconds; fail fast rather than queue
pool_recycle  = 3600 # seconds; recycle stale connections
pool_pre_ping = True
```

Peak FD usage per user: `(20 + 40) × 2 + 1 = 121` (WAL mode).

## Decision

Use a single shared QueuePool engine per user for all threads (request
handlers and background workers). Do not maintain per-thread engines.
Periodic `dispose()` mitigates the SQLCipher+WAL handle leak.

## Consequences

- FD usage is bounded and predictable.
- `pool_timeout=10` makes pool exhaustion a loud error rather than a
  silent deadlock.
- The `ParallelConstrainedStrategy` (`max_workers=100`) could
  theoretically spike past 60 simultaneous checkouts. Sessions are
  short-lived (millisecond metric writes), so sustained contention is
  unlikely. Flagged as a known follow-up.

## Addendum — PR #3487 investigation (2026-04-16)

PR #3487 proposed skipping the 30-min `engine.dispose()` for any engine
with `pool.checkedout() > 0`, citing a claim that dispose orphans
checked-out connections and causes torn writes in the post-login bulk
settings import. Investigation found:

- **SA 2.0 source disagrees.** `QueuePool.dispose` only drains idle
  queue entries (`_pool.get(False)`); `Engine.dispose` replaces the
  pool via `pool.recreate()`. SA docs: *"Connections that are still
  checked out will not be closed."* A thread holding a checked-out
  connection keeps using it until return.
- **Real root cause was elsewhere.** The sticky-loop symptom came
  from the post-login path committing twice
  (`load_from_defaults_file(commit=True)` then `update_db_version()`
  with its own commit), which left `app.version` unwritten on any
  inter-commit failure. Fix: one session, one terminal commit, both
  calls use `commit=False`. See `web/auth/routes.py` ATOMICITY
  INVARIANT comment.
- **Regression guard:** `tests/database/test_post_login_settings_atomicity.py`
  locks in both properties — the atomic all-or-nothing write and the
  SA 2.0 checked-out-survives-dispose contract — so neither can
  silently regress.

Future refactors of the cleanup cycle should preserve the
checked-out-survival property; do not add a `checkedout()` skip guard
without a real reproducer.