appproxy-coordinator DB engine missing pool_pre_ping: a dropped Postgres connection poisons the pool with "SAVEPOINT can only be used in transaction blocks" until restart
- Dominant language
- Python
- Stars
- 670
- Forks
- 183
- Avg merge
- 15h 13m
- Merged PRs (30d)
- 368
Description
## Summary
The appproxy-coordinator SQLAlchemy engine is created **without** `pool_pre_ping` (and without `pool_recycle`), unlike the manager engine which has both. When a pooled Postgres connection is dropped underneath SQLAlchemy (server-side termination / failover / network blip / `idle_in_transaction_session_timeout`), asyncpg silently reconnects the physical socket with a clean, no-transaction state, but SQLAlchemy's pooled connection still carries stale transaction state. The next query is then issued as a nested (SAVEPOINT) transaction, which the fresh asyncpg connection rejects:
```
asyncpg.exceptions.NoActiveSQLTransactionError: SAVEPOINT can only be used in transaction blocks
```
Without `pool_pre_ping`, SQLAlchemy never validates the connection at checkout, so this desynced connection stays in the pool and every subsequent request reusing it fails the same way. The service is effectively down until the process is restarted; there is no automatic recovery.
## History: coordinator never received the manager's robust-connection fix
The manager engine gained `pool_pre_ping` in **#1991 "feat: Enable robust DB connection handling" (2024-04-03, commit 2a2b5e5a6)** — which targets exactly this class of dropped-connection failure. account-manager also has it. The appproxy-coordinator is a separate code path and never received that hardening, so it remains vulnerable. This ticket is essentially "apply the #1991 robust-connection handling to appproxy-coordinator."
## Trigger — a dropped connection, NOT a serialization conflict
The very first error in the incident is already the SAVEPOINT error, with **no preceding SQLSTATE 40001 / serialization failure** in the coordinator log. The full traceback shows it happens while asyncpg is _starting a transaction_ for the first statement of a `PATCH /api/worker/{id`} heartbeat handler:
```
sqlalchemy/dialects/postgresql/asyncpg.py _prepare_and_execute
-> adapt_connection._start_transaction()
-> asyncpg transaction.start() -> execute("SAVEPOINT ...")
-> NoActiveSQLTransactionError: SAVEPOINT can only be used in transaction blocks
[SQL: SELECT workers ... WHERE workers.id = $1] # Worker.get, first query of heartbeat
```
i.e. SQLAlchemy believes an outer transaction exists (so it opens a SAVEPOINT), but the underlying asyncpg connection has none — the classic symptom of a connection that was dropped and transparently reconnected while the pool kept stale state.
(Note: high-volume `SerializationError` (SQLSTATE 40001) events sometimes seen on the **manager** side come from `container_registry:image` scan/update and are a _separate_ issue, unrelated to this coordinator bug.)
## Impact (observed in a production deployment)
- appproxy-coordinator ran degraded for ~24h with every worker `PATCH /api/worker/{id`} heartbeat returning HTTP 500.
- Worker heartbeats could not be persisted, the coordinator marked workers stale, and app/SSH endpoint creation failed with `AppLaunchError: Worker not available` (surfaced in the WebUI as `Failed to fetch` / `Worker not available`).
- Recovered only by restarting the coordinator process.
## Root cause
`src/ai/backend/appproxy/coordinator/models/utils.py` `connect_database()` builds the engine without connection-health settings:
```python
db = create_async_engine(
str(db_url),
connect_args=pgsql_connect_opts,
pool_size=db_config.pool_size,
max_overflow=db_config.max_overflow,
# <-- no pool_pre_ping, no pool_recycle
...
)
```
Manager (`src/ai/backend/manager/models/utils.py`, since #1991) and account-manager both have:
```python
db = create_async_engine(
...
pool_recycle=db_config.pool_recycle,
pool_pre_ping=db_config.pool_pre_ping,
...
)
```
Contributing factor: `pgsql_connect_opts` sets `idle_in_transaction_session_timeout = 60s`, so a pooled connection left idle inside a transaction is terminated server-side after 60s — a concrete way to produce the dropped connection that then poisons the pool.
## Fix
1. **Primary: port the #1991 robust-connection handling to appproxy-coordinator** — add `pool_pre_ping` and `pool_recycle` to the coordinator engine and the corresponding `DBConfig` fields (currently only `pool_size` / `max_overflow` exist). `pool_pre_ping` issues a lightweight liveness check at checkout and transparently reconnects dead connections with clean state, which prevents the stale-connection desync entirely.
- Note: manager's / account-manager's `pool_pre_ping` defaults to `False`. This ticket defaults the coordinator's to `True` so the resilience is on by default; consider also flipping the manager/account-manager defaults (or enabling them in deployment config) so production actually benefits.
1. **Secondary hardening (optional):** `execute_with_txn_retry` currently reuses one caller-provided connection across all retry attempts, so once a connection is poisoned it cannot recover mid-loop. Running each attempt on a fresh transaction/connection would make the retry path robust as well. Aggravator, not root cause.
## Reproduction
1. Open a pooled coordinator connection, begin a transaction, leave it idle > `idle_in_transaction_session_timeout` (60s) so Postgres terminates it (or force a Postgres restart/failover).
1. Reuse the pooled connection: SQLAlchemy issues a SAVEPOINT on the reconnected asyncpg connection -> `NoActiveSQLTransactionError: SAVEPOINT can only be used in transaction blocks`, repeating for all subsequent reuse.
1. With `pool_pre_ping=True`, checkout detects the dead connection and reconnects cleanly, and the error does not occur.
## Affected code
- `src/ai/backend/appproxy/coordinator/models/utils.py::connect_database` / `create_async_engine` (missing `pool_pre_ping` / `pool_recycle`)
- `src/ai/backend/appproxy/coordinator/config.py::DBConfig` (missing `pool_pre_ping` / `pool_recycle` fields)
- secondary: `execute_with_txn_retry` in the same module (and the identical one in manager / account-manager)
Reference: manager gained this via #1991 (2024-04-03). Present on `main` (26.4.4).
JIRA Issue: BA-6678
Contributor guide
Research direction
Read src/ai/backend/appproxy/coordinator/models/utils.py::connect_database and src/ai/backend/appproxy/coordinator/config.py::DBConfig, then compare them with the manager implementation from #1991. Verify the coordinator exposes and applies the pool health settings, with recovery from dropped pooled connections as the completion criterion; treat execute_with_txn_retry as optional secondary hardening.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- postgresql, python, sqlalchemy
- Domain
- backend, databases
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 58/100