Skip to content

fix(pool): fence stale automatic-primary checkouts - #1494

Open
jokonski-indeed wants to merge 2 commits into
pgdogdev:mainfrom
jokonski-indeed:fix/stale-automatic-primary-checkouts
Open

fix(pool): fence stale automatic-primary checkouts#1494
jokonski-indeed wants to merge 2 commits into
pgdogdev:mainfrom
jokonski-indeed:fix/stale-automatic-primary-checkouts

Conversation

@jokonski-indeed

Copy link
Copy Markdown

This is an AI-assisted PR. Consider this an early draft for now and feel free to recommend significant changes.

I ran into an issue where PgDog was not routing queries to my role=auto instances after an AWS-induced RDS Aurora failover event. The failover was caused by excessive load on CPU & drained freeable memory. After the event, I've seen a lot of SQLSTATE 25006s - indicative of PgDog routing writer queries to what it is now a read replica. I believe PgDog should be adjusted to keep track of the latest & best evidence as to who's the primary.

With the fix applied, I was able to avoid these errors in reproduction scenarios, using an automated testing harness. I only tested this patch with the settings I use: role=auto, pooler_mode=session, query_parser=off, and having two app-side connection pools using the pgdog.role=replica/primary option.

I will follow up with timing measurements. This patch series successfully avoids 25006 errors, but I haven't yet measured the unavailability window from the client perspective.


Summary

During a managed PostgreSQL failover, PgDog can retain an automatic target's old Primary role after the backend that supplied that evidence disappears. When the database returns as a reader, a fresh session-pool checkout can then bind to that reader. The client receives SQLSTATE 25006 (cannot execute ... in a read-only transaction), and session pooling can keep returning the same bad backend until the frontend session is replaced.

This change makes automatic-primary selection require valid, non-replica role evidence and adds an independent checkout-time pg_is_in_recovery() fence before handing a backend to a write client. A backend found to be in recovery is closed, its automatic-role evidence is cleared, and selection retries another qualified target.

PgDog version

Reproduced on the incident-era PgDog revision (eff27d42) and on upstream commit 5e8b8858. The proposed change is based on v0.1.54 (7b40c0c2), whose additional commit changes only version metadata.

Description

The failure requires a narrow sequence:

  1. PgDog is using role = auto with session pooling and has identified a backend as the automatic primary.
  2. A managed PostgreSQL failover causes the old writer's connections to close while monitoring and topology information are changing.
  3. The old database instance returns or remains reachable as a standby during a short stale-role window.
  4. PgDog still has the target labelled Primary, or reuses that target before fresh writer evidence is available.
  5. New client sessions are checked out against that target and are pinned by session pooling.

The resulting error is produced after a query reaches the wrong backend. A query-level retry therefore does not guarantee a fresh backend when session pooling has pinned the stale binding.

The failure is easiest to observe with sustained concurrent client traffic, short requests that identify the backend role, and a failover triggered while the clients are reconnecting. The application authentication method is not the root cause. The tested setup used IAM backend authentication and query_parser = "off", but the relevant conditions are automatic role selection, session-pool rebinding, and a stale reader/writer topology window.

How it was observed

The test used one synchronized failover event for all matrix arms. Each arm ran the same client workload and recorded, for every checkout:

  • the PgDog/frontend connection identity;
  • the backend address and process identity;
  • pg_is_in_recovery() / writer-versus-reader role;
  • query errors, including genuine 25006 responses; and
  • whether the client later rebound to a healthy writer.

The failover was considered relevant only when direct database role checks and the client observations bracketed the same topology change. This distinguishes a stale-reader rebind from a separate case where an already-held backend is demoted after a query has already been sent.

The compact matrix compared five arms on the same failover event:

