Skip to content

Commit

Permalink
Update client varcache with canonical values from server
Browse files Browse the repository at this point in the history
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
  • Loading branch information
JelteF committed Jul 3, 2023
1 parent d746015 commit 2b6d125
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 1 deletion.
1 change: 1 addition & 0 deletions include/varcache.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,4 @@ void varcache_fill_unset(VarCache *src, PgSocket *dst);
void varcache_clean(VarCache *cache);
void varcache_add_params(PktBuf *pkt, VarCache *vars);
void varcache_deinit(void);
void varcache_set_canonical(PgSocket *server, PgSocket *client);
12 changes: 12 additions & 0 deletions src/server.c
Original file line number Diff line number Diff line change
Expand Up @@ -610,6 +610,18 @@ bool server_proto(SBuf *sbuf, SBufEvent evtype, struct MBuf *data)
PgSocket *client = server->link;
Assert(client);

/*
* It's possible that the client vars and server vars have
* different string representations, but still Postgres did not
* send a ParamaterStatus packet. This happens when the server
* variable is the canonical version of the client variable, i.e.
* they mean the same just written slightly different. To make sure
* that the canonical version is also stored in the client, we now
* copy the server variables over to the client variables.
* See issue #776 for an example of this.
*/
varcache_set_canonical(server, client);

server->setting_vars = false;
sbuf_continue(&client->sbuf);
break;
Expand Down
17 changes: 17 additions & 0 deletions src/varcache.c
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,23 @@ bool varcache_apply(PgSocket *server, PgSocket *client, bool *changes_p)
return pktbuf_send_immediate(pkt, server);
}

void varcache_set_canonical(PgSocket *server, PgSocket *client)
{
struct PStr *server_val, *client_val;
const struct var_lookup *lk;
for (lk = lookup; lk->name; lk++) {
server_val = server->vars.var_list[lk->idx];
client_val = client->vars.var_list[lk->idx];
if (client_val && server_val && client_val != server_val) {
slog_debug(client, "varcache_set_canonical: setting %s to its canonical version %s -> %s",
lk->name, client_val->str, server_val->str);
strpool_incref(server_val);
strpool_decref(client_val);
client->vars.var_list[lk->idx] = server_val;
}
}
}

void varcache_fill_unset(VarCache *src, PgSocket *dst)
{
struct PStr *srcval, *dstval;
Expand Down
17 changes: 16 additions & 1 deletion test/test_misc.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
import psycopg
import pytest

from .utils import HAVE_IPV6_LOCALHOST
from .utils import HAVE_IPV6_LOCALHOST, PG_MAJOR_VERSION


def test_connect_query(bouncer):
Expand Down Expand Up @@ -143,3 +143,18 @@ def test_options_startup_param(bouncer):
match="unsupported options startup parameter: parameter too long",
):
bouncer.test(options="-c timezone=" + too_long_param)


def test_equivalent_startup_param(bouncer):
bouncer.admin("set verbose=2")

canonical_expected_times = 1 if PG_MAJOR_VERSION >= 14 else 0
with bouncer.cur(options="-c DateStyle=ISO") as cur:
with bouncer.log_contains(
"varcache_apply: SET DateStyle='ISO'", times=1
), bouncer.log_contains(
"varcache_set_canonical: setting DateStyle to its canonical version ISO -> ISO, MDY",
times=canonical_expected_times,
):
cur.execute("SELECT 1")
cur.execute("SELECT 1")

0 comments on commit 2b6d125

Please sign in to comment.