Daniel Gustafsson <dan...@yesql.se> writes:
> I’ve split the patch into two logical parts, the signalling functionality and
> the userfacing terminate/cancel part. For extra clarity I’ve also included
> the
> full v19 patch, in case you prefer that instead when seeing the two.
I'm a bit befuddled by this patch, or at least the proposed test cases.
Those show no proof that the feature actually works, ie, delivers a
message to the target backend. Also, what am I to make of the test cases
involving NULL arguments?
Personally, rather than messing around with defaulted arguments and
detecting nulls at runtime, I'd make two C functions that are both strict.
The signal_message stuff seems both overly complicated and overly fragile.
You have, for example, largely ignored the coding rule that says to have
only straight-line code within a spinlock hold. I am also not very
enamored of setting up Yet Another per-backend data structure in shared
memory, especially one that can so easily get out of sync with the
ProcArray. If we're going to expend dedicated shared memory on this
feature, let's just add the necessary fields to the PGPROC structs.
That would also provide some opportunity to interlock things in a way that
would fix the race conditions, ie, once we've found the relevant PGPROC
entry, hold a lock on it till we've inserted the message and sent the
signal.
I don't especially like orig_length: that information is of no use
to the recipient backend, and what the field is actually being used
as, ie a boolean "slot is full" indicator, is nowhere to be guessed
from the field's documentation.
The current patch fails to apply against parallel_schedule, which
reminds me that you forgot to include an update to serial_schedule.
regards, tom lane
