| From: | Manu <manuelreyesbravo(at)gmail(dot)com> |
|---|---|
| To: | pgsql-hackers(at)lists(dot)postgresql(dot)org |
| Cc: | Hannu Krosing <hannuk(at)google(dot)com> |
| Subject: | Re: [PATCH] Extensible ReadyForQuery wire protocol message and C hook, for connection pools and WAIT FOR LSN |
| Date: | 2026-10-08 11:48:02 |
| Message-ID: | 179146008286.393055.16733548853161134599@gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
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'. 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).
> 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.
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.
- P reflects the whole cluster, not the session: a session that never
prepared a transaction reports P=1 while another session has one
prepared.
- 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).
- 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.
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.
The attached README lists the exact steps for each point above.
Regards,
Manu
| Attachment | Content-Type | Size |
|---|---|---|
| nocfbot-rfq-repro-README.txt | text/plain | 5.9 KB |
| nocfbot-rfq-wire-client.py.txt | text/plain | 5.0 KB |
| nocfbot-rfq-old-libpq-client.c.txt | text/plain | 2.6 KB |
| nocfbot-rfq-pgbench.sh.txt | text/plain | 3.0 KB |
| nocfbot-rfq-instructions.sh.txt | text/plain | 4.1 KB |
| From | Date | Subject | |
|---|---|---|---|
| Previous Message | Kirill Reshke | 2026-10-08 11:38:59 | REPACK hits assertion failure on postmaster death exit |