Conversation
Keep reserve waiters queued when a connection attempt starts, including reconnects after a server-side close. Clear the closed session query state so a stale fatal error cannot reject a new reserve. Add real-server regressions for queued reserves and terminated in-flight queries.
colll78
added a commit
to Anastasia-Labs/midgard
that referenced
this pull request
Sep 29, 2026
postgres.js 3.4.9 can lose a queued reserve() request. When a pooled connection closes or reconnects while requests wait for a connection, the pool hands the next waiter to the reconnecting connection and drops it once that connection is ready, so the request never settles (porsager/postgres#1195). A waiter that fails with CONNECT_TIMEOUT also stays queued and later takes a connection that is never returned. @effect/sql-pg reserves every transaction's connection this way, inside an acquisition that cannot be interrupted, so a timeout around the transaction does not help. In the node this can strand the history-lease renewal: the owner schedules no further renewal, the lease expires, and shutdown waits on the stuck renewal. It is what hung the saturated batch-pool latency test in 2 of 11 local runs. Holding one batch connection's handshake past the 10 s connect timeout reproduces it every time. No released version contains the fix; 3.4.9 is the latest. This patch is the source change of the open upstream PR porsager/postgres#1229, applied to the ESM and CommonJS builds. Every reserve waiter stays in the pool's queue, including one handed to a reconnect, a rejected waiter leaves the queue, and a closed connection clears its per-query state. With the patch, the connection-delay case passes in about 13 s. Drop the patch once a release includes the fix. pnpm also rewrites the lockfile's overrides in package.json order.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A queued
reserve()can wait forever when the server terminates a connection. The close handler removes the waiter from the pool queue and passes it into reconnect. Connection startup discards that reserve marker, expecting the pool queue to still contain the waiter, so nothing resolves it.Keep reserve waiters in the pool queue whenever a connection attempt starts, including reconnects. The normal open handler can then hand out the connection. Also clear the closed session's query, results, and pending error: otherwise a fatal error from an interrupted query can reject a new reserve, and the pool hands the reconnected slot to that already-rejected waiter. A waiter rejected by a failed connection attempt removes itself from the queue, so the pool neither keeps reconnecting for it nor hands a later connection to it.
Adds two tests using the existing
t()harness and a real server: terminate a reserved backend while another reserve is queued, and terminate a reserved backend while a query is sleeping before reserving again. Both fail before the patch and pass afterward.Validation:
npm testpasses, 266 tests each for ESM, CommonJS, and Deno.Fixes #1195