Hi,
Autoprewarm module is using it's own SIGHUP(apw_sigterm_handler,
got_sigterm) and SIGTERM(apw_sighup_handler, got_sighup) handlers which are
similar to standard signal handlers(except for a difference [1]). Isn't it
good to remove them and use standard SignalHandlerForConfigReload and
SignalHandlerForShutdownRequest?
Attaching the patch for the above changes.
Looks like the commit[2] replaced custom handlers with standard handlers.
Thoughts?
[1] apw_sigterm_handler() and apw_sighup_handler() use MyProc->procLatch
if (MyProc)
SetLatch(&MyProc->procLatch);
where as standard handlers use MyLatch
SetLatch(MyLatch);
Both MyProc->procLatch and MyLatch point to same, see comment from global.c
/*
* MyLatch points to the latch that should be used for signal handling by
the
* current process. It will either point to a process local latch if the
* current process does not have a PGPROC entry in that moment, or to
* PGPROC->procLatch if it has.
*Thus it can always be used in signal handlers, * without checking for its
existence.*
*/
struct Latch *MyLatch;
(gdb) p MyProc->procLatch
$6 = {is_set = 0, is_shared = true, owner_pid = 1448807}
(gdb) p MyLatch
*$7 = (struct Latch *) 0x7fcacc6d902c*
(gdb) p &MyProc->procLatch
*$8 = (Latch *) 0x7fcacc6d902c*
(gdb) p *MyLatch
$9 = {is_set = 0, is_shared = true, owner_pid = 1448807}
[2] commit 1e53fe0e70f610c34f4c9e770d108cd94151342c
Author: Robert Haas <[email protected]>
Date: 2019-12-17 13:03:57 -0500
Use PostgresSigHupHandler in more places.
There seems to be no reason for every background process to have
its own flag indicating that a config-file reload is needed.
Instead, let's just use ConfigFilePending for that purpose
everywhere.
With Regards,
Bharath Rupireddy.
EnterpriseDB: http://www.enterprisedb.com
From 7659d36c227fb8193bc76c03f8f215f0a3ac3bb5 Mon Sep 17 00:00:00 2001
From: Bharath Rupireddy <[email protected]>
Date: Sat, 3 Oct 2020 09:40:05 +0530
Subject: [PATCH v1] Use Standard SIGTERM,SIGHUP Handlers In AutoPrewarm
Replace apw_sigterm_handler and apw_sighup_handler with standard
signal handlers SignalHandlerForShutdownRequest and
SignalHandlerForConfigReload.
---
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
