fix(db): bound asyncpg statement and connect time so a dead socket recovers - #1552

Two conflicts, both resolved by keeping each side:

- `database_manager.py`: speedkick (MemMachine#1552) factored the engine keywords into
  `_sql_engine_kwargs` and added asyncpg's command/connect timeouts, exactly
  where this branch adds `enable_sqlite_foreign_keys` and its inline keyword
  building. Kept speedkick's helper as the keyword source and this branch's
  pragma hook on the constructed engine.
- `test_event_backend_wiring.py`: `import logging` (this branch) against
  `import math` (speedkick, from MemMachine#1547). Kept both.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01PVtg6Zea292Pb9L7GXnTJp

…covers (MemMachine#1552)

A pooled connection whose peer stops responding is not an error, it is a
wait. The kernel retransmits with exponential backoff - observed at
tcp_retries2 attempt 13, Send-Q stuck at 34 bytes and unchanged over
20 seconds - and asyncpg has no deadline of its own, so the request blocks
for the whole of it. pool_pre_ping does not help: its liveness SELECT is
written into the same dead socket and waits with everything else.

The failure this produces is worse than a slow request. The pool never
discards the dead connections, so the process stays wedged after the network
recovers and only a restart clears it. Seen twice on a benchmark deployment:
searches timing out indefinitely while /api/v2/health answered in about a
millisecond, four uvicorn workers idle in select(), no lock contention in
Postgres, and a fresh connection from the same pod completing in 18 ms.

create_async_engine was passed no connect_args, so nothing bounded either
the statement or the connect. This adds command_timeout and connect_timeout,
defaulted to 60 s and 10 s and settable to null for the old behaviour. Both
are gated on driver == "asyncpg": they reach asyncpg.connect() and mean
nothing to aiosqlite or aiomysql. The keyword assembly moves into a helper,
because adding a branch to async_get_sql_engine pushed it past ruff's
complexity limit and the repeated "if not None" lines were asking for it.

Verified by fault injection on a deployed platform - iptables DROP on the
pod's traffic to Postgres:5432, so pooled connections wedge exactly as they
did in the incident:

                        under the fault          after the fault is removed
  stock        no response at 150 s (cap)    still hung, no response at 90 s
  with fix     500 after 194 s               200 in 1.5 s

The recovery is the point: the stock build stays wedged once the network is
healthy again, which is what forced the restarts.

Under a total outage the request still takes ~194 s to fail rather than
~60 s. A search touches three separate stores, so it pays a timeout per
store; the bound is roughly command_timeout times the number of stores
touched, not command_timeout. Lowering the default would shorten it
proportionally - 60 s is chosen as generous against real query times here,
which are milliseconds, and is left tunable rather than tuned.

ruff check and ruff format both pass.

Claude-Session: https://claude.ai/code/session_01Nr9kacmpFVTTfkZRw6esxP

Co-authored-by: Claude Opus 5 <[email protected]>
(cherry picked from commit 10b3706)
Signed-off-by: Haiyan Wang <[email protected]>

…covers (port of #1552 to main) (#1679)

* fix(db): bound asyncpg statement and connect time so a dead socket recovers (#1552)

A pooled connection whose peer stops responding is not an error, it is a
wait. The kernel retransmits with exponential backoff - observed at
tcp_retries2 attempt 13, Send-Q stuck at 34 bytes and unchanged over
20 seconds - and asyncpg has no deadline of its own, so the request blocks
for the whole of it. pool_pre_ping does not help: its liveness SELECT is
written into the same dead socket and waits with everything else.

The failure this produces is worse than a slow request. The pool never
discards the dead connections, so the process stays wedged after the network
recovers and only a restart clears it. Seen twice on a benchmark deployment:
searches timing out indefinitely while /api/v2/health answered in about a
millisecond, four uvicorn workers idle in select(), no lock contention in
Postgres, and a fresh connection from the same pod completing in 18 ms.

create_async_engine was passed no connect_args, so nothing bounded either
the statement or the connect. This adds command_timeout and connect_timeout,
defaulted to 60 s and 10 s and settable to null for the old behaviour. Both
are gated on driver == "asyncpg": they reach asyncpg.connect() and mean
nothing to aiosqlite or aiomysql. The keyword assembly moves into a helper,
because adding a branch to async_get_sql_engine pushed it past ruff's
complexity limit and the repeated "if not None" lines were asking for it.

Verified by fault injection on a deployed platform - iptables DROP on the
pod's traffic to Postgres:5432, so pooled connections wedge exactly as they
did in the incident:

                        under the fault          after the fault is removed
  stock        no response at 150 s (cap)    still hung, no response at 90 s
  with fix     500 after 194 s               200 in 1.5 s

The recovery is the point: the stock build stays wedged once the network is
healthy again, which is what forced the restarts.

Under a total outage the request still takes ~194 s to fail rather than
~60 s. A search touches three separate stores, so it pays a timeout per
store; the bound is roughly command_timeout times the number of stores
touched, not command_timeout. Lowering the default would shorten it
proportionally - 60 s is chosen as generous against real query times here,
which are milliseconds, and is left tunable rather than tuned.

ruff check and ruff format both pass.

Claude-Session: https://claude.ai/code/session_01Nr9kacmpFVTTfkZRw6esxP

Co-authored-by: Claude Opus 5 <[email protected]>
(cherry picked from commit 10b3706)
Signed-off-by: Haiyan Wang <[email protected]>

* fix(db): surface the new timeouts in the chart, explain asyncpg's key name

Review follow-ups on #1679.

Shu: add both fields to the sample configuration. They now sit beside the
pool_* settings in the chart's values and are rendered into the server config,
following the same pattern.

Setting either to null in values.yaml renders an empty value, which YAML reads
as null and the config model accepts, so the documented "null restores the
unbounded behaviour" is reachable from the chart and not just from a hand-written
config. Checked by rendering the chart both ways and feeding the result through
SqlAlchemyConf and _sql_engine_kwargs.

Edwin: rename the unqualified "timeout" key. It cannot be renamed - it is
asyncpg's own parameter name for the connect deadline, and asyncpg has no
connect_timeout parameter, so the key is fixed by their API. Our config field is
already qualified as connect_timeout; only the wire name is bare. Added a comment
so the next reader does not have to check asyncpg's signature to find that out.

ruff, ty (3.12) and helm lint clean; server unit suite unchanged at 1869 passed.

Signed-off-by: Haiyan Wang <[email protected]>

* docs(config): add the two timeouts to the sample configs

Shu asked for these in the sample config as well; the earlier commit only
reached the Helm chart, which is a different file.

Both fields now sit beside the pool_* settings in the profile_storage block of
all three episodic_memory_config samples, with the same comment style.

Checked rather than assumed: each sample still parses, its profile_storage
config still validates against SqlAlchemyConf, and the documented "~ = unbounded"
resolves to null and drops connect_args entirely.

Signed-off-by: Haiyan Wang <[email protected]>

---------

Signed-off-by: Haiyan Wang <[email protected]>
Co-authored-by: Claude Opus 5 <[email protected]>