Skip to content

pgxpool: Acquire destroys a healthy idle connection when the caller's context is cancelled during the liveness ping #2680

Closed

Description

@KlausVii

Drafted by Claude (Anthropic) on behalf of, and reviewed by, @KlausVii.

Describe the bug

Pool.Acquire pings a connection that has been idle for more than 1 s (default ShouldPing). The ping uses the caller's context (pingCtx := ctx, optionally wrapped in PingTimeout), and on any ping error it calls res.Destroy():

shouldPingParams := ShouldPingParams{Conn: cr.conn, IdleDuration: res.IdleDuration()}
if p.shouldPing(ctx, shouldPingParams) {
    err := func() error {
        pingCtx := ctx
        ...
        return cr.conn.Ping(pingCtx)
    }()
    if err != nil {
        res.Destroy()
        continue
    }
}

If the caller cancels its context while that ping is in flight, the context watcher breaks the socket, the ping fails, and the pool destroys a connection that was healthy. The same happens when the context is cancelled after the pool hands out the resource but before the ping starts: Ping then fails immediately, and the untouched connection is destroyed. The caller gets context.Canceled, which is correct. But the pool loses a connection it had no reason to lose, and none of the Stat() destroy counters record it (MaxLifetimeDestroyCount and MaxIdleDestroyCount stay 0). The only trace is NewConnsCount going up.

This differs from cancelling a query, where terminating the connection is expected. Here the cancelled work is the pool's own health check of an idle connection that the caller never used.

It matters for code that issues parallel short queries and cancels the losers once one branch answers. In an authorization server built on pgxpool (OpenFGA), this turned into a steady stream of reconnects:

  • new connections went from ~4.4k to ~225k per day;
  • cancelled acquires went from 0 to ~347k per day;
  • both destroy counters stayed at 0;
  • p99 latency doubled.

To Reproduce

The program below adds a 20 ms server→client delay through an in-process TCP proxy, so a ping takes at least 20 ms, standing in for a network round trip. In each round it:

  1. lets the single connection idle for 1.1 s;
  2. calls Acquire with a context that is cancelled 5 ms later, i.e. during the ping;
  3. acquires again with a live context and compares the backend PID.
go run -race . -dsn 'postgres://postgres:repro@127.0.0.1:PORT/postgres?sslmode=disable'
// Repro: pgxpool destroys a healthy idle connection when the caller's context is
// cancelled while Acquire is running its liveness ping.
//
// usage: go run . -dsn 'postgres://...' [-delay 20ms] [-cancel-after 5ms] [-rounds 5] [-detached]
package main

import (
	"context"
	"errors"
	"flag"
	"fmt"
	"io"
	"log"
	"net"
	"net/url"
	"time"

	"github.com/jackc/pgx/v5"
	"github.com/jackc/pgx/v5/pgxpool"
)

// delayProxy forwards TCP to target, delaying every server->client chunk by d,
// so a ping takes at least d (stands in for a network round trip).
func delayProxy(target string, d time.Duration) string {
	ln, err := net.Listen("tcp", "127.0.0.1:0")
	if err != nil {
		log.Fatal(err)
	}
	go func() {
		for {
			c, err := ln.Accept()
			if err != nil {
				return
			}
			go func() {
				s, err := net.Dial("tcp", target)
				if err != nil {
					c.Close()
					return
				}
				go func() { io.Copy(s, c); s.Close() }()
				buf := make([]byte, 32<<10)
				for {
					n, err := s.Read(buf)
					if n > 0 {
						time.Sleep(d)
						if _, werr := c.Write(buf[:n]); werr != nil {
							break
						}
					}
					if err != nil {
						break
					}
				}
				c.Close()
			}()
		}
	}()
	return ln.Addr().String()
}

