public inbox for [email protected]
help / color / mirror / Atom feedFrom: Chao Li <[email protected]>
To: Fujii Masao <[email protected]>
Cc: Xuneng Zhou <[email protected]>
Cc: Andreas Karlsson <[email protected]>
Cc: PostgreSQL-development <[email protected]>
Subject: Re: Use proc_exit() in WalRcvWaitForStartPosition
Date: Fri, 10 Apr 2026 15:07:03 +0800
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAHGQGwHHrDv8Z=D-UP+-RUR2yntv3Ab=yw4_uPWEMY0vLX_O6g@mail.gmail.com>
References: <[email protected]>
<[email protected]>
<CABPTF7V=y2rHC=gLTeC-42aRc+jcXueomUBjbNiy3pOfhJs3_A@mail.gmail.com>
<CAHGQGwHHrDv8Z=D-UP+-RUR2yntv3Ab=yw4_uPWEMY0vLX_O6g@mail.gmail.com>
> On Apr 10, 2026, at 14:16, Fujii Masao <[email protected]> wrote:
>
> On Thu, Apr 9, 2026 at 10:09 AM Xuneng Zhou <[email protected]> wrote:
>>
>> On Thu, Apr 9, 2026 at 5:00 AM Andreas Karlsson <[email protected]> wrote:
>>>
>>> On 4/8/26 11:08 AM, Chao Li wrote:
>>>> While working on another patch, I happened to notice that WalRcvWaitForStartPosition() calls raw exit(1). I think this should use proc_exit(1) instead, so that the normal cleanup machinery is not bypassed.
>>>>
>>>> This tiny patch just replaces exit(1) with proc_exit(1) in WalRcvWaitForStartPosition().
>>>
>>> This looks likely to be correct since when we exit in WalReceiverMain()
>>> (on WALRCV_STOPPING and WALRCV_STOPPED) we call proc_exit(1). I feel we
>>> should exit the same way in WalRcvWaitForStartPosition() as we do in
>>> WalReceiverMain() and if not I would like a comment explaining why those
>>> two cases are different.
>>
>> +1
>
> +1
>
>
>> WalRcvWaitForStartPosition, WALRCV_STOPPING before entering wait loop
>> uses proc_exit(0) for WALRCV_STOPPING, while this path should probably
>> use proc_exit(0) as well (not proc_exit(1)), since the stop was a
>> requested shutdown, not an error. Using exit code 1 for a clean
>> stop-on-request seems inconsistent.
>
> The requested shutdown is handled in ShutdownWalRcv(), which sets the state to
> WALRCV_STOPPING and sends SIGTERM to the walreceiver.
>
> Although this might be considered a normal shutdown (suggesting exit code 0),
> when the walreceiver receives SIGTERM it exits via ereport(FATAL), resulting
> in exit code 1. In contrast, if it exits early in WalRcvWaitForStartPosition()
> due to the WALRCV_STOPPING state, it uses exit code 0, as you noted. So
> there seems to be some inconsistency in exit codes.
>
> That said, the exit code (0 vs 1) does not affect behavior, since
> the postmaster treats both as non-crash exits.
>
> For consistency, I would prefer using exit code 1 in proc_exit() in
> WalRcvWaitForStartPosition(), to match the ereport(FATAL) path. But I'm fine
> with other approaches as well.
>
> Also, the comment at the top of walreceiver.c may need updating:
>
> * Normal termination is by SIGTERM, which instructs the walreceiver to
> * exit(0). Emergency termination is by SIGQUIT; like any postmaster child
> * process, the walreceiver will simply abort and exit on SIGQUIT. A close
> * of the connection and a FATAL error are treated not as a crash but as
> * normal operation.
>
> Regards,
>
> --
> Fujii Masao
PFA v2 - updated header comment of walreceive.c. I tried to avoid mentioning the exact exit value in the comment, so I just changed “exit(0)” to “terminate”.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
Attachments:
[application/octet-stream] v2-0001-Use-proc_exit-in-WalRcvWaitForStartPosition.patch (1.5K, 2-v2-0001-Use-proc_exit-in-WalRcvWaitForStartPosition.patch)
download | inline diff:
From e1070dbd77f251774fcddee163f16fbd89b70be5 Mon Sep 17 00:00:00 2001
From: "Chao Li (Evan)" <[email protected]>
Date: Wed, 8 Apr 2026 17:02:28 +0800
Subject: [PATCH v2] Use proc_exit() in WalRcvWaitForStartPosition
Author: Chao Li <[email protected]>
Reviewed-by: Fujii Masao <[email protected]>
Reviewed-by: Andreas Karlsson <[email protected]>
Reviewed-by: Xuneng Zhou <[email protected]>
Discussion: https://postgr.es/m/[email protected]
---
src/backend/replication/walreceiver.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/backend/replication/walreceiver.c b/src/backend/replication/walreceiver.c
index 09fde92bfd7..0941c9c4674 100644
--- a/src/backend/replication/walreceiver.c
+++ b/src/backend/replication/walreceiver.c
@@ -30,7 +30,7 @@
* a new one.
*
* Normal termination is by SIGTERM, which instructs the walreceiver to
- * exit(0). Emergency termination is by SIGQUIT; like any postmaster child
+ * terminate. Emergency termination is by SIGQUIT; like any postmaster child
* process, the walreceiver will simply abort and exit on SIGQUIT. A close
* of the connection and a FATAL error are treated not as a crash but as
* normal operation.
@@ -710,7 +710,7 @@ WalRcvWaitForStartPosition(XLogRecPtr *startpoint, TimeLineID *startpointTLI)
* to die, but might as well check it here too.
*/
SpinLockRelease(&walrcv->mutex);
- exit(1);
+ proc_exit(1);
}
SpinLockRelease(&walrcv->mutex);
--
2.50.1 (Apple Git-155)
view thread (9+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected]
Subject: Re: Use proc_exit() in WalRcvWaitForStartPosition
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox