Re: [PATCH] Extensible ReadyForQuery wire protocol message and C hook, for connection pools and WAIT FOR LSN

Thanks a lot for review, very useful!

On Thu, Oct 8, 2026 at 1:48 PM Manu <manuelreyesbravo(at)gmail(dot)com> wrote:
>
> Hi Hannu,
>
> I measured v2 on current master (it applies cleanly). Behaviourally
> it is v1 plus the postgresql.conf.sample entry, so the observations
> below hold on both versions; the attached scripts reproduce every
> point. I looked at the behaviour rather than the design; some of this
> may well be intended.
>
> The good news first: in plain mode the cost is negligible -- +2
> instructions per ReadyForQuery (+0.01%), tps within noise over two
> runs, and the 'Z' message is byte-identical to master.
>
> Two of the stated claims did not hold in my tests.
>
> > libpq (fe-protocol3.c) is updated so that clients ignore any trailing
> > extension bytes in ReadyForQuery messages if they receive them
>
> An unpatched client does not tolerate a rich 'Z'.

Sorry for being unclear - the protocol change is for the distant
future. The main protection is that one should not set it to rich when
client libarary can not handle this.

> With master's own
> libpq and with PG18's libpq, a rich session fails to connect:
> message contents do not agree with length in message type "Z"
> And since ready_for_query_message is PGC_USERSET, a client that
> connects in plain and later runs SET ready_for_query_message = rich
> hits the same error mid-session and hangs (I had to kill psql after
> 8s, twice).

This is indeed the current behaviour.

But as there are plenty of other ways to make the connection
unresponsive I am not too worried about such abuse.

> > Unless you set ready_for_query_message=rich the protocol does not
> > change at all
>
> The 'Z' itself is unchanged, but because the GUC is GUC_REPORT the
> patched server sends one extra ParameterStatus
> (ready_for_query_message=plain) per connection that master does not.
> Harmless, but not strictly no change.

Agreed, and there is really no need to be GUC_REPORT.

Will remove

> On the session indicators, comparing the rich values with SQL in the
> same session:
> - T stays 1 after the temp objects are gone -- DROP, DISCARD TEMP and
> DISCARD ALL all leave T=1 with an empty pg_class. A pooler would
> read the connection as dirty permanently.

Ouch. Thanks!

> - P reflects the whole cluster, not the session: a session that never
> prepared a transaction reports P=1 while another session has one
> prepared.

Will check.

I need to figure out how it even knows what other sessions have
prepared. That should be backend-local info...

> - L matches pg_current_wal_insert_lsn() numerically, but it is printed
> with %X where pg_lsn uses %08X, so the text differs only while the
> low word is under 8 hex digits, e.g. on a fresh cluster (0/17F5830
> vs 0/017F5830).

Hmm, this is what I get on PostgreSQL 16.4

hannuk=# select '0/017F5830'::pg_lsn;
pg_lsn
───────────
0/17F5830
(1 row)

Maybe there should also be 'l' which just returns the binary uint64 ?

> - H was correct in every case.
>
> Cost of rich grows with open cursors. The H check walks the full
> portal list on every 'Z', so it is O(cursors), not O(1): +5.9%
> instructions per 'Z' on a clean session, +21% with 100 open cursors,
> +160% with 1000. As 'Z' is on the per-query path, that may matter for
> the pooling case the patch targets.

Yup, at least the default should be boolean 0/1. If someone wants the
number of cursors they can write an extension :)

> On CI: v2 turns it green by adding the GUC to postgresql.conf.sample
> (that was the 003_check_guc failure). The rich path itself is
> unchanged -- fe-trace.c is still not updated, so PQtrace() prints
> "mismatched message length" on each rich 'Z' and the 7 libpq_pipeline
> trace tests fail under rich; they pass in plain, which is what CI
> runs. make check is 239/239 in both modes. The new test module
> passes but does not exercise any of the cases above, and there is
> still an unrelated change to Cluster.pm.

Thanks, will add that test as well.

> The attached README lists the exact steps for each point above.

Thanks
Hannu