Re: BackendKeyData is mandatory?

On Mon, 30 Jun 2025 at 19:59, Jacob Champion
<jacob(dot)champion(at)enterprisedb(dot)com> wrote:
>
> On Wed, Jun 25, 2025 at 12:12 AM Jelte Fennema-Nio <postgres(at)jeltef(dot)nl> wrote:
> > Attached is an attempt at implementing the above. I did not test it
> > against these systems though.
>
> With 0001, psycopg2 appears to function again when talking to a server
> that doesn't send a cancellation key, so that's good.

Great!

> It looks like Heikki has an open item for this, so I'll defer to him

Oh... Sorry for the confusion. I added that open item to the list (so
it would not be missed), and I added Heikki as the owner because he
committed the original patch. I checked now, and since Heikki hasn't
sent an email to hackers since June 10th, I'm not sure that was the
correct decision. So feel free to take ownership of the open item. I
changed it to "Unknown" for now (I did the same for some of the other
protocol docs related patches).

> for copyediting preferences around the comments and commit messages. I
> do have a problem with this part of the new documentation:
>
> + [...] If no
> + BackendKeyData is sent by the server, then that means that the backend
> + does not support canceling queries using the CancelRequest messages.
>
> I don't think we really know that it means that, yet. I'd prefer something like
>
> Beginning with PostgreSQL 18, libpq assumes that servers do not
> support query cancellation if they do not send BackendKeyData.
>
> in the hope that either anyone affected will complain at us to revert,
> or it'll become the de facto meaning after some time.

I understand your intent, but I don't like that the protocol docs
would be saying anything about libpq specific behaviour. I'd like to
be explicit about protocol behaviour, without considering the specific
client. That we weren't before is exactly how we got into the current
mess (maybe those IETF are on to something with their MUST/SHOULD/etc
stuff).

So I still prefer my wording over yours, especially since no-one has
been able to think of any reasonable way a production system could
utilize an "all zeros" cancellation key (except for having only one
connection, which automatically makes it non-production afaict). But I
do understand your concern, so how about this wording:

Since protocol version 3.2, if the server sent no BackendKeyData, then
that means that the backend does not support canceling queries using
the CancelRequest messages. In protocol versions before 3.2 the
behaviour is undefined if the client receives no BackendKeyData.

That way we define the behavior as "sensible" in 3.2, while still
allowing for the rare case that someone somewhere relied on the "all
zeros" cancel message being sent in libpq for PG17. And if it's a big
enough problem, we could then still change libpq to continue sending
"all zeros" if the server used protocol 3.0.

> 0002 seems to result in a connection hang if the server sends a bigger
> key. I think this may be a latent bug in the original patch -- if
> getBackendKeyData() fails, no effort is made to clean up state or
> manage the connection buffer. We just... return.

Hmm... Yeah, seems like that needs a bit more work then.

> Also, what should we do if the server sends a zero-length key?

Given that that only applies to protocol 3.2, I'd like to define that
strictly. How about simply not allowing that and throwing an error in
libpq in that case? e.g. by defining the secret so that it should be
at minimum 4 and at most 256 bytes?