Skip to content

Commit d7d8279

Browse files
jackcclaude
andcommitted
Identify pooled connections by pointer instead of backend PID
PostgreSQL can assign a terminated backend's PID to a new backend, so a PID does not uniquely identify a connection over time. PID reuse is common on the Windows CI runners, where it made these tests fail: TestPoolAcquireChecksIdleConns: []uint32{0xd48, 0x23bc, 0x1738} should not contain 0x1738 TestPoolAfterRelease: expected: 5, actual: 4 Compare *pgx.Conn pointers instead. TestPoolAfterRelease keeps them in a map, which also retains the destroyed connections so their addresses cannot be reused either. TestPoolAcquireChecksIdleConns still needs the PIDs to call pg_terminate_backend(). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 4aafe91 commit d7d8279

2 files changed

Lines changed: 31 additions & 19 deletions

File tree

pgxpool/pool_test.go

Lines changed: 20 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -176,8 +176,10 @@ func TestPoolAcquireChecksIdleConns(t *testing.T) {
176176
require.EqualValues(t, 3, pool.Stat().TotalConns())
177177

178178
var pids []uint32
179+
var originalConns []*pgx.Conn
179180
for _, c := range conns {
180181
pids = append(pids, c.Conn().PgConn().PID())
182+
originalConns = append(originalConns, c.Conn())
181183
c.Release()
182184
}
183185

@@ -199,10 +201,14 @@ func TestPoolAcquireChecksIdleConns(t *testing.T) {
199201
c, err := pool.Acquire(ctx)
200202
require.NoError(t, err)
201203

202-
cPID := c.Conn().PgConn().PID()
204+
newConn := c.Conn()
203205
c.Release()
204206

205-
require.NotContains(t, pids, cPID)
207+
// Compare connections by identity instead of by backend PID. PostgreSQL can assign a terminated backend's PID to a
208+
// new backend, which made this test flaky in CI.
209+
for _, originalConn := range originalConns {
210+
require.NotSame(t, originalConn, newConn)
211+
}
206212
}
207213

208214
func TestPoolAcquireChecksIdleConnsWithShouldPing(t *testing.T) {
@@ -477,17 +483,20 @@ func TestPoolAfterRelease(t *testing.T) {
477483
require.NoError(t, err)
478484
defer db.Close()
479485

480-
connPIDs := map[uint32]struct{}{}
486+
// Count distinct connections by identity instead of by backend PID. PostgreSQL can assign a terminated backend's
487+
// PID to a new backend, which made this test flaky in CI. Retaining the connections in the map also prevents them
488+
// from being garbage collected, so a destroyed connection's address cannot be reused either.
489+
distinctConns := map[*pgx.Conn]struct{}{}
481490

482491
for range 10 {
483492
conn, err := db.Acquire(ctx)
484493
assert.NoError(t, err)
485-
connPIDs[conn.Conn().PgConn().PID()] = struct{}{}
494+
distinctConns[conn.Conn()] = struct{}{}
486495
conn.Release()
487496
waitForReleaseToComplete()
488497
}
489498

490-
assert.EqualValues(t, 5, len(connPIDs))
499+
assert.EqualValues(t, 5, len(distinctConns))
491500
}
492501

493502
func TestPoolBeforeClose(t *testing.T) {
@@ -1405,13 +1414,10 @@ func TestPoolAcquirePingTimeout(t *testing.T) {
14051414
config.PingTimeout = 200 * time.Millisecond
14061415
config.ConnConfig.DialFunc = newDelayProxyDialFunc(500 * time.Millisecond)
14071416

1408-
var conID *uint32
1409-
// Only ping the connection with the original PID to force creation of a new connection
1417+
var originalConn *pgx.Conn
1418+
// Only ping the original connection to force creation of a new connection
14101419
config.ShouldPing = func(_ context.Context, params pgxpool.ShouldPingParams) bool {
1411-
if conID != nil && params.Conn.PgConn().PID() == *conID {
1412-
return true
1413-
}
1414-
return false
1420+
return originalConn != nil && params.Conn == originalConn
14151421
}
14161422

14171423
// Limit to a single connection to ensure the same connection is reused
@@ -1425,21 +1431,20 @@ func TestPoolAcquirePingTimeout(t *testing.T) {
14251431
c, err := pool.Acquire(ctx)
14261432
require.NoError(t, err)
14271433
require.EqualValues(t, 1, pool.Stat().TotalConns())
1428-
originalPID := c.Conn().PgConn().PID()
1429-
conID = &originalPID
1434+
originalConn = c.Conn()
14301435

14311436
c.Release()
14321437
require.EqualValues(t, 1, pool.Stat().TotalConns())
14331438

14341439
c, err = pool.Acquire(ctx)
14351440
require.NoError(t, err)
14361441
require.EqualValues(t, 1, pool.Stat().TotalConns())
1437-
newPID := c.Conn().PgConn().PID()
1442+
newConn := c.Conn()
14381443

14391444
c.Release()
14401445

14411446
require.EqualValues(t, 1, pool.Stat().TotalConns())
14421447
assert.Nil(t, ctx.Err())
1443-
assert.NotEqualValues(t, originalPID, newPID,
1448+
assert.NotSame(t, originalConn, newConn,
14441449
"Expected new connection due to ping timeout, but got same connection")
14451450
}

stdlib/sql_test.go

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1328,9 +1328,12 @@ func TestCheckIdleConn(t *testing.T) {
13281328
require.EqualValues(t, 3, db.Stats().OpenConnections)
13291329

13301330
var pids []uint32
1331+
var originalConns []*pgx.Conn
13311332
for _, c := range conns {
13321333
err := c.Raw(func(driverConn any) error {
1333-
pids = append(pids, driverConn.(*stdlib.Conn).Conn().PgConn().PID())
1334+
pgxConn := driverConn.(*stdlib.Conn).Conn()
1335+
pids = append(pids, pgxConn.PgConn().PID())
1336+
originalConns = append(originalConns, pgxConn)
13341337
return nil
13351338
})
13361339
require.NoError(t, err)
@@ -1359,16 +1362,20 @@ func TestCheckIdleConn(t *testing.T) {
13591362
c, err := db.Conn(context.Background())
13601363
require.NoError(t, err)
13611364

1362-
var cPID uint32
1365+
var newConn *pgx.Conn
13631366
err = c.Raw(func(driverConn any) error {
1364-
cPID = driverConn.(*stdlib.Conn).Conn().PgConn().PID()
1367+
newConn = driverConn.(*stdlib.Conn).Conn()
13651368
return nil
13661369
})
13671370
require.NoError(t, err)
13681371
err = c.Close()
13691372
require.NoError(t, err)
13701373

1371-
require.NotContains(t, pids, cPID)
1374+
// Compare connections by identity instead of by backend PID. PostgreSQL can assign a terminated backend's PID to a
1375+
// new backend, which made the equivalent pgxpool test flaky in CI.
1376+
for _, originalConn := range originalConns {
1377+
require.NotSame(t, originalConn, newConn)
1378+
}
13721379
}
13731380

13741381
func TestOptionShouldPing_HookCalledOnReuse(t *testing.T) {

0 commit comments

Comments
 (0)