Update client varcache with canonical values from server #879
Conversation
This file contains 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
JelteF
added a commit
to JelteF/pgjdbc
that referenced
this pull request
Jul 3, 2023
This makes caching variable caching with PgBouncer more efficient, because now the client is setting the same values that the server reports. See these PgBouncer issues/PRs for details: pgbouncer/pgbouncer#776 pgbouncer/pgbouncer#879
9 tasks
JelteF
force-pushed
the
replace-client-vars-with-canonical-version
branch
from
2b6d125
to
418f48e
Compare
davecramer
pushed a commit
to pgjdbc/pgjdbc
that referenced
this pull request
Jul 4, 2023
This makes caching variable caching with PgBouncer more efficient, because now the client is setting the same values that the server reports. See these PgBouncer issues/PRs for details: pgbouncer/pgbouncer#776 pgbouncer/pgbouncer#879
JelteF
force-pushed
the
replace-client-vars-with-canonical-version
branch
from
418f48e
to
c069e6e
Compare
JelteF
force-pushed
the
replace-client-vars-with-canonical-version
branch
from
c069e6e
to
889fb5e
Compare
JelteF
force-pushed
the
replace-client-vars-with-canonical-version
branch
2 times, most recently
from
0ffae79
to
b8aa11b
Compare
|
I did some basic benchmarks against PG15 using: And the TPS is as follows on my machine:
If I introduce 10ms of artificial latency between PgBouncer and Postgres then it goes from:
|
Since PG14, Postgres doesn't send ParameterStatus packets anymore when a setting has not actually changed. This could cause the client varcache and server varcache to become out of sync. Which then in turn resulted in unnecessary `SET` commands to be sent to the server. Fixes pgbouncer#776
JelteF
force-pushed
the
replace-client-vars-with-canonical-version
branch
from
b8aa11b
to
2c032fa
Compare
|
I tested this fix with PG 16.0. It looks good from functional perspective. The downside is the overhead of checking the startup parameters against the canonical ones for every new client connection even if such clients might be in minority. But I am not sure if this feature should be made opt-in. Since the size of varcache we need to iterate for every new connection is so small, the perf hit should be negligible. So, IMO, it does not worth it to introduce a yet another config param. |
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.
Since PG14, Postgres doesn't send ParameterStatus packets anymore when a
setting has not actually changed. This could cause the client varcache
and server varcache to become out of sync. Which then in turn resulted
in unnecessary
SETcommands to be sent to the server.Fixes #776