Re: Make query cancellation keys longer
Jelte Fennema-Nio <postgres@jeltef.nl>
From: Jelte Fennema-Nio <postgres@jeltef.nl>
To: Robert Haas <robertmhaas@gmail.com>
Cc: Heikki Linnakangas <hlinnaka@iki.fi>,
pgsql-hackers <pgsql-hackers@postgresql.org>, Thomas Munro <thomas.munro@gmail.com>
Date: 2025-02-25T14:28:20Z
Lists: pgsql-hackers
On Mon, 24 Feb 2025 at 18:24, Robert Haas <robertmhaas@gmail.com> wrote: > > On Sat, Feb 22, 2025 at 9:58 AM Jelte Fennema-Nio <postgres@jeltef.nl> wrote: > > My patchset in the other protocol thread needed a rebase. So I took > > that as an opportunity to rebase this patchset on top of it, because > > this seems to be the protocol change that we can most easily push over > > the finish line. > > I'm still not really convinced about that -- see comments upthread -- You've said quite a few things upthread, so I'm going to assume you are referring to the following. If that's not the case could you please quote the part of the thread that you think is important: On Wed, 5 Jun 2024 at 17:26, Robert Haas <robertmhaas@gmail.com> wrote: > I think we shouldn't underestimate the impact of bumping the minor > protocol version. Minor version negotiation is probably not supported > by all drivers and Jelte says that it's not supported by any poolers, > so for anybody but direct libpq users, there will be some breakage. > Now, on the one hand, as Jelte has said, there's little value in > having a protocol version if we're too afraid to make use of it. But > on the other hand, is this problem serious enough to justify the > breakage we'll cause? I'm not sure. It seems pretty silly to be using > a 32-bit value for this in 2024, but I'm sure some people aren't going > to like it, and they may not all have noticed this thread. On the > third hand, if we do this, it may help to unblock a bunch of other > pending patches that also want to do protocol-related things. So to be clear: the current patchset *does not* change any client defaults. libpq will still connect to a server using the protocol version 3.0 by default. It will only connect with 3.2 unless instructed explicitly in the connection string using min/max_protocol_version. So no breakage by default is going to happen, and very few users will hit any breakage at all, because they won't change the default. The reason I think that this longer cancel key change is the easiest protocol change to get through, is precisely because it's a really minor change that almost no-one actually cares about. Don't get me wrong, it's useful feature from a security perspective, and for people implementing poolers, but for both of those it's a relatively minor benefit. To me the main feature of doing this change, is to nudge poolers and other servers to implement the NegotiateProtocolVersion feature. I think there are two reasons why no server except Postgres has implemented it: 1. There was no immediate usecase, because there was no new protocol version in sight that would be useful to them. 2. There was no easy way to test this. They would need to recompile libpq/psql to be able to send a different protocol version. Both of those would be addressed by getting this really minor change in. 1 would be addressed, because this change is a signal that we intend to do more useful protocol changes in the somewhat near future. And 2 is addressed because you can now send 2 different protocol versions with a single psql binary. So by releasing this the ecosystem is able to catch up to Postgres its NegotiateProtocolVersion support, which will mean much less breakage in the future when we make a protocol change that users will actively want to use (e.g. because it's a feature they care about or makes their queries faster). > but I committed 0001 and 0002, which appear to be straightforward bug > fixes. Nice.
Commits
-
Add timingsafe_bcmp(), for constant-time memory comparison
- b282280e9b69 14.23 landed
- 9dcfcb92fff8 15.18 landed
- 1604939b2210 16.14 landed
- 8e34acfda115 17.10 landed
- 09be39112654 18.0 landed
-
Add missing declarations to pg_config.h.in
- b82e7eddb023 18.0 landed
-
docs: Add a new section and a table listing protocol versions
- b05751220b0c 18.0 landed
-
Make cancel request keys longer
- a460251f0a1a 18.0 landed
-
libpq: Add min/max_protocol_version connection options
- 285613c60a7a 18.0 landed
-
libpq: Handle NegotiateProtocolVersion message differently
- 5070349102af 18.0 landed
-
docs: Update phrase on message lengths in the protocol
- 85d799ba8a7f 18.0 landed
-
libpq: Trace all NegotiateProtocolVersion fields
- e87c14b19ed4 18.0 landed
-
libpq: Add PQfullProtocolVersion to exports.txt
- c9d94ea2158b 18.0 landed
-
Move cancel key generation to after forking the backend
- 9d9b9d46f3c5 18.0 landed