Horton

Re: Make query cancellation keys longer

Heikki Linnakangas <hlinnaka@iki.fi>

View thread View on postgr.es
From: Heikki Linnakangas <hlinnaka@iki.fi>
To: pgsql-hackers <pgsql-hackers@postgresql.org>, Thomas Munro <thomas.munro@gmail.com>
Cc: Jelte Fennema-Nio <postgres@jeltef.nl>, Robert Haas <robertmhaas@gmail.com>
Date: 2024-07-04T10:32:37Z
Lists: pgsql-hackers 
Here's a new version of the first patch. In the previous version, I 
added the pid cancellation key to pmsignal.c, but on second thoughts, I 
think procsignal.c is a better place. The ProcSignal array already 
contains the pid, we just need to add the cancellation key there.

This first patch just refactors the current code, without changing the 
protocol or length of the cancellation key. I'd like to get this 
reviewed and committed first, and get back to the protocol changes after 
that.

We currently don't do any locking on the ProcSignal array. For query 
cancellations, that's good because a query cancel packet is processed 
without having a PGPROC entry, so we cannot take LWLocks. We could use 
spinlocks though. In this patch, I used memory barriers to ensure that 
we load/store the pid and the cancellation key in a sensible order, so 
that you cannot e.g. send a cancellation signal to a backend that's just 
starting up and hasn't advertised its cancellation key in ProcSignal 
yet. But I think this might be simpler and less error-prone by just 
adding a spinlock to each ProcSignal slot. That would also fix the 
existing race condition where we might set the pss_signalFlags flag for 
a slot, when the process concurrently terminates and the slot is reused 
for a different process. Because of that, we currently have this caveat: 
"... all the signals are such that no harm is done if they're mistakenly 
fired". With a spinlock, we could eliminate that race.

Thoughts?

-- 
Heikki Linnakangas
Neon (https://neon.tech)

Commits

  1. Add timingsafe_bcmp(), for constant-time memory comparison

  2. Add missing declarations to pg_config.h.in

  3. docs: Add a new section and a table listing protocol versions

  4. Make cancel request keys longer

  5. libpq: Add min/max_protocol_version connection options

  6. libpq: Handle NegotiateProtocolVersion message differently

  7. docs: Update phrase on message lengths in the protocol

  8. libpq: Trace all NegotiateProtocolVersion fields

  9. libpq: Add PQfullProtocolVersion to exports.txt

  10. Move cancel key generation to after forking the backend