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.
With Regards,
Bharath Rupireddy.
EnterpriseDB: http://www.enterprisedb.com
From b9d77b6e84d3eee87693893c1687f120ee3db68b Mon Sep 17 00:00:00 2001
From: Bharath Rupireddy <bharath.rupireddy@enterprisedb.com>
Date: Wed, 7 Oct 2020 07:12:57 +0530
Subject: [PATCH v1] autoprewarm use standard SIGHUP and SIGTERM handlers
Replace apw_sigterm_handler() and apw_sighup_handler() with standard
SignalHandlerForShutdownRequest() and SignalHandlerForConfigReload()
respectively.
---
contrib/pg_prewarm/autoprewarm.c | 49 ++++----------------------------
1 file changed, 6 insertions(+), 43 deletions(-)
diff --git a/contrib/pg_prewarm/autoprewarm.c b/contrib/pg_prewarm/autoprewarm.c
index c32ddc56fd..93e526ef62 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,12 +201,12 @@ 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);
}
@@ -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;
-}
--
2.25.1
From 50d718fec6ade8c101d466ee71f0ab30ed020182 Mon Sep 17 00:00:00 2001
From: Bharath Rupireddy <bharath.rupireddy@enterprisedb.com>
Date: Wed, 7 Oct 2020 07:17:49 +0530
Subject: [PATCH v1] startup use MyLatch and standard SIGHUP handler
Remove a separate shared latch with MyLatch which is also a shared
latch in startup process. This change can let startup process use
standard SIGHUP handler SignalHandlerForConfigReload() instead of
StartupProcSigHupHandler().
---
src/backend/access/transam/xlog.c | 30 +++++++++++-------------------
src/backend/postmaster/startup.c | 24 +++++-------------------
2 files changed, 16 insertions(+), 38 deletions(-)
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 52a67b1170..c211d66994 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -682,7 +682,7 @@ typedef struct XLogCtlData
* WAL replay, if it is waiting for WAL to arrive or failover trigger file
* to appear.
*/
- Latch recoveryWakeupLatch;
+ Latch *recoveryWakeupLatch;
/*
* During recovery, we keep a copy of the latest checkpoint record here.
@@ -5185,7 +5185,6 @@ XLOGShmemInit(void)
SpinLockInit(&XLogCtl->Insert.insertpos_lck);
SpinLockInit(&XLogCtl->info_lck);
SpinLockInit(&XLogCtl->ulsn_lck);
- InitSharedLatch(&XLogCtl->recoveryWakeupLatch);
}
/*
@@ -6122,7 +6121,7 @@ recoveryApplyDelay(XLogReaderState *record)
while (true)
{
- ResetLatch(&XLogCtl->recoveryWakeupLatch);
+ ResetLatch(MyLatch);
/* might change the trigger file's location */
HandleStartupProcInterrupts();
@@ -6146,7 +6145,7 @@ recoveryApplyDelay(XLogReaderState *record)
elog(DEBUG2, "recovery apply delay %ld seconds, %d milliseconds",
secs, microsecs / 1000);
- (void) WaitLatch(&XLogCtl->recoveryWakeupLatch,
+ (void) WaitLatch(MyLatch,
WL_LATCH_SET | WL_TIMEOUT | WL_EXIT_ON_PM_DEATH,
secs * 1000L + microsecs / 1000,
WAIT_EVENT_RECOVERY_APPLY_DELAY);
@@ -6474,11 +6473,11 @@ StartupXLOG(void)
}
/*
- * Take ownership of the wakeup latch if we're going to sleep during
- * recovery.
+ * Advertise our latch that other processes can use to wake us up
+ * if we're going to sleep during recovery.
*/
if (ArchiveRecoveryRequested)
- OwnLatch(&XLogCtl->recoveryWakeupLatch);
+ XLogCtl->recoveryWakeupLatch = &MyProc->procLatch;
/* Set up XLOG reader facility */
MemSet(&private, 0, sizeof(XLogPageReadPrivate));
@@ -7488,13 +7487,6 @@ StartupXLOG(void)
if (InRecovery)
ResetUnloggedRelations(UNLOGGED_RELATION_INIT);
- /*
- * We don't need the latch anymore. It's not strictly necessary to disown
- * it, but let's do it for the sake of tidiness.
- */
- if (ArchiveRecoveryRequested)
- DisownLatch(&XLogCtl->recoveryWakeupLatch);
-
/*
* We are now done reading the xlog from stream. Turn off streaming
* recovery to force fetching the files (which would be required at end of
@@ -12242,12 +12234,12 @@ WaitForWALToBecomeAvailable(XLogRecPtr RecPtr, bool randAccess,
wait_time = wal_retrieve_retry_interval -
(secs * 1000 + usecs / 1000);
- (void) WaitLatch(&XLogCtl->recoveryWakeupLatch,
+ (void) WaitLatch(MyLatch,
WL_LATCH_SET | WL_TIMEOUT |
WL_EXIT_ON_PM_DEATH,
wait_time,
WAIT_EVENT_RECOVERY_RETRIEVE_RETRY_INTERVAL);
- ResetLatch(&XLogCtl->recoveryWakeupLatch);
+ ResetLatch(MyLatch);
now = GetCurrentTimestamp();
}
last_fail_time = now;
@@ -12498,11 +12490,11 @@ WaitForWALToBecomeAvailable(XLogRecPtr RecPtr, bool randAccess,
* to react to a trigger file promptly and to check if the
* WAL receiver is still active.
*/
- (void) WaitLatch(&XLogCtl->recoveryWakeupLatch,
+ (void) WaitLatch(MyLatch,
WL_LATCH_SET | WL_TIMEOUT |
WL_EXIT_ON_PM_DEATH,
5000L, WAIT_EVENT_RECOVERY_WAL_STREAM);
- ResetLatch(&XLogCtl->recoveryWakeupLatch);
+ ResetLatch(MyLatch);
break;
}
@@ -12674,7 +12666,7 @@ CheckPromoteSignal(void)
void
WakeupRecovery(void)
{
- SetLatch(&XLogCtl->recoveryWakeupLatch);
+ SetLatch(XLogCtl->recoveryWakeupLatch);
}
/*
diff --git a/src/backend/postmaster/startup.c b/src/backend/postmaster/startup.c
index 64af7b8707..eab9c8c4ed 100644
--- a/src/backend/postmaster/startup.c
+++ b/src/backend/postmaster/startup.c
@@ -37,7 +37,6 @@
/*
* Flags set by interrupt handlers for later service in the redo loop.
*/
-static volatile sig_atomic_t got_SIGHUP = false;
static volatile sig_atomic_t shutdown_requested = false;
static volatile sig_atomic_t promote_signaled = false;
@@ -49,7 +48,6 @@ static volatile sig_atomic_t in_restore_command = false;
/* Signal handlers */
static void StartupProcTriggerHandler(SIGNAL_ARGS);
-static void StartupProcSigHupHandler(SIGNAL_ARGS);
/* --------------------------------
@@ -64,19 +62,7 @@ StartupProcTriggerHandler(SIGNAL_ARGS)
int save_errno = errno;
promote_signaled = true;
- WakeupRecovery();
-
- errno = save_errno;
-}
-
-/* SIGHUP: set flag to re-read config file at next convenient time */
-static void
-StartupProcSigHupHandler(SIGNAL_ARGS)
-{
- int save_errno = errno;
-
- got_SIGHUP = true;
- WakeupRecovery();
+ SetLatch(MyLatch);
errno = save_errno;
}
@@ -91,7 +77,7 @@ StartupProcShutdownHandler(SIGNAL_ARGS)
proc_exit(1);
else
shutdown_requested = true;
- WakeupRecovery();
+ SetLatch(MyLatch);
errno = save_errno;
}
@@ -137,9 +123,9 @@ HandleStartupProcInterrupts(void)
/*
* Process any requests or signals received recently.
*/
- if (got_SIGHUP)
+ if (ConfigReloadPending)
{
- got_SIGHUP = false;
+ ConfigReloadPending = false;
StartupRereadConfig();
}
@@ -172,7 +158,7 @@ StartupProcessMain(void)
/*
* Properly accept or ignore signals the postmaster might send us.
*/
- pqsignal(SIGHUP, StartupProcSigHupHandler); /* reload config file */
+ pqsignal(SIGHUP, SignalHandlerForConfigReload); /* reload config file */
pqsignal(SIGINT, SIG_IGN); /* ignore query cancel */
pqsignal(SIGTERM, StartupProcShutdownHandler); /* request shutdown */
/* SIGQUIT handler was already set up by InitPostmasterChild */
--
2.25.1
From a521ed1c62df788a7b613b7ee669d58844951668 Mon Sep 17 00:00:00 2001
From: Bharath Rupireddy <bharath.rupireddy@enterprisedb.com>
Date: Wed, 7 Oct 2020 07:34:38 +0530
Subject: [PATCH v1] syslogger - use standard SIGHUP handler
Replace sigHupHanlder() with standard SignalHandlerForConfigReload().
---
src/backend/postmaster/syslogger.c | 21 ++++-----------------
1 file changed, 4 insertions(+), 17 deletions(-)
diff --git a/src/backend/postmaster/syslogger.c b/src/backend/postmaster/syslogger.c
index ffcb54968f..faa82ec481 100644
--- a/src/backend/postmaster/syslogger.c
+++ b/src/backend/postmaster/syslogger.c
@@ -39,6 +39,7 @@
#include "pgstat.h"
#include "pgtime.h"
#include "postmaster/fork_process.h"
+#include "postmaster/interrupt.h"
#include "postmaster/postmaster.h"
#include "postmaster/syslogger.h"
#include "storage/dsm.h"
@@ -123,7 +124,6 @@ static CRITICAL_SECTION sysloggerSection;
/*
* Flags set by interrupt handlers for later service in the main loop.
*/
-static volatile sig_atomic_t got_SIGHUP = false;
static volatile sig_atomic_t rotation_requested = false;
@@ -144,7 +144,6 @@ static unsigned int __stdcall pipeThread(void *arg);
static void logfile_rotate(bool time_based_rotation, int size_rotation_for);
static char *logfile_getname(pg_time_t timestamp, const char *suffix);
static void set_next_rotation_time(void);
-static void sigHupHandler(SIGNAL_ARGS);
static void sigUsr1Handler(SIGNAL_ARGS);
static void update_metainfo_datafile(void);
@@ -240,7 +239,7 @@ SysLoggerMain(int argc, char *argv[])
* broken backends...
*/
- pqsignal(SIGHUP, sigHupHandler); /* set flag to read config file */
+ pqsignal(SIGHUP, SignalHandlerForConfigReload); /* set flag to read config file */
pqsignal(SIGINT, SIG_IGN);
pqsignal(SIGTERM, SIG_IGN);
pqsignal(SIGQUIT, SIG_IGN);
@@ -324,9 +323,9 @@ SysLoggerMain(int argc, char *argv[])
/*
* Process any requests or signals received recently.
*/
- if (got_SIGHUP)
+ if (ConfigReloadPending)
{
- got_SIGHUP = false;
+ ConfigReloadPending = false;
ProcessConfigFile(PGC_SIGHUP);
/*
@@ -1553,18 +1552,6 @@ RemoveLogrotateSignalFiles(void)
unlink(LOGROTATE_SIGNAL_FILE);
}
-/* SIGHUP: set flag to reload config file */
-static void
-sigHupHandler(SIGNAL_ARGS)
-{
- int save_errno = errno;
-
- got_SIGHUP = true;
- SetLatch(MyLatch);
-
- errno = save_errno;
-}
-
/* SIGUSR1: set flag to rotate logfile */
static void
sigUsr1Handler(SIGNAL_ARGS)
--
2.25.1
From 45c36aab4a2b1c66967320688ffc9ee916032abc Mon Sep 17 00:00:00 2001
From: Bharath Rupireddy <bharath.rupireddy@enterprisedb.com>
Date: Wed, 7 Oct 2020 07:09:35 +0530
Subject: [PATCH v1] walreceiver use standard SIGTERM handler
Replace WalRcvShutdownHandler() with standard SignalHandlerForShutdownRequest().
---
src/backend/replication/walreceiver.c | 22 ++--------------------
1 file changed, 2 insertions(+), 20 deletions(-)
diff --git a/src/backend/replication/walreceiver.c b/src/backend/replication/walreceiver.c
index bb1d44ccb7..65f933898d 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);
@@ -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.
*/
--
2.25.1
