feat(db): configurable writer session timeouts (lock, idle-txn, statement)

[TheSentinel454]

Why

A wedged relay boot pod holding a relation lock can park every other writer in the fleet behind it: DB load pins at pool capacity in Lock:relation waits while CPU stays flat, and nothing server-side releases the lock until the holder dies. We hit exactly this in production — ~1,400 sessions queued behind one crash-looping pod's boot transaction for ~20 minutes until kubelet killed the container.

What

Applies session-level Postgres timeouts to every writer connection inside the existing single after_connect hook in buzz-db, all env-tunable through the same Config::from_env → DbConfig path as the existing pool-size knobs:

Env var	GUC	Default	Effect
BUZZ_DB_LOCK_TIMEOUT_MS	lock_timeout	5000	statements waiting on any lock fail fast instead of parking behind a wedged holder
BUZZ_DB_IDLE_TXN_TIMEOUT_MS	idle_in_transaction_session_timeout	60000	reaps wedged clients idling inside an open transaction while holding locks
BUZZ_DB_STATEMENT_TIMEOUT_MS	statement_timeout	0 (off)	opt-in runaway-statement cap; off by default because startup migrations/backfills legitimately run long statements

0 disables a timeout (Postgres semantics) and deliberately passes through the env parsing — unlike the pool-size knobs where 0 falls back to the default. The reader pool is untouched: replica sessions never take contended locks and already fail acquire in 150 ms.

Deployers tune these via plain env vars (.env, or relay.extraEnv in the Helm chart) — no code changes needed.

Behavior change to note

With the 5 s default lock_timeout, a boot-time migration or backfill that waits >5 s on a lock now errors (surfacing in logs / crash-looping the pod) instead of stalling silently. That is the intended visible-failure-over-fleet-stall tradeoff; deployers with slow contended migrations can set BUZZ_DB_LOCK_TIMEOUT_MS=0.

Testing

• cargo test -p buzz-db -p buzz-relay — buzz-db green; buzz-relay has 9 failures that also fail on clean main in this environment (api::admin/api::media/mesh_demo — unrelated, pre-existing).
• New config test covers override / 0-passthrough / invalid-fallback for all three env vars.
• Extended the existing writer_pool_safety_hook_is_single_and_composed source-shape test so the timeouts can't drift out of the single after_connect hook (SQLx replaces hooks — a second hook would silently disarm the floor guard).
• cargo fmt --check and cargo clippy --all-targets clean for the touched crates.

Closest existing PR/issue: none found.

[TheSentinel454]

Open to bumping this until we have a better understanding of how long we tend to spend running migrations.

[TheSentinel454]

🤖 Leaving this thread open: the 5s default is unchanged pending real migration-duration data. There are no boot-time migration duration metrics today; the proposal on the table is a buzz_boot_migration_duration_seconds gauge around db.migrate() (each pod boot is a sample), either in this PR or a small follow-up — awaiting the maintainer's call. Note that as of 605244c the migration advisory-lock path is exempt from lock_timeout/statement_timeout, so the default now only governs runtime traffic, which lowers the stakes of the choice.

This reply was generated by an AI agent (Fizz).

[wpfleger96]

🤖 Combined review — two independent agent passes (Paul and Thufir) that converged on the migration finding, deduped into the inline comments below. The mechanism itself looks right to me: one composed after_connect hook, 0-passthrough env semantics, reader pool left alone, bare-integer ms values verified live against Postgres. Nobody's asking for a different design — two blocking items and two nits.

[TheSentinel454]

To be honest, we shouldn't be doing this, and we need to move away from applying migrations on boot, but that's a bigger change that I'll be pushing for separately.

[wpfleger96]

🤖 Re-reviewed the exact current head. The previous two IMPORTANT findings plus two MINOR notes are addressed for pools constructed through Db::new(): the Postgres-backed CI test covers effective GUCs, ordinary 55P03 contention, and migration-lock exemption; independent PG 17 verification reproduced 55P03 at 502 ms and migration success after waiting 1,521 ms past both configured lock/statement budgets. Admin and deletion now share the centralized env overlay, and the comments/docs are accurate.

IMPORTANT / Correctness — crates/buzz-relay/src/main.rs:356-364: the timeout policy is only installed by Db::new(), but the audit service is a separate production relay writer pool built with raw PgPoolOptions. AuditService::log() writes audit_log in a transaction and waits on a session-scoped pg_advisory_lock; live verification at this exact head showed all three GUCs remained 0 on a production-shaped direct pool and the waiter was still blocked at the 1,201 ms harness deadline. Because the relay has one audit worker and a bounded queue whose producers use .send().await, a blocked audit lock can stall the worker, fill the queue, and backpressure event/media handlers.

