fix(db-postgres): release the client checked out during connect - #17831
Open
meikocho1 wants to merge 1 commit into
Open
fix(db-postgres): release the client checked out during connect#17831meikocho1 wants to merge 1 commit into
meikocho1 wants to merge 1 commit into
Conversation
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
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.
What?
@payloadcms/db-postgreschecks out one client from the pool duringconnectand never releases it. This releases it, and adds a regression test asserting the pool has no checked-out clients after connecting.Why?
connectWithReconnectacquires a client, attaches anerrorlistener to it, and returns without callingrelease():A checked-out client is never handed back to the pool, so it is pinned for the lifetime of the process. Two consequences:
pool.end()never drains. It waits for every checked-out client to come back, so it hangs. The reporter of db-postgres: payload.destroy leaves one checked-out pg client, causing pool.end() timeout #15674 measured exactly this — one client outstanding before and afterpayload.destroy(), then apool.end()timeout:One connection is permanently unavailable for queries, on the primary pool and on every read-replica pool, since
connectrunsconnectWithReconnectfor each of them.This also matters for
payload jobs:run, wherepayload.destroy()is called specifically to "close database connections after running jobs so process can exit cleanly" (packages/payload/src/bin/index.ts).Why not end the pool in
destroyinstead? That was my first thought, and it is wrong.reload()inpackages/payload/src/index.tscallspayload.db.destroy()on every hot reload and then reconnects, andconnectguards its pool creation withif (!this.pool)— so the pool is deliberately reused across hot reloads. Ending it indestroywould leave dev with a pool that has been shut down. The leak is in the checkout, so that is where it is fixed.await pool.connect()itself is load-bearing and stays:connectrelies on the error it throws to detect a missing database and branch intocreateDatabase(). Only the checkout is released; theerrorlistener stays attached to the client instance, so the ECONNRESET reconnect path is unchanged.How?
One
result.release()after the listener is attached, plus a test intest/database/int.spec.tsassertingpool.totalCount - pool.idleCount === 0once the adapter is connected — scoped to the adapters that usepg.Pool.Verified against the Postgres suite (
pnpm test:int:postgres database):expected 1 to be +0— one client outstanding, matching the reported measurement exactly.Fixes #15674
Fixes #11727