| From: | Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> |
|---|---|
| To: | Jakub Wartak <jakub(dot)wartak(at)enterprisedb(dot)com> |
| Cc: | Andrew Dunstan <andrew(at)dunslane(dot)net>, Jim Jones <jim(dot)jones(at)uni-muenster(dot)de>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: Add errdetail() with PID and UID about source of termination signal |
| Date: | 2026-02-25 09:15:16 |
| Message-ID: | C1589B0B-9CBC-4957-8F65-44FBB7C5BBB1@gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
> On Feb 25, 2026, at 16:26, Jakub Wartak <jakub(dot)wartak(at)enterprisedb(dot)com> wrote:
>
> On Tue, Feb 24, 2026 at 5:15 PM Andrew Dunstan <andrew(at)dunslane(dot)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(dot)evan(dot)chao(at)gmail(dot)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 other languages).
>>>> ```
>>>>
>>>> I also noticed that some existing DETAIL and HINT messages do not fully follow this guideline. But I believe new code should adhere to the documented style as much as possible. In particular, DETAIL and HINT messages should begin with a 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 administrator command"),
- 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 :
+ 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 the standby.")));
+ errdetail("The transaction has already committed locally, but might not have been replicated to the standby."),
+ 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 just formats 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/
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Álvaro Herrera | 2026-02-25 09:48:58 | Re: More speedups for tuple deformation |
| Previous Message | Antonin Houska | 2026-02-25 08:55:59 | Re: Adding REPACK [concurrently] |