Re: Make query cancellation keys longer

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)