fix(server): retry ingest writes on Patroni read-only failover (JEF-496)#125
Merged
thejefflarson merged 1 commit intoJul 23, 2026
Conversation
At a Patroni failover the sqlx pool keeps live connections to the old leader, now demoted to a read-only replica, so batched ingest INSERTs hit a read-only backend (SQLSTATE 25006) and were dropped (DROP_INSERT++) rather than retried. Wrap every batched write (insert_spans/insert_logs and the metric number/ histogram flushes) in a shared write_with_failover_retry helper: on a 25006 (read_only_sql_transaction) or 57P01 (admin_shutdown) error it evicts the pooled connection (close_on_drop, so the pool can't hand the same demoted backend back) and retries on a fresh connection — bounded to 3 attempts with a short 200ms/600ms backoff to let -master DNS re-resolve to the new leader. Only these two SQLSTATEs retry; any other error still falls through to the per-row fallback + DROP_INSERT accounting, so a bad row is never masked. A row counts as dropped only after retries are exhausted, so a transient failover that recovers drops nothing. Defense in depth in db.rs: set the pool max_lifetime to 5m and keep test_before_acquire on so connections rotate off a demoted node promptly even absent a write. Tests (real Postgres): a forced 25006 (SET default_transaction_read_only) recovers on retry with zero drops; a non-25006 error (22012) surfaces immediately without retry. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JbrmfzHsTMMzPaSrUkZgWo
thejefflarson
deleted the
thejefflarson/jef-496-ingest-writes-fail-during-patroni-failover-read-only
branch
July 23, 2026 04:56
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.
Closes JEF-496.
Problem
At a Patroni failover (watcher's own telemetry, 2026-07-22 06:41) dozens of
insert log failed: cannot execute INSERT in a read-only transactionWARNs fired within ~1s and those inserts were dropped (DROP_INSERT++), not retried. The sqlx pool holds live connections to the old primary; on failover that node is demoted to a read-only replica, but the pool keeps handing out its existing connections, so writes hit a read-only backend → SQLSTATE25006. It self-heals only once connections recycle and re-resolve the-masterDNS to the new leader.Fix
otlp.rs). A sharedwrite_with_failover_retryhelper wraps every batched write —insert_spans/insert_logs(viawrite_chunked) and the metricinsert_numbers/insert_histogramsflushes. On a25006(read_only_sql_transaction) or57P01(admin_shutdown) error it evicts the pooled connection (close_on_drop, so the pool can't hand the same demoted backend back), backs off, and retries on a fresh connection. Bounded to 3 attempts with a 200ms → 600ms backoff (capped ≤ 1s) — long enough for-masterDNS to re-resolve to the new leader, short enough not to stall ingest on a transient blip. Only those two SQLSTATEs retry; every other DB error returns immediately.db.rs). Poolmax_lifetime = 5m+test_before_acquire(true)(sqlx's default, now explicit) so connections rotate off a demoted node promptly even for idle connections / even absent a write. 5m is long enough not to churn under steady ingest yet bounds post-failover staleness to minutes.test_before_acquirecatches a closed backend (57P01);max_lifetimerotates a still-open read-only one; the write retry covers the window in between.Layering / accounting
25006that recovers on retry drops nothing.How I tested the 25006 path
Two DB-backed unit tests in
otlp.rs(real Postgres, skip cleanly withoutDATABASE_URL):failover_retry_recovers_from_read_only— forces a genuine25006by settingdefault_transaction_read_only = onon the first acquired connection (mirrors the demoted-primary backend); the helper evicts it and the retry on a fresh connection succeeds. AssertsOkand exactly 2 attempts (fail once → recover), zero drops.failover_retry_does_not_retry_other_errors— a non-25006error (22012divide-by-zero) surfaces immediately, classified non-failover, tried exactly once.Gates (against local Postgres):
cargo fmt --check,cargo clippy --all-targets(warnings-as-errors),cargo test --locked— all green (56 integration + 46 lib unit tests).DATABASE_URL /
-mastertarget — human follow-upThe server composes
DATABASE_URLfrom env; the host is supplied by the Helm chart in the separate../clusterGitOps repo (not in this repo), so I could not verify it here. Please confirm the chart points the pool at the Patroni-master(leader) service, not a headless all-members endpoint. If it already targets-master, no change needed; the retry + recycle above handle the failover window regardless. I did not edit the chart.🤖 Generated with Claude Code
https://claude.ai/code/session_01JbrmfzHsTMMzPaSrUkZgWo