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: Jelte Fennema-Nio <postgres@jeltef.nl>
Cc: pgsql-hackers <pgsql-hackers@postgresql.org>, Thomas Munro <thomas.munro@gmail.com>, Robert Haas <robertmhaas@gmail.com>
Date: 2024-07-04T11:35:00Z
Lists: pgsql-hackers 
On 04/07/2024 13:50, Jelte Fennema-Nio wrote:
> On Thu, 4 Jul 2024 at 12:35, Heikki Linnakangas <hlinnaka@iki.fi> wrote:
>>
>> On 04/07/2024 13:32, Heikki Linnakangas wrote:
>>> Here's a new version of the first patch.
>>
>> Sorry, forgot attachment.
> 
> It seems you undid the following earlier change. Was that on purpose?
> If not, did you undo any other earlier changes by accident?
> 
>>> +SendCancelRequest(int backendPID, int32 cancelAuthCode)
>>>
>>> I think this name of the function is quite confusing, it's not sending
>>> a cancel request, it is processing one. It sends a SIGINT.
>>
>> Heh, well, it's sending the cancel request signal to the right backend,
>> but I see your point. Renamed to ProcessCancelRequest.

Ah, I made that change as part of the second patch earlier, so I didn't 
consider it now.

I don't feel strongly about it, but I think SendCancelRequest() actually 
feels a little better, in procsignal.c. It's more consistent with the 
existing SendProcSignal() function.

There was indeed another change in the second patch that I missed:

> +				/* If we have setsid(), signal the backend's whole process group */
> +#ifdef HAVE_SETSID
> +				kill(-backendPID, SIGINT);
> +#else
>  				kill(backendPID, SIGINT);
> +#endif

I'm not sure that's really required, when sending SIGINT to a backend 
process. A backend process shouldn't have any child processes, and if it 
did, it's not clear what good SIGINT will do them. But I guess it makes 
sense to do it anyway, for consistency with pg_cancel_backend(), which 
also signals the whole process group.

-- 
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