fix(pg-pool): maintain min connections after maxUses/error retirement#3696
fix(pg-pool): maintain min connections after maxUses/error retirement#3696slukes wants to merge 1 commit into
Conversation
The `min` pool option was not properly maintained: when connections were retired due to `maxUses`, `maxLifetimeSeconds`, or background errors, the pool shrank below `min` and never refilled unless a new query arrived. Two root causes: 1. `_pulseQueue` (called after retirement) only creates connections when `_pendingQueue` is non-empty, so an idle pool never refills. 2. The idle-error path in `makeIdleListener` called `_remove(client)` without a callback, so `_pulseQueue` was not called at all, leaving pending requests stuck indefinitely. Fix: - Add `_ensureMinimum()` that synchronously schedules `newClient` calls for each connection needed to reach `options.min`. It is called from `_remove` immediately after the client is spliced from `_clients`, so every removal path (maxUses, maxLifetimeSeconds, errors) triggers a refill. - Pass `pool._pulseQueue` as the callback to `_remove` inside `makeIdleListener` so that queued `connect()` calls are served after an idle client errors out. - Update the `maxLifetimeSeconds` sizing test whose assertion assumed the old (non-refilling) behaviour; it now verifies that the pool returns to `min` after expiry. - Add `test/min-connections.js` with mock-client tests covering all three scenarios: maxUses retirement, idle-error refill, and pending-request service after idle-error. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
That’s what it’s documented to do (https://node-postgres.com/apis/pool):
|
Yes, my apologies. Would you be open to a PR that changes that behaviour in an opt in way when combined with maxUses? To explain why I would want to do this:
|
Problem
The `min` pool option does not maintain a minimum number of live connections. It only prevents the idle timeout from evicting connections when at or below `min`. When connections are retired for other reasons — `maxUses`, `maxLifetimeSeconds`, or background errors — the pool shrinks below `min` and never refills unless a new query arrives.
Concretely, with `min: 15, max: 15, maxUses: `, a low-traffic window causes the pool to drain nearly to zero as each connection exhausts its use count and is removed without replacement.
There are two root causes:
1. `_pulseQueue` only refills for queued requests
`_pulseQueue` (called after every retirement via `_remove`'s callback) only creates new connections when `_pendingQueue` is non-empty. An idle pool with no waiting callers therefore never gets a refill.
2. `makeIdleListener` does not call `_pulseQueue`
When an idle client emits a background error, `makeIdleListener` called:
```js
pool._remove(client) // no callback — _pulseQueue is never called
```
This meant:
Fix
`_ensureMinimum()` — a new method that synchronously schedules `newClient` calls for however many connections are needed to reach `options.min`:
```js
_ensureMinimum() {
if (this.ending || this.ended) return
const needed = this.options.min - this._clients.length
for (let i = 0; i < needed; i++) {
this.newClient(new PendingItem((err, client, clientRelease) => {
if (!err) clientRelease()
}))
}
}
```
Because `newClient` pushes to `_clients` immediately (before `connect()` resolves), the loop naturally caps at exactly `min` even when called multiple times concurrently.
Called from `_remove` immediately after the retiring client is spliced from `_clients`, covering every removal path: `maxUses`, `maxLifetimeSeconds`, and errors.
`makeIdleListener` now passes `_pulseQueue` as the `_remove` callback, so queued `connect()` callers are unblocked after an idle client errors out.
Tests
`test/min-connections.js` — new file using a mock `Client` (no real PG needed) with four scenarios:
`test/sizing.js` — the existing `maxLifetimeSeconds` test that asserted `idleCount === 0` after expiry is updated: the correct new behaviour is that the pool refills to `min`, so the test now asserts `totalCount === 1` and `idleCount === 1` after the replacement connects.
Behaviour notes