Re: Make query cancellation keys longer

On Thu, Aug 15, 2024 at 6:07 PM Heikki Linnakangas <hlinnaka(at)iki(dot)fi> wrote:
> Ok, I've read through that thread now, and opined there too. One
> difference is with libpq option name: My patch adds "protocol_version",
> while Jelte proposes "max_protocol_version". I don't have strong
> opinions on that. I hope the ecosystem catches up to support
> NegotiateProtocolVersion quickly, so that only few people will need to
> set this option. In particular, I hope that there will never be need to
> use "max_protocol_version=3.2", because by the time we introduce version
> 3.3, all the connection poolers that support 3.2 will also implement
> NegotiateProtocolVersion.

In Jelte's design, there end up being two connection parameters. We
tell the server we want max_protocol_version, but we accept that it
might give us something older. If, however, it tries to degrade us to
something lower than min_protocol_version, we bail out. I see you've
gone for a simpler design: you ask the server for protocol_version and
you get that or you die. To be honest, right up until exactly now, I
was assuming we wanted a two-parameter system like that, just because
being able to tolerate a range of protocol versions seems useful.
However, maybe we don't need it. Alternatively, we could do this for
now, and then later we could adjust the parameter so that you can say
protocol_version=3.2-3.7 and the client will ask for 3.7 but tolerate
anything >= 3.2. Hmm, I kind of like that idea.

I think it's likely that the ecosystem will catch up with
NegotiateProtocolVersion once things start breaking. However, I feel
pretty confident that there are going to be glitches. Clients are
going to want to force newer protocol versions to make sure they get
new features, or to make sure that security features that they want to
have (like this one) are enabled. Some users are going to be running
old poolers that can't handle 3.2, or there will be weirder things
where the pooler says it supports it but it doesn't actually work
properly in all cases. There are also non-PG servers that reimplement
the PG wire protocol. I can't really enumerate all the things that go
wrong, but I think there are a number of wire protocol changes that
various people have been wanting for a long while now, and when we
start to get the infrastructure in place to make that practical,
people are going to take advantage of it. So I think we can expect a
number of protocol enhancements and changes -- Peter's transparent
column encryption stuff is another example -- and there will be
mistakes by us and mistakes by others along the way. Allowing users to
specify what protocol version they want is probably an important part
of coping with that.

The documentation in the patch you attached still seems to think
there's an explicit length field for the cancel key. Also, I think it
would be good to split this into two patches, one to bump the protocol
version and a second to change the cancel key stuff. It would
facilitate review, and I also think that bumping the protocol version
is a big enough deal that it should have its own entry in the commit
log.

--
Robert Haas
EDB: http://www.enterprisedb.com