Arm Purpose
Incident-era vanilla Positive control matching the affected PgDog behavior
Upstream socket-liveness fix (#1318) Tests whether detecting closed idle sockets is sufficient
Upstream #1323 (b050a570) bundled change Starts unelected Auto targets as replicas and waits for a primary on write checkout; this arm does not isolate those two behaviors
Current upstream baseline at the time of testing (5e8b885). Current baseline
Proposed port Qualified-primary evidence plus checkout-time backend fencing

The final slim series was then run in five synchronized events of the same matrix. The counts below are totals across those completed events; the proposed arm had 60 paired clients per event. The socket-liveness comparator (#1318) was also present in the matrix, but its partial-run total is omitted from this concise summary.

Arm Stale reader binds Genuine 25006 Clients proven wedged within the observation window Writer binds
Incident-era vanilla 31 334 12
Upstream #1323 (b050a570) bundled change 2 55 2
Current upstream 1 28 1
Proposed two-commit series 0 0 0 300/300

These are strict observation-window totals and descriptive developmental evidence, not a general safety proof or a formal treatment-efficacy claim. The formal recovery deadline right-censored the completed event outcomes. The preceding ten-event broad prototype showed the same directional contrast, but those results are retained as developmental evidence only.

Root cause

Automatic role detection and checkout treated cached role state as sufficient to select a writer. Monitoring could lose, clear, or fail to refresh the LSN/recovery evidence without revoking the target's cached Primary role. A later checkout therefore had no independent guard against receiving a backend that was now in recovery.

The important distinction is between:

  • a backend that was already handed to a client and is later demoted; and
  • a new checkout that can still be prevented from binding to a known standby.

This PR addresses the second, incident-relevant path. It does not claim to migrate an in-flight PostgreSQL session or replay an ambiguous query.

Fix

The series is intentionally split into two reviewable commits:

  1. Automatic-primary role selection now requires valid, non-replica role evidence. Unavailable, timed-out, malformed, or failed monitoring evidence revokes stale automatic-primary qualification.
  2. Before an automatic-primary backend is returned, PgDog checks pg_is_in_recovery() within the existing checkout timeout. A standby is force-closed, its cached automatic-role evidence is cleared, and PgDog retries another qualified primary. Probe failures fail closed rather than handing the client an unverified writer.

The checkout probe is the correctness barrier. Evidence revocation prevents the load balancer from repeatedly selecting the same stale target while the topology converges.

Testing

Local validation on the final slim series:

  • formatting and diff checks: passed;
  • workspace check: passed;
  • all-workspace, all-target Clippy with warnings denied: passed;
  • seven focused automatic-role/evidence/checkout regressions: passed (the broader 19-test focused set also passed);
  • full disposable-PostgreSQL suite: 2,384 passed, 7 skipped;
  • release build and IAM/session/parser-off configuration check: passed;
  • offline listener, OpenMetrics, and clean-shutdown smoke test: passed.

Synchronized failover rerun: five completed events in the same five-arm matrix; the partial, non-claimable results are summarized above. Raw artifacts and provenance are retained by the submitter and are available to reproduce the summary if maintainers want the full diagnostic detail.

Configuration

No new configuration is required. The behavior applies to automatic-role targets. Static primary and replica configurations retain their existing semantics.

Scope and limitations

This series targets fresh stale-reader checkouts after a topology change. It does not make arbitrary in-flight PostgreSQL operations replayable, and it does not transfer transaction state, cursors, temporary objects, prepared statements, or advisory locks between servers. A query whose outcome is ambiguous when a backend connection dies still requires the client/application's existing error-handling policy.

The checkout probe adds a bounded round trip for automatic-primary checkouts. That is an intentional availability/correctness tradeoff during failover: PgDog may return a checkout error while writer identity cannot be verified, rather than risk sending a write to a standby.

@CLAassistant

CLAassistant commented Sep 3, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@levkk

levkk commented Sep 3, 2026

Copy link
Copy Markdown
Collaborator

Unfortunately I don't understand the root cause. If you could spend some time explaining it with pointers to original code that works incorrectly, that would be helpful.

@jokonski-indeed

Copy link
Copy Markdown
Author

I logged a bunch of stuff from my testing harness so I can share the observations.

For the upstream 5e8b8858 version, following an AWS-induced failover of the overloaded writer:

09:06:39.062  server cleanup failed: connection closed by peer [former writer A]
09:06:39.805  direct SQL observer: B writable, pg_is_in_recovery() = false
09:06:41.000  SHOW REPLICATION: A role=primary,
              lsn_age_ms=10333, pg_is_in_recovery=f
09:06:41.198  upstream client checkout starts
09:06:41.712  PgDog: new primary chosen: B
09:06:46.079  checkout completes: backend=A,
              backend_start=09:06:44.799,
              pg_is_in_recovery()=true, role=reader
09:06:46.247  SQLSTATE 25006:
              cannot execute INSERT in a read-only transaction

The backend connection was created ~3 seconds after PgDog logged that B was the new primary, but it still went to the pre-failover primary A.

I asked my AI agent to explain the causal chain through PgDog's code:

On the tested upstream revision, an LSN query error or timeout returned None without revoking the previous observation:

async fn run_query(&self, conn: &mut Server, query: &str) -> Option<DataRow> {
match safe_timeout(self.pool.config().lsn_check_timeout, conn.fetch_all(query)).await {
Ok(Ok(rows)) => rows.into_iter().next(),
Ok(Err(err)) => {
error!("lsn monitor query error: {} [{}]", err, self.pool.addr());
None
}
Err(_) => {
error!("lsn monitor query timeout [{}]", self.pool.addr());
None
}
}

That retained Aurora observation still passed valid() regardless of its age:

impl LsnStats {
/// Stats contain real data.
pub fn valid(&self) -> bool {
self.aurora || self.lsn.lsn > 0
}

Role detection could consequently continue selecting the cached non-replica observation:

let primary = targets
.iter()
.position(|target| !target.0.replica && target.0.valid());
if let Some(primary) = primary {
promoted = targets[primary].1.set_role(Role::Primary);
if promoted {
warn!("new primary chosen: {}", targets[primary].1.pool.addr());
}
// Demote everyone else to replicas.
targets
.iter()
.enumerate()
.filter(|(i, _)| *i != primary)
.for_each(|(_, target)| {
target.1.set_role(Role::Replica);
});
} else if targets.iter().all(|target| target.0.valid()) {
// All targets are replicas until we get a primary.
targets.iter().for_each(|target| {
target.1.set_role(Role::Replica);
});

Finally, get_primary() selected that target and awaited pool.get() without checking either the target again or the backend actually returned:

async fn get_primary_internal(&self, request: &Request) -> Result<Guard, Error> {
self.wait_primary().await?;
self.primary_target()
.ok_or(Error::NoPrimary)?
.pool
.get(request)
.await
}

That last point explains the timing above: even though role detection corrected itself while pool.get() was in progress, the checkout still returned a newly established connection to A.


For the patched version built on this branch, my logs from the same event look like this:

09:06:41.207  patched client checkout starts
09:06:41.707  PgDog: new primary chosen: B
09:06:46.403  checkout fails closed; no backend is returned
09:06:46.918  client reconnect starts
09:06:51.039  checkout completes: backend=B,
              pg_is_in_recovery()=false, role=writer
09:06:51.503  write succeeds

The proposed path revalidates after pool.get() and queries pg_is_in_recovery() on the exact backend before returning it:

async fn checkout_target(
&self,
target: &Target,
request: &Request,
primary_required: bool,
) -> Result<Guard, Error> {
let automatic_primary_before = target.is_automatic_primary();
if (primary_required || automatic_primary_before) && !target.is_qualified_primary() {
return Err(Error::NoPrimary);
}
let guard = target.pool.get(request).await?;
if primary_required || automatic_primary_before || target.is_automatic_primary() {
self.check_automatic_primary_guard(target, guard).await
} else {
Ok(guard)
}
}
async fn check_automatic_primary_guard(
&self,
target: &Target,
guard: Guard,
) -> Result<Guard, Error> {
let mut guard = guard;
if !target.is_qualified_primary() {
guard.stats_mut().state(State::ForceClose);
return Err(Error::NoPrimary);
}
if target.pool.addr().configured_role == Role::Auto {
match guard
.check_automatic_primary_backend(target.pool.config().lsn_check_timeout)
.await
{
Ok(true) => {}
Ok(false) => {
warn!(
"automatic primary checkout rejected: backend {} is in recovery [{}]",
guard.id(),
guard.addr(),
);
self.reject_automatic_primary(target, &mut guard);
return Err(Error::NoPrimary);
}
Err(err) => {
self.reject_automatic_primary(target, &mut guard);
return Err(
if matches!(err, crate::backend::Error::AutomaticPrimaryCheckTimeout) {
Error::CheckoutTimeout
} else {
Error::ServerError
},
);
}

pub(super) async fn check_automatic_primary_backend(
&mut self,
timeout: Duration,
) -> Result<bool, Error> {
let replica: bool = safe_timeout(
timeout,
self.fetch_all::<DataRow>("SELECT pg_is_in_recovery()"),
)
.await
.unwrap_or(Err(Error::AutomaticPrimaryCheckTimeout))
.and_then(|rows| {
rows.into_iter()
.next()
.ok_or(Error::AutomaticPrimaryCheckInvalidResponse)
})
.and_then(|row| {
row.get(0, Format::Text)
.ok_or(Error::AutomaticPrimaryCheckInvalidResponse)
})
.inspect_err(|_| self.stats.state(State::ForceClose))?;
Ok(!replica)

Thus the behavioral change in this event was: a short, retryable connection failure followed by a writer checkout, instead of a successful checkout permanently bound to a reader and returning repeated 25006 errors.

The runtime records directly establish the stale PgDog view, the checkout/election ordering, the fresh backend identity, and 25006. They do not identify which particular monitor-query failure preserved the old observation; that narrower part follows from the pre-patch control flow above.

This patch is making a new trade off because, at least in the session mode I used, we will stall new primary-routed connections until we can prove it's the primary with pg_is_in_recovery(). So we get accurate routing at the expense of more connection latency and some checkouts timing out. Realistically, I think it only matters when the server is overloaded and that it's the right trade off to make.

Big caveat - I haven't looked at what this patch would mean for transaction mode.

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.

3 participants