func main() {
	dsn := flag.String("dsn", "", "postgres connection string")
	delay := flag.Duration("delay", 20*time.Millisecond, "added server->client latency")
	cancelAfter := flag.Duration("cancel-after", 5*time.Millisecond, "cancel the acquirer's context after this long")
	rounds := flag.Int("rounds", 5, "rounds")
	detached := flag.Bool("detached", false, "use a ShouldPing hook that pings under context.WithoutCancel (proposed behaviour)")
	flag.Parse()

	u, err := url.Parse(*dsn)
	if err != nil {
		log.Fatal(err)
	}
	u.Host = delayProxy(u.Host, *delay)
	cfg, err := pgxpool.ParseConfig(u.String())
	if err != nil {
		log.Fatal(err)
	}
	cfg.MaxConns, cfg.MinConns = 1, 0
	if *detached {
		cfg.ShouldPing = func(ctx context.Context, p pgxpool.ShouldPingParams) bool {
			if p.IdleDuration <= time.Second { // pgx's default rule
				return false
			}
			pctx, cancel := context.WithTimeout(context.WithoutCancel(ctx), 2*time.Second)
			defer cancel()
			return p.Conn.Ping(pctx) != nil // only a failed ping lets pgx ping again and destroy
		}
		// If the caller left during the detached ping, hand the healthy connection back
		// (ok=true + error => pgx releases it) and return the caller's context error.
		cfg.PrepareConn = func(ctx context.Context, _ *pgx.Conn) (bool, error) { return true, ctx.Err() }
	}
	ctx := context.Background()
	pool, err := pgxpool.NewWithConfig(ctx, cfg)
	if err != nil {
		log.Fatal(err)
	}
	defer pool.Close()

	backendPID := func() uint32 {
		c, err := pool.Acquire(ctx)
		if err != nil {
			log.Fatal(err)
		}
		defer c.Release()
		return c.Conn().PgConn().PID()
	}
	fmt.Printf("pgx config: delay=%s cancel-after=%s detached=%v\n", *delay, *cancelAfter, *detached)
	pid := backendPID()
	for i := 1; i <= *rounds; i++ {
		time.Sleep(1100 * time.Millisecond) // idle > 1s, so the next Acquire pings
		cctx, cancel := context.WithCancel(ctx)
		time.AfterFunc(*cancelAfter, cancel)
		c, aerr := pool.Acquire(cctx)
		if c != nil {
			c.Release()
		}
		cancel()
		if aerr == nil || !errors.Is(aerr, context.Canceled) {
			fmt.Printf("round %d: acquire did not fail with context.Canceled: %v\n", i, aerr)
		}
		next := backendPID()
		st := pool.Stat()
		fmt.Printf("round %d: cancelled acquire -> %v | backend pid %d -> %d (same connection: %v) | NewConnsCount=%d MaxLifetimeDestroy=%d MaxIdleDestroy=%d\n",
			i, aerr, pid, next, pid == next, st.NewConnsCount(), st.MaxLifetimeDestroyCount(), st.MaxIdleDestroyCount())
		pid = next
	}
}

Expected behavior

Acquire returns context.Canceled, and the healthy idle connection stays in the pool: same backend PID, NewConnsCount unchanged.

Actual behavior

pgx v5.11.0 (v5.10.0 is identical; full output for both versions, the control and the hook variant is in repro.log):

== pgx v5.11.0, caller cancelled mid-ping (default config)
pgx config: delay=20ms cancel-after=5ms detached=false
round 1: cancelled acquire -> context canceled | backend pid 101 -> 103 (same connection: false) | NewConnsCount=2 MaxLifetimeDestroy=0 MaxIdleDestroy=0
round 2: cancelled acquire -> context canceled | backend pid 103 -> 105 (same connection: false) | NewConnsCount=3 MaxLifetimeDestroy=0 MaxIdleDestroy=0
round 3: cancelled acquire -> context canceled | backend pid 105 -> 107 (same connection: false) | NewConnsCount=4 MaxLifetimeDestroy=0 MaxIdleDestroy=0
round 4: cancelled acquire -> context canceled | backend pid 107 -> 109 (same connection: false) | NewConnsCount=5 MaxLifetimeDestroy=0 MaxIdleDestroy=0
round 5: cancelled acquire -> context canceled | backend pid 109 -> 111 (same connection: false) | NewConnsCount=6 MaxLifetimeDestroy=0 MaxIdleDestroy=0

Every cancelled acquire costs a new connection. With -cancel-after 1s (no cancellation during the ping), the PID stays the same.

The repro's -detached mode shows the behaviour we expected. It sets two hooks:

  • a ShouldPing hook that pings under context.WithoutCancel(ctx) (bounded by a timeout) and returns false unless that ping failed;
  • PrepareConn returning (true, ctx.Err()), so a caller that left is handed its context error and the connection is released, not destroyed.

With those hooks the PID stays the same and NewConnsCount stays at 1. We now run this as a workaround.

Suggested fix (any of these):

  1. Ping under context.WithoutCancel(ctx) bounded by PingTimeout (or a default). If the caller's context is done after a successful ping, Release() the resource and return ctx.Err().
  2. At least: when the ping error comes from the caller's context (ctx.Err() != nil), check ctx.Err() before pinging. Then a context that is already cancelled never reaches Ping, and the already-cancelled path stops destroying untouched connections. The mid-ping case still needs (1), because the context watcher has already broken the socket.
  3. Count these destroys in Stat(), so they are at least visible.

Version

  • Go: go1.26.0 darwin/arm64
  • PostgreSQL: 17.11 on aarch64-unknown-linux-musl (postgres:17-alpine)
  • pgx: v5.10.0 and v5.11.0 (same behaviour); master 252aff14 has the same code

Additional context

The cancellation is real in our case: OpenFGA v1.19.0 started acquiring under the request context (openfga/openfga#3255), which is a correct change on its own, and a union that resolves early cancels its sibling branches while they are pinging.

Activity

  1. added a commit that references this issue on Oct 16, 2026
    11a9732
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions