Re: Add errdetail() with PID and UID about source of termination signal

On 2026-02-26 Th 4:25 AM, Jakub Wartak wrote:
> On Thu, Feb 26, 2026 at 4:09 AM Chao Li <li.evan.chao@gmail.com> wrote:
>
> Hi Chao,
>
>> I just reviewed v4 again and got a few more comments:
>>
>> 1. This patch only set the global proc_die_sender_pid/uid to 0 at startup, then assign values to them upon receiving SIGTERM, and never reset them, which assumes a process must die upon SIGTERM. Is the assumption true? I guess not. If a process receives SIGTERM and not die immediately, then die for other reason, then it may report a misleading PID and UID.
> Hmm, I'm not sure I follow. If we receive SIGTERM and not die immediately
> (for whatever reason), then two scenarios can happen as far as I'm concerned:
> * another SIGTERM comes in from the same or different uid/pid and it wll be
> reported properly
> * different SIGKILL, but in this case we won't report UID/PID at all
>
> am I missing something or do You have any particular scenario in mind?
> The flow will be wrapper_handler()->die()->SetLatch()->..->directyl to
> err reporting facilities.
>
>> 2.
>> syncrpe.c uses errhint to print PID and UID, and postgres.c uses errdetail. We should keep consistency, maybe all use errhint.
> Right, let's make it that way.
>
>> 3.
>> ```
>> @@ -319,7 +323,11 @@ SyncRepWaitForLSN(XLogRecPtr lsn, bool commit)
>>                          QueryCancelPending = false;
>>                          ereport(WARNING,
>>                                          (errmsg("canceling wait for synchronous replication due to user request"),
>> -                                        errdetail("The transaction has already committed locally, but might not have been replicated to the standby.")));
>> +                                        errdetail("The transaction has already committed locally, but might not have been replicated to the standby."),
>> +                                               proc_die_sender_pid == 0 ? 0 :
>> +                                                       errhint("Signal sent by PID %d, UID %d.",
>> +                                                               proc_die_sender_pid, proc_die_sender_uid)
>> +                                               ));
>>                          SyncRepCancelWait();
>>                          break;
>>                  }
>> ```
>>
>> I don’t think the query cancel case relates to SIGTERM, so we don’t need to log PID and UID here.
> Right, it was superfluous.
>
> v5 attached.
>


I'd kinda like to sneak this in for pg19, because I think it's useful. 
Here's a v6 that changes one or two things:


- changes the globals to sig_atomic_t

- in ProcessInterrupts, copies to local sender_pid/sender_uid, then 
zeros the globals before any ereport

- uses errdetail() for all the messages


Plus a few more cosmetic changes like consistent casing.


cheers


andrew



--
Andrew Dunstan
EDB: https://www.enterprisedb.com