Skip to content

fix(server): retry ingest writes on Patroni read-only failover (JEF-496)#125

Merged
thejefflarson merged 1 commit into
mainfrom
thejefflarson/jef-496-ingest-writes-fail-during-patroni-failover-read-only
Jul 23, 2026
Merged

fix(server): retry ingest writes on Patroni read-only failover (JEF-496)#125
thejefflarson merged 1 commit into
mainfrom
thejefflarson/jef-496-ingest-writes-fail-during-patroni-failover-read-only

Conversation

@thejefflarson

Copy link
Copy Markdown
Owner

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 transaction WARNs 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 → SQLSTATE 25006. It self-heals only once connections recycle and re-resolve the -master DNS to the new leader.

Fix

  1. Targeted retry (otlp.rs). A shared write_with_failover_retry helper wraps every batched write — insert_spans / insert_logs (via write_chunked) and the metric insert_numbers / insert_histograms flushes. 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), backs off, and retries on a fresh connection. Bounded to 3 attempts with a 200ms → 600ms backoff (capped ≤ 1s) — long enough for -master DNS 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.
  2. Faster recycle (db.rs). Pool max_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_acquire catches a closed backend (57P01); max_lifetime rotates a still-open read-only one; the write retry covers the window in between.

Layering / accounting

  • The retry wraps the whole-batch execute and each per-row fallback execute, so a read-only error on either path retries rather than drops.
  • DROP_INSERT accounting stays correct: a row counts as dropped only after retries are exhausted or on a non-retryable error. A transient 25006 that recovers on retry drops nothing.
  • If a chunk still fails with a failover error after retries, per-row isolation would hit the same read-only wall, so the chunk is dropped and counted once rather than multiplying the retry storm. A non-failover chunk error still isolates per-row (so one bad row can't sink the batch) exactly as before.

How I tested the 25006 path

Two DB-backed unit tests in otlp.rs (real Postgres, skip cleanly without DATABASE_URL):

  • failover_retry_recovers_from_read_only — forces a genuine 25006 by setting default_transaction_read_only = on on the first acquired connection (mirrors the demoted-primary backend); the helper evicts it and the retry on a fresh connection succeeds. Asserts Ok and exactly 2 attempts (fail once → recover), zero drops.
  • failover_retry_does_not_retry_other_errors — a non-25006 error (22012 divide-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 / -master target — human follow-up

The server composes DATABASE_URL from env; the host is supplied by the Helm chart in the separate ../cluster GitOps 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

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
thejefflarson merged commit d4354a2 into main Jul 23, 2026
4 checks passed
@thejefflarson
thejefflarson deleted the thejefflarson/jef-496-ingest-writes-fail-during-patroni-failover-read-only branch July 23, 2026 04:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant