Thanks for digging into this!
On 03/07/2025 09:13, Jelte Fennema-Nio wrote:
On Thu Jul 3, 2025 at 2:03 AM CEST, Jacob Champion wrote:
On Wed, Jul 2, 2025 at 3:18 PM Jelte Fennema-Nio <postg...@jeltef.nl>
wrote:
I will hold off on detailed review until Heikki gives an opinion on
the design (or we get closer to the end of the month), to avoid making
busy work for you -- but I will say that I think you need to prove
that the new `failure:` case in getBackendKeyData() is safe, because I
don't think any of the other failure modes behave that way inside
pqParseInput3().
I changed it slightly now to align with the handleSyncLoss function its
implementation.
To recap, there is a concrete problem with psycopg2 and libpq v18: if
the server doesn't end BackendKeyData, the connection fails with "can't
get cancellation key" error. With previous versions, it worked, and if
you actually try to cancel, it will send a bogus cancellation message
with all-zeros key and pid.
psycopg3 works better. It only calls PQgetCancel() when actually
cancelling. So you can connect, and only if try to actually cancel, you
get a "couldn't create cancel object" error. That seems quite reasonable.
I think agree with the changes to PQgetCancel()/PQcancel(). It fixes
psycopg2. I don't see any nicer way to signal that cancellation is not
available with PQgetCancel(). New applications should use to
PQcancelCreate(), where we have more options.
I'm not quite sold on the change to PQcancelCreate(). The current
behavior seems nicer: if cancellation is not available because the
server didn't send a cancellation key, PQcancelCreate() returns a
(cancel) connection object that's in a failed state with an error
message explaining what's wrong. The client can choose to continue
without cancellation capability, or bail out.
Are there any known drivers that require the change to PQcancelCreate()?
BTW, if the server doesn't send a BackendKeyData, PQbackendPID() returns
0. Not much we can do about that.
- Heikki
