feat(db): configurable writer session timeouts (lock, idle-txn, state… - #7
Merged
Conversation
…ment) (#6229) ## 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. --- **Update Aug 28, 17:06 EDT:** Rebased onto `main` at `a3730784fc` and addressed the latest correctness review. - Ported the timeout policy onto the refactored `buzz-db::runtime` pool constructor and kept the shared env overlay for relay, admin, deletion, and audit writers. - Migration/schema-destruction connections now disable `lock_timeout` and `statement_timeout` for their intentional long wait/DDL path. This supersedes the earlier “Behavior change to note”: contended boot migrations wait for the current migration owner rather than crash-looping after five seconds. - The audit worker now preserves and retries the same entry on PostgreSQL `55P03` lock timeouts, using exponential backoff capped at one second. Other database errors retain the existing terminal error behavior, and retries emit `buzz_audit_log_lock_retries_total`. - Added CI-backed PostgreSQL regressions for writer GUC installation/migration exemption, audit-pool lock timeouts, and worker recovery. The worker regression holds the real audit advisory lock past `lock_timeout`, observes a retry, releases the lock, and proves the original entry is appended exactly once. Current verification supersedes the earlier testing notes: workspace Rust clippy passed with warnings denied; all nine infrastructure-free backend unit-test lanes passed; all three focused PostgreSQL regressions passed against PostgreSQL 17; formatting, diff checks, file-size guards, and desktop frontend checks passed. The Linux Blox workstation could not run the unrelated Tauri native lane because `glib-2.0` is absent, so that platform check is left to PR CI. --- **Update Aug 31, 11:09 EDT:** Rebased onto current `main` at `c3132c3ee9` and reran the requested audit-lock contention scenario on Blox at head `896c3fe9ed`. - `git range-diff` reports both PR commits unchanged by the rebase; the branch remains two commits and the worktree is clean. - `cargo fmt --all -- --check` and clippy with warnings denied passed for `buzz-db`, `buzz-relay`, `buzz-admin`, and `buzz-deletion`. - All three focused PostgreSQL 17 regressions passed: writer session timeout/migration exemption, audit writer timeout bounds, and audit worker recovery of the original entry exactly once. - Live protocol verification used a head-built relay and CLI, native PostgreSQL 17/Redis, `BUZZ_DB_LOCK_TIMEOUT_MS=300`, and an eight-second hold on the community audit advisory lock. The real message was accepted and persisted once while the lock was held; its audit-row count remained zero during contention while retries accumulated. After release, exactly one `event_created` audit row appeared and remained exactly one after an additional two-second duplicate check. The run recorded nine lock-timeout retries, zero audit failures, and event ID `7f8c4ffae28e78555fcf2d56396d6e6c01b3712e5411288dc79e9a54af9d9444`. Generated with Codex --------- Signed-off-by: Luke Tornquist <tornquist@squareup.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
…ment) (block#6229)
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:relationwaits 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_connecthook inbuzz-db, all env-tunable through the sameConfig::from_env → DbConfigpath as the existing pool-size knobs:BUZZ_DB_LOCK_TIMEOUT_MSlock_timeoutBUZZ_DB_STATEMENT_TIMEOUT_MSstatement_timeout0disables a timeout (Postgres semantics) and deliberately passes through the env parsing — unlike the pool-size knobs where0falls 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, orrelay.extraEnvin 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 setBUZZ_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 cleanmainin this environment (api::admin/api::media/mesh_demo — unrelated, pre-existing).0-passthrough / invalid-fallback for all three env vars.writer_pool_safety_hook_is_single_and_composedsource-shape test so the timeouts can't drift out of the singleafter_connecthook (SQLx replaces hooks — a second hook would silently disarm the floor guard).cargo fmt --checkandcargo clippy --all-targetsclean for the touched crates.Closest existing PR/issue: none found.
Update Aug 28, 17:06 EDT: Rebased onto
mainata3730784fcand addressed the latest correctness review.buzz-db::runtimepool constructor and kept the shared env overlay for relay, admin, deletion, and audit writers.lock_timeoutandstatement_timeoutfor their intentional long wait/DDL path. This supersedes the earlier “Behavior change to note”: contended boot migrations wait for the current migration owner rather than crash-looping after five seconds.55P03lock timeouts, using exponential backoff capped at one second. Other database errors retain the existing terminal error behavior, and retries emitbuzz_audit_log_lock_retries_total.lock_timeout, observes a retry, releases the lock, and proves the original entry is appended exactly once.Current verification supersedes the earlier testing notes: workspace Rust clippy passed with warnings denied; all nine infrastructure-free backend unit-test lanes passed; all three focused PostgreSQL regressions passed against PostgreSQL 17; formatting, diff checks, file-size guards, and desktop frontend checks passed. The Linux Blox workstation could not run the unrelated Tauri native lane because
glib-2.0is absent, so that platform check is left to PR CI.Update Aug 31, 11:09 EDT: Rebased onto current
mainatc3132c3ee9and reran the requested audit-lock contention scenario on Blox at head896c3fe9ed.git range-diffreports both PR commits unchanged by the rebase; the branch remains two commits and the worktree is clean.cargo fmt --all -- --checkand clippy with warnings denied passed forbuzz-db,buzz-relay,buzz-admin, andbuzz-deletion.BUZZ_DB_LOCK_TIMEOUT_MS=300, and an eight-second hold on the community audit advisory lock. The real message was accepted and persisted once while the lock was held; its audit-row count remained zero during contention while retries accumulated. After release, exactly oneevent_createdaudit row appeared and remained exactly one after an additional two-second duplicate check. The run recorded nine lock-timeout retries, zero audit failures, and event ID7f8c4ffae28e78555fcf2d56396d6e6c01b3712e5411288dc79e9a54af9d9444.Generated with Codex
Summary
Related issue
Testing