fix(common.postgresql): Skip connection liveness ping for PgBouncer#19258
Open
skartikey wants to merge 2 commits into
Open
fix(common.postgresql): Skip connection liveness ping for PgBouncer#19258skartikey wants to merge 2 commits into
skartikey wants to merge 2 commits into
Conversation
…admin console The IsPgBouncer flag is also set by the postgresql and postgresql_extensible plugins whenever prepared_statements is disabled, which is the documented setting for reaching a server through a PgBouncer with pool_mode set to transaction. Those connections talk to a real PostgreSQL that parses the "-- ping" liveness query fine, so skipping the ping there only drops the liveness check without fixing anything. Let IsPgBouncer mark the admin console only and move the prepared-statement handling to a separate SimpleProtocol flag. Cover the PostgreSQL side with an integration test asserting that disabling prepared statements keeps the ping.
Contributor
|
Download PR build artifacts for linux_amd64.tar.gz, darwin_arm64.tar.gz, and windows_amd64.zip. 📦 Click here to get additional PR build artifactsArtifact URLs |
srebhan
approved these changes
Jul 15, 2026
srebhan
left a comment
srebhan
left a comment
Member
There was a problem hiding this comment.
Awesome job @skartikey! Approving it, so feel free to merge it if testing succeeds without any change to the code...
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
2 participants
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.
Summary
Since the pgx v5 bump (#18587, v1.38.2) the driver executes "-- ping" before reusing a connection idle for more than one second. PgBouncer's admin console cannot parse this query, so every gather cycle floods the PgBouncer log with "invalid command" errors and the failed ping forces a reconnect.
Skip the ping for admin console connections via the driver's OptionShouldPing hook. This requires creating the database handle from a connector instead of a registered DSN, which as a side effect no longer leaks a driver-registry entry per plugin initialization.
The skip must not reach anything else, which the shared
IsPgBouncerflag could not express:inputs.postgresqlandinputs.postgresql_extensiblealso set it wheneverprepared_statementsis disabled, i.e. for a real PostgreSQL behind a PgBouncer in transaction mode, which parses "-- ping" fine and needs the liveness check.IsPgBouncernow marks the admin console only, with prepared statements moved to a separateSimpleProtocolflag.Integration tests cover both sides: the PgBouncer one fails without the fix with the log lines from the issue, the PostgreSQL one asserts the ping survives disabling prepared statements.
Checklist
Related issues
resolves #19252