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
<bharath.rupireddyforpostg...@gmail.com> wrote:
On Tue, Oct 6, 2020 at 11:20 AM Fujii Masao <masao.fu...@oss.nttdata.com> 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.
*/
