From 29ad0c5dda308eda447fa48eb920131cd818a1e4 Mon Sep 17 00:00:00 2001 From: Kyotaro Horiguchi Date: Fri, 29 Jun 2018 16:58:32 +0900 Subject: [PATCH 2/3] Change stats collector to an axiliary process. Shared memory and LWLocks are required to let stats collector use dshash. This patch makes stats collector an auxiliary process. --- src/backend/bootstrap/bootstrap.c | 8 +++++ src/backend/postmaster/pgstat.c | 58 +++++++++++++++++++++++++------------ src/backend/postmaster/postmaster.c | 24 +++++++++------ src/include/miscadmin.h | 3 +- src/include/pgstat.h | 6 +++- 5 files changed, 70 insertions(+), 29 deletions(-) diff --git a/src/backend/bootstrap/bootstrap.c b/src/backend/bootstrap/bootstrap.c index 7e34bee63e..8f8327495a 100644 --- a/src/backend/bootstrap/bootstrap.c +++ b/src/backend/bootstrap/bootstrap.c @@ -335,6 +335,9 @@ AuxiliaryProcessMain(int argc, char *argv[]) case WalReceiverProcess: statmsg = pgstat_get_backend_desc(B_WAL_RECEIVER); break; + case StatsCollectorProcess: + statmsg = pgstat_get_backend_desc(B_STATS_COLLECTOR); + break; default: statmsg = "??? process"; break; @@ -462,6 +465,11 @@ AuxiliaryProcessMain(int argc, char *argv[]) WalReceiverMain(); proc_exit(1); /* should never return */ + case StatsCollectorProcess: + /* don't set signals, stats collector has its own agenda */ + PgstatCollectorMain(); + proc_exit(1); /* should never return */ + default: elog(PANIC, "unrecognized process type: %d", (int) MyAuxProcType); proc_exit(1); diff --git a/src/backend/postmaster/pgstat.c b/src/backend/postmaster/pgstat.c index bbe73618c7..c09d837afd 100644 --- a/src/backend/postmaster/pgstat.c +++ b/src/backend/postmaster/pgstat.c @@ -267,6 +267,7 @@ static List *pending_write_requests = NIL; /* Signal handler flags */ static volatile bool need_exit = false; static volatile bool got_SIGHUP = false; +static volatile bool got_SIGTERM = false; /* * Total time charged to functions so far in the current backend. @@ -284,8 +285,8 @@ static instr_time total_func_time; static pid_t pgstat_forkexec(void); #endif -NON_EXEC_STATIC void PgstatCollectorMain(int argc, char *argv[]) pg_attribute_noreturn(); -static void pgstat_exit(SIGNAL_ARGS); +static void pgstat_shutdown_handler(SIGNAL_ARGS); +static void pgstat_quickdie_handler(SIGNAL_ARGS); static void pgstat_beshutdown_hook(int code, Datum arg); static void pgstat_sighup_handler(SIGNAL_ARGS); @@ -770,11 +771,7 @@ pgstat_start(void) /* Close the postmaster's sockets */ ClosePostmasterPorts(false); - /* Drop our connection to postmaster's shared memory, as well */ - dsm_detach_all(); - PGSharedMemoryDetach(); - - PgstatCollectorMain(0, NULL); + PgstatCollectorMain(); break; #endif @@ -2870,6 +2867,9 @@ pgstat_bestart(void) case WalReceiverProcess: beentry->st_backendType = B_WAL_RECEIVER; break; + case StatsCollectorProcess: + beentry->st_backendType = B_STATS_COLLECTOR; + break; default: elog(FATAL, "unrecognized process type: %d", (int) MyAuxProcType); @@ -4135,6 +4135,9 @@ pgstat_get_backend_desc(BackendType backendType) case B_WAL_WRITER: backendDesc = "walwriter"; break; + case B_STATS_COLLECTOR: + backendDesc = "stats collector"; + break; } return backendDesc; @@ -4252,8 +4255,8 @@ pgstat_send_bgwriter(void) * The argc/argv parameters are valid only in EXEC_BACKEND case. * ---------- */ -NON_EXEC_STATIC void -PgstatCollectorMain(int argc, char *argv[]) +void +PgstatCollectorMain(void) { int len; PgStat_Msg msg; @@ -4266,8 +4269,8 @@ PgstatCollectorMain(int argc, char *argv[]) */ pqsignal(SIGHUP, pgstat_sighup_handler); pqsignal(SIGINT, SIG_IGN); - pqsignal(SIGTERM, SIG_IGN); - pqsignal(SIGQUIT, pgstat_exit); + pqsignal(SIGTERM, pgstat_shutdown_handler); + pqsignal(SIGQUIT, pgstat_quickdie_handler); pqsignal(SIGALRM, SIG_IGN); pqsignal(SIGPIPE, SIG_IGN); pqsignal(SIGUSR1, SIG_IGN); @@ -4312,14 +4315,14 @@ PgstatCollectorMain(int argc, char *argv[]) /* * Quit if we get SIGQUIT from the postmaster. */ - if (need_exit) + if (got_SIGTERM) break; /* * Inner loop iterates as long as we keep getting messages, or until * need_exit becomes set. */ - while (!need_exit) + while (!got_SIGTERM) { /* * Reload configuration if we got SIGHUP from the postmaster. @@ -4507,14 +4510,21 @@ PgstatCollectorMain(int argc, char *argv[]) /* SIGQUIT signal handler for collector process */ static void -pgstat_exit(SIGNAL_ARGS) +pgstat_quickdie_handler(SIGNAL_ARGS) { - int save_errno = errno; + PG_SETMASK(&BlockSig); - need_exit = true; - SetLatch(MyLatch); + /* + * We DO NOT want to run proc_exit() callbacks -- we're here because + * shared memory may be corrupted, so we don't want to try to clean up our + * transaction. Just nail the windows shut and get out of town. Now that + * there's an atexit callback to prevent third-party code from breaking + * things by calling exit() directly, we have to reset the callbacks + * explicitly to make this work as intended. + */ + on_exit_reset(); - errno = save_errno; + exit(2); } /* SIGHUP handler for collector process */ @@ -4529,6 +4539,18 @@ pgstat_sighup_handler(SIGNAL_ARGS) errno = save_errno; } +static void +pgstat_shutdown_handler(SIGNAL_ARGS) +{ + int save_errno = errno; + + got_SIGTERM = true; + + SetLatch(MyLatch); + + errno = save_errno; +} + /* * Subroutine to clear stats in a database entry * diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c index a4b53b33cd..a6209cd749 100644 --- a/src/backend/postmaster/postmaster.c +++ b/src/backend/postmaster/postmaster.c @@ -145,7 +145,8 @@ #define BACKEND_TYPE_AUTOVAC 0x0002 /* autovacuum worker process */ #define BACKEND_TYPE_WALSND 0x0004 /* walsender process */ #define BACKEND_TYPE_BGWORKER 0x0008 /* bgworker process */ -#define BACKEND_TYPE_ALL 0x000F /* OR of all the above */ +#define BACKEND_TYPE_STATS 0x0010 /* bgworker process */ +#define BACKEND_TYPE_ALL 0x001F /* OR of all the above */ #define BACKEND_TYPE_WORKER (BACKEND_TYPE_AUTOVAC | BACKEND_TYPE_BGWORKER) @@ -551,6 +552,7 @@ static void ShmemBackendArrayRemove(Backend *bn); #define StartCheckpointer() StartChildProcess(CheckpointerProcess) #define StartWalWriter() StartChildProcess(WalWriterProcess) #define StartWalReceiver() StartChildProcess(WalReceiverProcess) +#define StartStatsCollector() StartChildProcess(StatsCollectorProcess) /* Macros to check exit status of a child process */ #define EXIT_STATUS_0(st) ((st) == 0) @@ -1760,7 +1762,7 @@ ServerLoop(void) /* If we have lost the stats collector, try to start a new one */ if (PgStatPID == 0 && (pmState == PM_RUN || pmState == PM_HOT_STANDBY)) - PgStatPID = pgstat_start(); + PgStatPID = StartStatsCollector(); /* If we have lost the archiver, try to start a new one. */ if (PgArchPID == 0 && PgArchStartupAllowed()) @@ -2878,7 +2880,7 @@ reaper(SIGNAL_ARGS) if (PgArchStartupAllowed() && PgArchPID == 0) PgArchPID = pgarch_start(); if (PgStatPID == 0) - PgStatPID = pgstat_start(); + PgStatPID = StartStatsCollector(); /* workers may be scheduled to start now */ maybe_start_bgworkers(); @@ -2951,7 +2953,7 @@ reaper(SIGNAL_ARGS) * nothing left for it to do. */ if (PgStatPID != 0) - signal_child(PgStatPID, SIGQUIT); + signal_child(PgStatPID, SIGTERM); } else { @@ -3037,10 +3039,10 @@ reaper(SIGNAL_ARGS) { PgStatPID = 0; if (!EXIT_STATUS_0(exitstatus)) - LogChildExit(LOG, _("statistics collector process"), - pid, exitstatus); + HandleChildCrash(pid, exitstatus, + _("statistics collector process")); if (pmState == PM_RUN || pmState == PM_HOT_STANDBY) - PgStatPID = pgstat_start(); + PgStatPID = StartStatsCollector(); continue; } @@ -3270,7 +3272,7 @@ CleanupBackend(int pid, /* * HandleChildCrash -- cleanup after failed backend, bgwriter, checkpointer, - * walwriter, autovacuum, or background worker. + * walwriter, autovacuum, stats collector or background worker. * * The objectives here are to clean up our local state about the child * process, and to signal all other remaining children to quickdie. @@ -5071,7 +5073,7 @@ sigusr1_handler(SIGNAL_ARGS) * Likewise, start other special children as needed. */ Assert(PgStatPID == 0); - PgStatPID = pgstat_start(); + PgStatPID = StartStatsCollector(); ereport(LOG, (errmsg("database system is ready to accept read only connections"))); @@ -5361,6 +5363,10 @@ StartChildProcess(AuxProcType type) ereport(LOG, (errmsg("could not fork WAL receiver process: %m"))); break; + case StatsCollectorProcess: + ereport(LOG, + (errmsg("could not fork stats collector process: %m"))); + break; default: ereport(LOG, (errmsg("could not fork process: %m"))); diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h index e167ee8fcb..53b260cb1f 100644 --- a/src/include/miscadmin.h +++ b/src/include/miscadmin.h @@ -400,7 +400,7 @@ typedef enum CheckpointerProcess, WalWriterProcess, WalReceiverProcess, - + StatsCollectorProcess, NUM_AUXPROCTYPES /* Must be last! */ } AuxProcType; @@ -412,6 +412,7 @@ extern AuxProcType MyAuxProcType; #define AmCheckpointerProcess() (MyAuxProcType == CheckpointerProcess) #define AmWalWriterProcess() (MyAuxProcType == WalWriterProcess) #define AmWalReceiverProcess() (MyAuxProcType == WalReceiverProcess) +#define AmStatsCollectorProcess() (MyAuxProcType == StatsCollectorProcess) /***************************************************************************** diff --git a/src/include/pgstat.h b/src/include/pgstat.h index d59c24ae23..87957e093a 100644 --- a/src/include/pgstat.h +++ b/src/include/pgstat.h @@ -710,7 +710,8 @@ typedef enum BackendType B_STARTUP, B_WAL_RECEIVER, B_WAL_SENDER, - B_WAL_WRITER + B_WAL_WRITER, + B_STATS_COLLECTOR } BackendType; @@ -1353,4 +1354,7 @@ extern int pgstat_fetch_stat_numbackends(void); extern PgStat_ArchiverStats *pgstat_fetch_stat_archiver(void); extern PgStat_GlobalStats *pgstat_fetch_global(void); +/* Main loop */ +extern void PgstatCollectorMain(void); + #endif /* PGSTAT_H */ -- 2.16.3