Re: Add errdetail() with PID and UID about source of termination signal - Mailing list pgsql-hackers
| From | Chao Li |
|---|---|
| Subject | Re: Add errdetail() with PID and UID about source of termination signal |
| Date | |
| Msg-id | C1589B0B-9CBC-4957-8F65-44FBB7C5BBB1@gmail.com Whole thread Raw |
| In response to | Re: Add errdetail() with PID and UID about source of termination signal (Jakub Wartak <jakub.wartak@enterprisedb.com>) |
| Responses |
Re: Add errdetail() with PID and UID about source of termination signal
|
| List | pgsql-hackers |
> On Feb 25, 2026, at 16:26, Jakub Wartak <jakub.wartak@enterprisedb.com> wrote: > > On Tue, Feb 24, 2026 at 5:15 PM Andrew Dunstan <andrew@dunslane.net> wrote: >> >> >> On 2026-02-24 Tu 5:05 AM, Jakub Wartak wrote: >>> On Tue, Feb 24, 2026 at 9:40 AM Chao Li <li.evan.chao@gmail.com> wrote: >>> [..] >>> >>>> There is guidance in the documentation regarding error message style: https://www.postgresql.org/docs/current/error-style-guide.html >>>> ``` >>>> Detail and hint messages: Use complete sentences, and end each with a period. Capitalize the first word of sentences.Put two spaces after the period if another sentence follows (for English text; might be inappropriate in otherlanguages). >>>> ``` >>>> >>>> I also noticed that some existing DETAIL and HINT messages do not fully follow this guideline. But I believe new codeshould adhere to the documented style as much as possible. In particular, DETAIL and HINT messages should begin witha capital letter and follow the complete-sentence convention. >>> Hi, v2 attached, WIP, the only known remaining issue to me is that >>> windows might fail to compile as it probably doesn't have concept of >>> uid_t... I'm wondering what to do there. >> >> >> I don't think it will arise, as Windows doesn't have the siginfo stuff, >> AFAIK. >> > > Hi Andrew. Ok, so V3 is attached with just one change: uid_t/pid_t > changed to uint32 to make win32 happy. > > Perhaps one question is whether this should be toggleable with GUC or not. > > -J. > <v3-0001-Add-errdetail-with-PID-and-UID-about-source-of-te.patch> A few comments for v3: 1 - syncrep.c ``` @@ -302,7 +302,11 @@ SyncRepWaitForLSN(XLogRecPtr lsn, bool commit) ereport(WARNING, (errcode(ERRCODE_ADMIN_SHUTDOWN), errmsg("canceling the wait for synchronous replication and terminating connection due to administratorcommand"), - errdetail("The transaction has already committed locally, but might not have been replicated to thestandby."))); + errdetail("The transaction has already committed locally, but might not have been replicated to thestandby."), + proc_die_sender_pid == 0 ? 0 : + errdetail("Signal sent by PID %lld, UID %lld.", + (long long)proc_die_sender_pid, (long long)proc_die_sender_uid) + )); ``` Here errdetail is used twice. I guess the second conditional one should be errhint. 2 - syncrep.c ``` - errdetail("The transaction has already committed locally, but might not have been replicated to thestandby."))); + errdetail("The transaction has already committed locally, but might not have been replicated to thestandby."), + proc_die_sender_pid == 0 ? 0 : + errdetail("Signal sent by PID %lld, UID %lld.", + (long long)proc_die_sender_pid, (long long)proc_die_sender_uid) + )); ``` Same as comment 1. 3 ``` +volatile uint32 proc_die_sender_pid = 0; +volatile uint32 proc_die_sender_uid = 0; ``` These two globals are only written in the signal handler, I think they should be sig_atomic_t to ensure atomic writes. 4 ``` - errmsg("terminating walreceiver process due to administrator command"))); + errmsg("terminating walreceiver process due to administrator command"), + proc_die_sender_pid == 0 ? 0 : + errdetail("Signal sent by PID %lld, UID %lld.", + (long long)proc_die_sender_pid, (long long)proc_die_sender_uid) + )); ``` Why do we need to format pid and uid in “long long” format? I searched over the source tree, the current postmaster.c justformats pid as int (%d): ``` /* in parent, successful fork */ ereport(DEBUG2, (errmsg_internal("forked new %s, pid=%d socket=%d", GetBackendTypeDesc(bn->bkend_type), (int) pid, (int) client_sock->sock))); ``` Best regards, -- Chao Li (Evan) HighGo Software Co., Ltd. https://www.highgo.com/
pgsql-hackers by date: