public inbox for [email protected]
help / color / mirror / Atom feedFrom: Fujii Masao <[email protected]>
To: Bharath Rupireddy <[email protected]>
Cc: PostgreSQL-development <[email protected]>
Subject: Re: Use standard SIGHUP and SIGTERM handlers in autoprewarm module
Date: Wed, 4 Nov 2020 18:06:22 +0900
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <CALj2ACXPorUqePswDtOeM_s82v9RW32E1fYmOPZ5NuE+TWKj_A@mail.gmail.com>
<[email protected]>
<CALj2ACV9NrMDT5Ndpm5STUYRBfKQJoPOgA5a27a1XRc4h8wVYw@mail.gmail.com>
<[email protected]>
<CALj2ACXoCsVFFMi4RAPE-OpWOegS8u5+y_DCP3TXi7GeG61gyg@mail.gmail.com>
<CALj2ACX0JkSM2BhiLog1xP9x2tsdjVdL8U-=OABf2YzYS+ywpQ@mail.gmail.com>
<[email protected]>
On 2020/10/29 15:21, Fujii Masao wrote:
>
>
> On 2020/10/07 11:30, Bharath Rupireddy wrote:
>> On Tue, Oct 6, 2020 at 11:41 AM Bharath Rupireddy
>> <[email protected]> wrote:
>>>
>>> On Tue, Oct 6, 2020 at 11:20 AM Fujii Masao <[email protected]> wrote:
>>>>
>>>> +1 Or it's also ok to make each patch separately.
>>>> Anyway, thanks!
>>>>
>>>
>>> Thanks. +1 to have separate patches. I will post two separate patches
>>> for autoprewarm and WalRcvShutdownHandler for further review. The
>>> other two patches can be for startup.c and syslogger.c.
>>>
>>
>> I'm attaching all the 4 patches here together.
>>
>> 1. v1-use-standard-SIGHUP-and-SIGTERM-handlers-in-autoprewarm-process.patch
>> -- This is the patch initially sent in this mail by me, I just
>> renamed it.
>> 2. v1-use-MyLatch-and-standard-SIGHUP-handler-in-startup-process.patch
>> -- This is the patch proposed by Fuji Masao, I just renamed it.
>> 3. v1-use-standard-SIGHUP-hanlder-in-syslogger-process.patch
>> 4. v1-use-standard-SIGTERM-handler-in-walreceiver-process.patch
>>
>> Please consider these patches for further review.
>
> Thanks for the patches! They look good to me. So barring any objections,
> I will commit them one by one.
I pushed the following two patches.
- v1-use-standard-SIGHUP-hanlder-in-syslogger-process.patch
- v1-use-MyLatch-and-standard-SIGHUP-handler-in-startup-process.patch
Regarding other two patches, I think that it's better to use MyLatch
rather than MyProc->procLatch or walrcv->latch in WaitLatch() and
ResetLatch(), like other code does. Attached are the updated versions
of the patches. Thought?
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
diff --git a/contrib/pg_prewarm/autoprewarm.c b/contrib/pg_prewarm/autoprewarm.c
index c32ddc56fd..d3dec6e3ec 100644
--- a/contrib/pg_prewarm/autoprewarm.c
+++ b/contrib/pg_prewarm/autoprewarm.c
@@ -35,6 +35,7 @@
#include "miscadmin.h"
#include "pgstat.h"
#include "postmaster/bgworker.h"
+#include "postmaster/interrupt.h"
#include "storage/buf_internals.h"
#include "storage/dsm.h"
#include "storage/ipc.h"
@@ -94,12 +95,6 @@ static void apw_start_database_worker(void);
static bool apw_init_shmem(void);
static void apw_detach_shmem(int code, Datum arg);
static int apw_compare_blockinfo(const void *p, const void *q);
-static void apw_sigterm_handler(SIGNAL_ARGS);
-static void apw_sighup_handler(SIGNAL_ARGS);
-
-/* Flags set by signal handlers */
-static volatile sig_atomic_t got_sigterm = false;
-static volatile sig_atomic_t got_sighup = false;
/* Pointer to shared-memory state. */
static AutoPrewarmSharedState *apw_state = NULL;
@@ -161,8 +156,8 @@ autoprewarm_main(Datum main_arg)
TimestampTz last_dump_time = 0;
/* Establish signal handlers; once that's done, unblock signals. */
- pqsignal(SIGTERM, apw_sigterm_handler);
- pqsignal(SIGHUP, apw_sighup_handler);
+ pqsignal(SIGTERM, SignalHandlerForShutdownRequest);
+ pqsignal(SIGHUP, SignalHandlerForConfigReload);
pqsignal(SIGUSR1, procsignal_sigusr1_handler);
BackgroundWorkerUnblockSignals();
@@ -206,19 +201,19 @@ autoprewarm_main(Datum main_arg)
}
/* Periodically dump buffers until terminated. */
- while (!got_sigterm)
+ while (!ShutdownRequestPending)
{
/* In case of a SIGHUP, just reload the configuration. */
- if (got_sighup)
+ if (ConfigReloadPending)
{
- got_sighup = false;
+ ConfigReloadPending = false;
ProcessConfigFile(PGC_SIGHUP);
}
if (autoprewarm_interval <= 0)
{
/* We're only dumping at shutdown, so just wait forever. */
- (void) WaitLatch(&MyProc->procLatch,
+ (void) WaitLatch(MyLatch,
WL_LATCH_SET | WL_EXIT_ON_PM_DEATH,
-1L,
PG_WAIT_EXTENSION);
@@ -247,14 +242,14 @@ autoprewarm_main(Datum main_arg)
}
/* Sleep until the next dump time. */
- (void) WaitLatch(&MyProc->procLatch,
+ (void) WaitLatch(MyLatch,
WL_LATCH_SET | WL_TIMEOUT | WL_EXIT_ON_PM_DEATH,
delay_in_ms,
PG_WAIT_EXTENSION);
}
/* Reset the latch, loop. */
- ResetLatch(&MyProc->procLatch);
+ ResetLatch(MyLatch);
}
/*
@@ -895,35 +890,3 @@ apw_compare_blockinfo(const void *p, const void *q)
return 0;
}
-
-/*
- * Signal handler for SIGTERM
- */
-static void
-apw_sigterm_handler(SIGNAL_ARGS)
-{
- int save_errno = errno;
-
- got_sigterm = true;
-
- if (MyProc)
- SetLatch(&MyProc->procLatch);
-
- errno = save_errno;
-}
-
-/*
- * Signal handler for SIGHUP
- */
-static void
-apw_sighup_handler(SIGNAL_ARGS)
-{
- int save_errno = errno;
-
- got_sighup = true;
-
- if (MyProc)
- SetLatch(&MyProc->procLatch);
-
- errno = save_errno;
-}
diff --git a/src/backend/replication/walreceiver.c b/src/backend/replication/walreceiver.c
index bb1d44ccb7..ff0dc7925c 100644
--- a/src/backend/replication/walreceiver.c
+++ b/src/backend/replication/walreceiver.c
@@ -109,7 +109,6 @@ static XLogSegNo recvSegNo = 0;
* main loop.
*/
static volatile sig_atomic_t got_SIGHUP = false;
-static volatile sig_atomic_t got_SIGTERM = false;
/*
* LogstreamResult indicates the byte positions that we have already
@@ -137,8 +136,6 @@ static void ProcessWalSndrMessage(XLogRecPtr walEnd, TimestampTz sendTime);
/* Signal handlers */
static void WalRcvSigHupHandler(SIGNAL_ARGS);
-static void WalRcvShutdownHandler(SIGNAL_ARGS);
-
/*
* Process any interrupts the walreceiver process may have received.
@@ -164,7 +161,7 @@ ProcessWalRcvInterrupts(void)
*/
CHECK_FOR_INTERRUPTS();
- if (got_SIGTERM)
+ if (ShutdownRequestPending)
{
ereport(FATAL,
(errcode(ERRCODE_ADMIN_SHUTDOWN),
@@ -269,7 +266,7 @@ WalReceiverMain(void)
/* Properly accept or ignore signals the postmaster might send us */
pqsignal(SIGHUP, WalRcvSigHupHandler); /* set flag to read config file */
pqsignal(SIGINT, SIG_IGN);
- pqsignal(SIGTERM, WalRcvShutdownHandler); /* request shutdown */
+ pqsignal(SIGTERM, SignalHandlerForShutdownRequest); /* request shutdown */
/* SIGQUIT handler was already set up by InitPostmasterChild */
pqsignal(SIGALRM, SIG_IGN);
pqsignal(SIGPIPE, SIG_IGN);
@@ -510,7 +507,7 @@ WalReceiverMain(void)
* avoiding some system calls.
*/
Assert(wait_fd != PGINVALID_SOCKET);
- rc = WaitLatchOrSocket(walrcv->latch,
+ rc = WaitLatchOrSocket(MyLatch,
WL_EXIT_ON_PM_DEATH | WL_SOCKET_READABLE |
WL_TIMEOUT | WL_LATCH_SET,
wait_fd,
@@ -518,7 +515,7 @@ WalReceiverMain(void)
WAIT_EVENT_WAL_RECEIVER_MAIN);
if (rc & WL_LATCH_SET)
{
- ResetLatch(walrcv->latch);
+ ResetLatch(MyLatch);
ProcessWalRcvInterrupts();
if (walrcv->force_reply)
@@ -669,7 +666,7 @@ WalRcvWaitForStartPosition(XLogRecPtr *startpoint, TimeLineID *startpointTLI)
WakeupRecovery();
for (;;)
{
- ResetLatch(walrcv->latch);
+ ResetLatch(MyLatch);
ProcessWalRcvInterrupts();
@@ -701,7 +698,7 @@ WalRcvWaitForStartPosition(XLogRecPtr *startpoint, TimeLineID *startpointTLI)
}
SpinLockRelease(&walrcv->mutex);
- (void) WaitLatch(walrcv->latch, WL_LATCH_SET | WL_EXIT_ON_PM_DEATH, 0,
+ (void) WaitLatch(MyLatch, WL_LATCH_SET | WL_EXIT_ON_PM_DEATH, 0,
WAIT_EVENT_WAL_RECEIVER_WAIT_START);
}
@@ -813,21 +810,6 @@ WalRcvSigHupHandler(SIGNAL_ARGS)
got_SIGHUP = true;
}
-
-/* SIGTERM: set flag for ProcessWalRcvInterrupts */
-static void
-WalRcvShutdownHandler(SIGNAL_ARGS)
-{
- int save_errno = errno;
-
- got_SIGTERM = true;
-
- if (WalRcv->latch)
- SetLatch(WalRcv->latch);
-
- errno = save_errno;
-}
-
/*
* Accept the message from XLOG stream, and process it.
*/
Attachments:
[text/plain] v2-use-standard-SIGHUP-and-SIGTERM-handlers-in-autoprewarm-process.patch (2.9K, ../[email protected]/2-v2-use-standard-SIGHUP-and-SIGTERM-handlers-in-autoprewarm-process.patch)
download | inline diff:
diff --git a/contrib/pg_prewarm/autoprewarm.c b/contrib/pg_prewarm/autoprewarm.c
index c32ddc56fd..d3dec6e3ec 100644
--- a/contrib/pg_prewarm/autoprewarm.c
+++ b/contrib/pg_prewarm/autoprewarm.c
@@ -35,6 +35,7 @@
#include "miscadmin.h"
#include "pgstat.h"
#include "postmaster/bgworker.h"
+#include "postmaster/interrupt.h"
#include "storage/buf_internals.h"
#include "storage/dsm.h"
#include "storage/ipc.h"
@@ -94,12 +95,6 @@ static void apw_start_database_worker(void);
static bool apw_init_shmem(void);
static void apw_detach_shmem(int code, Datum arg);
static int apw_compare_blockinfo(const void *p, const void *q);
-static void apw_sigterm_handler(SIGNAL_ARGS);
-static void apw_sighup_handler(SIGNAL_ARGS);
-
-/* Flags set by signal handlers */
-static volatile sig_atomic_t got_sigterm = false;
-static volatile sig_atomic_t got_sighup = false;
/* Pointer to shared-memory state. */
static AutoPrewarmSharedState *apw_state = NULL;
@@ -161,8 +156,8 @@ autoprewarm_main(Datum main_arg)
TimestampTz last_dump_time = 0;
/* Establish signal handlers; once that's done, unblock signals. */
- pqsignal(SIGTERM, apw_sigterm_handler);
- pqsignal(SIGHUP, apw_sighup_handler);
+ pqsignal(SIGTERM, SignalHandlerForShutdownRequest);
+ pqsignal(SIGHUP, SignalHandlerForConfigReload);
pqsignal(SIGUSR1, procsignal_sigusr1_handler);
BackgroundWorkerUnblockSignals();
@@ -206,19 +201,19 @@ autoprewarm_main(Datum main_arg)
}
/* Periodically dump buffers until terminated. */
- while (!got_sigterm)
+ while (!ShutdownRequestPending)
{
/* In case of a SIGHUP, just reload the configuration. */
- if (got_sighup)
+ if (ConfigReloadPending)
{
- got_sighup = false;
+ ConfigReloadPending = false;
ProcessConfigFile(PGC_SIGHUP);
}
if (autoprewarm_interval <= 0)
{
/* We're only dumping at shutdown, so just wait forever. */
- (void) WaitLatch(&MyProc->procLatch,
+ (void) WaitLatch(MyLatch,
WL_LATCH_SET | WL_EXIT_ON_PM_DEATH,
-1L,
PG_WAIT_EXTENSION);
@@ -247,14 +242,14 @@ autoprewarm_main(Datum main_arg)
}
/* Sleep until the next dump time. */
- (void) WaitLatch(&MyProc->procLatch,
+ (void) WaitLatch(MyLatch,
WL_LATCH_SET | WL_TIMEOUT | WL_EXIT_ON_PM_DEATH,
delay_in_ms,
PG_WAIT_EXTENSION);
}
/* Reset the latch, loop. */
- ResetLatch(&MyProc->procLatch);
+ ResetLatch(MyLatch);
}
/*
@@ -895,35 +890,3 @@ apw_compare_blockinfo(const void *p, const void *q)
return 0;
}
-
-/*
- * Signal handler for SIGTERM
- */
-static void
-apw_sigterm_handler(SIGNAL_ARGS)
-{
- int save_errno = errno;
-
- got_sigterm = true;
-
- if (MyProc)
- SetLatch(&MyProc->procLatch);
-
- errno = save_errno;
-}
-
-/*
- * Signal handler for SIGHUP
- */
-static void
-apw_sighup_handler(SIGNAL_ARGS)
-{
- int save_errno = errno;
-
- got_sighup = true;
-
- if (MyProc)
- SetLatch(&MyProc->procLatch);
-
- errno = save_errno;
-}
[text/plain] v2-use-standard-SIGTERM-handler-in-walreceiver-process.patch (2.9K, ../[email protected]/3-v2-use-standard-SIGTERM-handler-in-walreceiver-process.patch)
download | inline diff:
diff --git a/src/backend/replication/walreceiver.c b/src/backend/replication/walreceiver.c
index bb1d44ccb7..ff0dc7925c 100644
--- a/src/backend/replication/walreceiver.c
+++ b/src/backend/replication/walreceiver.c
@@ -109,7 +109,6 @@ static XLogSegNo recvSegNo = 0;
* main loop.
*/
static volatile sig_atomic_t got_SIGHUP = false;
-static volatile sig_atomic_t got_SIGTERM = false;
/*
* LogstreamResult indicates the byte positions that we have already
@@ -137,8 +136,6 @@ static void ProcessWalSndrMessage(XLogRecPtr walEnd, TimestampTz sendTime);
/* Signal handlers */
static void WalRcvSigHupHandler(SIGNAL_ARGS);
-static void WalRcvShutdownHandler(SIGNAL_ARGS);
-
/*
* Process any interrupts the walreceiver process may have received.
@@ -164,7 +161,7 @@ ProcessWalRcvInterrupts(void)
*/
CHECK_FOR_INTERRUPTS();
- if (got_SIGTERM)
+ if (ShutdownRequestPending)
{
ereport(FATAL,
(errcode(ERRCODE_ADMIN_SHUTDOWN),
@@ -269,7 +266,7 @@ WalReceiverMain(void)
/* Properly accept or ignore signals the postmaster might send us */
pqsignal(SIGHUP, WalRcvSigHupHandler); /* set flag to read config file */
pqsignal(SIGINT, SIG_IGN);
- pqsignal(SIGTERM, WalRcvShutdownHandler); /* request shutdown */
+ pqsignal(SIGTERM, SignalHandlerForShutdownRequest); /* request shutdown */
/* SIGQUIT handler was already set up by InitPostmasterChild */
pqsignal(SIGALRM, SIG_IGN);
pqsignal(SIGPIPE, SIG_IGN);
@@ -510,7 +507,7 @@ WalReceiverMain(void)
* avoiding some system calls.
*/
Assert(wait_fd != PGINVALID_SOCKET);
- rc = WaitLatchOrSocket(walrcv->latch,
+ rc = WaitLatchOrSocket(MyLatch,
WL_EXIT_ON_PM_DEATH | WL_SOCKET_READABLE |
WL_TIMEOUT | WL_LATCH_SET,
wait_fd,
@@ -518,7 +515,7 @@ WalReceiverMain(void)
WAIT_EVENT_WAL_RECEIVER_MAIN);
if (rc & WL_LATCH_SET)
{
- ResetLatch(walrcv->latch);
+ ResetLatch(MyLatch);
ProcessWalRcvInterrupts();
if (walrcv->force_reply)
@@ -669,7 +666,7 @@ WalRcvWaitForStartPosition(XLogRecPtr *startpoint, TimeLineID *startpointTLI)
WakeupRecovery();
for (;;)
{
- ResetLatch(walrcv->latch);
+ ResetLatch(MyLatch);
ProcessWalRcvInterrupts();
@@ -701,7 +698,7 @@ WalRcvWaitForStartPosition(XLogRecPtr *startpoint, TimeLineID *startpointTLI)
}
SpinLockRelease(&walrcv->mutex);
- (void) WaitLatch(walrcv->latch, WL_LATCH_SET | WL_EXIT_ON_PM_DEATH, 0,
+ (void) WaitLatch(MyLatch, WL_LATCH_SET | WL_EXIT_ON_PM_DEATH, 0,
WAIT_EVENT_WAL_RECEIVER_WAIT_START);
}
@@ -813,21 +810,6 @@ WalRcvSigHupHandler(SIGNAL_ARGS)
got_SIGHUP = true;
}
-
-/* SIGTERM: set flag for ProcessWalRcvInterrupts */
-static void
-WalRcvShutdownHandler(SIGNAL_ARGS)
-{
- int save_errno = errno;
-
- got_SIGTERM = true;
-
- if (WalRcv->latch)
- SetLatch(WalRcv->latch);
-
- errno = save_errno;
-}
-
/*
* Accept the message from XLOG stream, and process it.
*/
view thread (38+ 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]
Subject: Re: Use standard SIGHUP and SIGTERM handlers in autoprewarm module
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