On 04/07/2024 13:50, Jelte Fennema-Nio wrote:
On Thu, 4 Jul 2024 at 12:35, Heikki Linnakangas <hlinn...@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)