Please arm the audit writer with the same session settings—preferably through a reusable buzz-db pool configuration helper rather than duplicated SQL—and add a Postgres-backed regression asserting its effective GUCs and bounded advisory-lock wait. The separately deployed push gateway also constructs a raw writer pool; explicitly decide/document whether it belongs in this PR’s stated “every writer” scope.

[wpfleger96]

🤖 Combined review of exact head 9bbcade722b7a680e764feabcdb5acf738347ed3 — three independent agent passes (two source reviews plus a clean-relay live E2E run), deduped here. Both live probes converged on the same new defect independently, which is strong confirmation it's real.

Previous round's findings — all addressed:

• The audit pool now goes through the renamed public Db::connect_writer_pool via connect_audit_pool() (crates/buzz-relay/src/main.rs:38), inheriting the timeouts, the created_at floor guard, and the READ COMMITTED assertion; the source-shape guard test tracks the new name so the single-after_connect-hook invariant can't drift.
• Migration/schema-destruction connections exempt their legitimate long waits. Proven live twice: a relay with BUZZ_DB_LOCK_TIMEOUT_MS=300 waited ~3 s behind the schema-migration advisory lock and completed startup.
• Admin, deletion, relay, and audit pool configuration share the centralized env overlay; comments and operator docs match PostgreSQL semantics.
• The requested Postgres-backed regressions exist and actually run: the Backend Integration job at this head executed both focused tests from the archive (session_timeouts_install_through_db_new_and_bound_lock_waits PASS 4.96 s, audit_writer_pool_installs_timeouts_and_bounds_advisory_lock_waits PASS 0.52 s). The final commit's CI split is a genuine hardening: separate nextest invocations mean a relay-binary test missing from the archive fails loudly instead of the combined OR-filter silently passing on the buzz-db half.
• Push-gateway scope decision is documented consistently in .env.example, the with_session_timeouts_from_env doc, and docs/push-gateway-deployment.md.

Core mechanism verified live (clean relay, isolated Postgres/Redis/MinIO, head-built binaries): a real message write behind an ACCESS EXCLUSIVE lock on events failed in 0.486 s instead of parking, the relay stayed ready throughout, and accepted/read messages normally after release — the exact incident shape this PR exists to fix.

IMPORTANT / Correctness — audit lock timeouts now permanently discard accepted events' audit entries. connect_audit_pool() correctly installs the 5 s default lock_timeout, but AuditService::log() propagates SQLSTATE 55P03 and log_audit_entry() (crates/buzz-relay/src/state.rs:1342-1349) only logs the error and increments a metric before consuming the queue item — no retry, no durable outbox. Both live probes reproduced this independently at this exact head: holding one community's audit advisory lock past the timeout, the relay accepted and persisted the message (event_rows=1), the worker logged canceling statement due to lock timeout, and audit_rows remained 0 (one probe additionally drove a real end-to-end channel message through the head-built CLI: event accepted, permanently unaudited). Releasing the lock let the next event audit normally — transient contention became permanent audit loss, not database unavailability. Before this PR the audit pool had no lock_timeout, so the failure mode was indefinite worker blockage; the fix converts it into silent audit-chain gaps, which regresses the durable-audit contract (SECURITY.md:67-74, VISION_MODERATION.md) and the queue's stated no-drop intent.

Required fix: preserve the current queued entry across retryable lock-timeout failures — retry with bounded backoff until appended, or use a transactional durable outbox if request-path decoupling must survive prolonged contention. Add a Postgres-backed worker-level regression that holds the advisory lock past lock_timeout, releases it, then proves the original accepted event is eventually audited (the current audit test proves the pool fails fast, not that the consuming workflow preserves the entry). A fix also needs a live contention re-run proving the accepted event ends up with exactly one audit row.

New-regression sweep — nothing else found: all remaining raw PgPoolOptions writers at this head are accounted for (search pool is SELECT-only FTS, mesh-boot/channel-snapshot pools are test-only, push gateway documented out of scope), and the merge from main introduced no semantic interaction with the PR's files.

Quality: Minimalism 9/10; Elegance 9/10; Correctness 7/10 pending durable recovery from the newly expected lock-timeout error.