public inbox for [email protected]help / color / mirror / Atom feed
Use proc_exit() in WalRcvWaitForStartPosition 9+ messages / 4 participants [nested] [flat]
* Use proc_exit() in WalRcvWaitForStartPosition @ 2026-04-08 09:08 Chao Li <[email protected]> 0 siblings, 1 reply; 9+ messages in thread From: Chao Li @ 2026-04-08 09:08 UTC (permalink / raw) To: pgsql-hackers Hi, 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(). Best regards, -- Chao Li (Evan) HighGo Software Co., Ltd. https://www.highgo.com/ Attachments: [application/octet-stream] v1-0001-Use-proc_exit-in-WalRcvWaitForStartPosition.patch (860B, 2-v1-0001-Use-proc_exit-in-WalRcvWaitForStartPosition.patch) download | inline diff: From 2a05e01c58100349bf4815d82cd0ed6f0efc967f Mon Sep 17 00:00:00 2001 From: "Chao Li (Evan)" <[email protected]> Date: Wed, 8 Apr 2026 17:02:28 +0800 Subject: [PATCH v1] Use proc_exit() in WalRcvWaitForStartPosition Author: Chao Li <[email protected]> --- src/backend/replication/walreceiver.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/backend/replication/walreceiver.c b/src/backend/replication/walreceiver.c index 09fde92bfd7..47c238557b2 100644 --- a/src/backend/replication/walreceiver.c +++ b/src/backend/replication/walreceiver.c @@ -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) ^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: Use proc_exit() in WalRcvWaitForStartPosition @ 2026-04-08 20:59 Andreas Karlsson <[email protected]> parent: Chao Li <[email protected]> 0 siblings, 1 reply; 9+ messages in thread From: Andreas Karlsson @ 2026-04-08 20:59 UTC (permalink / raw) To: Chao Li <[email protected]>; pgsql-hackers 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. Andreas ^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: Use proc_exit() in WalRcvWaitForStartPosition @ 2026-04-09 01:09 Xuneng Zhou <[email protected]> parent: Andreas Karlsson <[email protected]> 0 siblings, 1 reply; 9+ messages in thread From: Xuneng Zhou @ 2026-04-09 01:09 UTC (permalink / raw) To: Andreas Karlsson <[email protected]>; +Cc: Chao Li <[email protected]>; pgsql-hackers 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 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. -- Best, Xuneng ^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: Use proc_exit() in WalRcvWaitForStartPosition @ 2026-04-10 06:16 Fujii Masao <[email protected]> parent: Xuneng Zhou <[email protected]> 0 siblings, 1 reply; 9+ messages in thread From: Fujii Masao @ 2026-04-10 06:16 UTC (permalink / raw) To: Xuneng Zhou <[email protected]>; +Cc: Andreas Karlsson <[email protected]>; Chao Li <[email protected]>; pgsql-hackers 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 ^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: Use proc_exit() in WalRcvWaitForStartPosition @ 2026-04-10 07:07 Chao Li <[email protected]> parent: Fujii Masao <[email protected]> 0 siblings, 1 reply; 9+ messages in thread From: Chao Li @ 2026-04-10 07:07 UTC (permalink / raw) To: Fujii Masao <[email protected]>; +Cc: Xuneng Zhou <[email protected]>; Andreas Karlsson <[email protected]>; pgsql-hackers > 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) ^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: Use proc_exit() in WalRcvWaitForStartPosition @ 2026-04-10 14:17 Fujii Masao <[email protected]> parent: Chao Li <[email protected]> 0 siblings, 1 reply; 9+ messages in thread From: Fujii Masao @ 2026-04-10 14:17 UTC (permalink / raw) To: Chao Li <[email protected]>; +Cc: Xuneng Zhou <[email protected]>; Andreas Karlsson <[email protected]>; pgsql-hackers On Fri, Apr 10, 2026 at 4:07 PM Chao Li <[email protected]> wrote: > 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”. Thanks for updating the patch! "termination instructs XXX to terminate" sounds a bit redundant. How about saying "to ereport(FATAL)" instead of “to terminate”? Regards, -- Fujii Masao ^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: Use proc_exit() in WalRcvWaitForStartPosition @ 2026-04-12 23:33 Chao Li <[email protected]> parent: Fujii Masao <[email protected]> 0 siblings, 1 reply; 9+ messages in thread From: Chao Li @ 2026-04-12 23:33 UTC (permalink / raw) To: Fujii Masao <[email protected]>; +Cc: Xuneng Zhou <[email protected]>; Andreas Karlsson <[email protected]>; pgsql-hackers > On Apr 10, 2026, at 22:17, Fujii Masao <[email protected]> wrote: > > On Fri, Apr 10, 2026 at 4:07 PM Chao Li <[email protected]> wrote: >> 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”. > > Thanks for updating the patch! > > "termination instructs XXX to terminate" sounds a bit redundant. How > about saying > "to ereport(FATAL)" instead of “to terminate”? > > Regards, > > > -- > Fujii Masao Okay, yes, that was a bit redundant. I changed it to “to ereport(FATAL)” in v3. After that change, the line went over 80 columns, so I also adjusted a few nearby lines to keep everything within the 80-column limit. There is no content change. Best regards, -- Chao Li (Evan) HighGo Software Co., Ltd. https://www.highgo.com/ Attachments: [application/octet-stream] v3-0001-Use-proc_exit-in-WalRcvWaitForStartPosition.patch (1.7K, 2-v3-0001-Use-proc_exit-in-WalRcvWaitForStartPosition.patch) download | inline diff: From 162eb0622f3f7ea436243aa2723737716a7c3e96 Mon Sep 17 00:00:00 2001 From: "Chao Li (Evan)" <[email protected]> Date: Wed, 8 Apr 2026 17:02:28 +0800 Subject: [PATCH v3] 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 | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/backend/replication/walreceiver.c b/src/backend/replication/walreceiver.c index 09fde92bfd7..80743e6af29 100644 --- a/src/backend/replication/walreceiver.c +++ b/src/backend/replication/walreceiver.c @@ -30,9 +30,9 @@ * a new one. * * 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 + * ereport(FATAL). 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. * * This file contains the server-facing parts of walreceiver. The libpq- @@ -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) ^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: Use proc_exit() in WalRcvWaitForStartPosition @ 2026-04-16 03:34 Fujii Masao <[email protected]> parent: Chao Li <[email protected]> 0 siblings, 1 reply; 9+ messages in thread From: Fujii Masao @ 2026-04-16 03:34 UTC (permalink / raw) To: Chao Li <[email protected]>; +Cc: Xuneng Zhou <[email protected]>; Andreas Karlsson <[email protected]>; pgsql-hackers On Mon, Apr 13, 2026 at 8:33 AM Chao Li <[email protected]> wrote: > Okay, yes, that was a bit redundant. I changed it to “to ereport(FATAL)” in v3. > > After that change, the line went over 80 columns, so I also adjusted a few nearby lines to keep everything within the 80-column limit. There is no content change. Thanks for updating the patch! I've pushed it. Regards, -- Fujii Masao ^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: Use proc_exit() in WalRcvWaitForStartPosition @ 2026-04-16 04:02 Chao Li <[email protected]> parent: Fujii Masao <[email protected]> 0 siblings, 0 replies; 9+ messages in thread From: Chao Li @ 2026-04-16 04:02 UTC (permalink / raw) To: Fujii Masao <[email protected]>; +Cc: Xuneng Zhou <[email protected]>; Andreas Karlsson <[email protected]>; pgsql-hackers > On Apr 16, 2026, at 11:34, Fujii Masao <[email protected]> wrote: > > On Mon, Apr 13, 2026 at 8:33 AM Chao Li <[email protected]> wrote: >> Okay, yes, that was a bit redundant. I changed it to “to ereport(FATAL)” in v3. >> >> After that change, the line went over 80 columns, so I also adjusted a few nearby lines to keep everything within the 80-column limit. There is no content change. > > Thanks for updating the patch! I've pushed it. > > Regards, > > -- > Fujii Masao Hi Fujii san, thank you very much for pushing. Best regards, -- Chao Li (Evan) HighGo Software Co., Ltd. https://www.highgo.com/ ^ permalink raw reply [nested|flat] 9+ messages in thread
end of thread, other threads:[~2026-04-16 04:02 UTC | newest] Thread overview: 9+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2026-04-08 09:08 Use proc_exit() in WalRcvWaitForStartPosition Chao Li <[email protected]> 2026-04-08 20:59 ` Andreas Karlsson <[email protected]> 2026-04-09 01:09 ` Xuneng Zhou <[email protected]> 2026-04-10 06:16 ` Fujii Masao <[email protected]> 2026-04-10 07:07 ` Chao Li <[email protected]> 2026-04-10 14:17 ` Fujii Masao <[email protected]> 2026-04-12 23:33 ` Chao Li <[email protected]> 2026-04-16 03:34 ` Fujii Masao <[email protected]> 2026-04-16 04:02 ` Chao Li <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox