From efc6f0531b71847c6500062b5a0fe43331a6ea7e Mon Sep 17 00:00:00 2001 From: Mike Palmiotto Date: Fri, 27 Sep 2019 19:13:53 -0400 Subject: [PATCH 3/8] Add centralized stat collector --- src/backend/postmaster/pgstat.c | 88 ++++++--------------------- src/backend/postmaster/postmaster.c | 5 +- src/include/pgstat.h | 4 +- src/include/postmaster/fork_process.h | 1 + 4 files changed, 26 insertions(+), 72 deletions(-) diff --git a/src/backend/postmaster/pgstat.c b/src/backend/postmaster/pgstat.c index 011076c3e3..73d953aedf 100644 --- a/src/backend/postmaster/pgstat.c +++ b/src/backend/postmaster/pgstat.c @@ -280,10 +280,6 @@ static instr_time total_func_time; * Local function forward declarations * ---------- */ -#ifdef EXEC_BACKEND -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_beshutdown_hook(int code, Datum arg); @@ -689,53 +685,26 @@ pgstat_reset_all(void) pgstat_reset_remove_files(PGSTAT_STAT_PERMANENT_DIRECTORY); } -#ifdef EXEC_BACKEND /* - * pgstat_forkexec() - - * - * Format up the arglist for, then fork and exec, statistics collector process - */ -static pid_t -pgstat_forkexec(void) -{ - char *av[10]; - int ac = 0; - - av[ac++] = "postgres"; - av[ac++] = "--forkcol"; - av[ac++] = NULL; /* filled in by postmaster_forkexec */ - - av[ac] = NULL; - Assert(ac < lengthof(av)); - - return postmaster_forkexec(ac, av); -} -#endif /* EXEC_BACKEND */ - - -/* - * pgstat_start() - + * PrepPgstatCollectorFork * * Called from postmaster at startup or after an existing collector * died. Attempt to fire up a fresh statistics collector. * - * Returns PID of child process, or 0 if fail. - * - * Note: if fail, we will be called again from the postmaster main loop. */ -int -pgstat_start(void) +void +PrepPgstatCollectorFork(ForkProcData *pgstat_fork) { + int ac = 0; time_t curtime; - pid_t pgStatPid; /* * Check that the socket is there, else pgstat_init failed and we can do * nothing useful. */ if (pgStatSock == PGINVALID_SOCKET) - return 0; + return; /* * Do nothing if too soon since last collector start. This is a safety @@ -746,45 +715,20 @@ pgstat_start(void) curtime = time(NULL); if ((unsigned int) (curtime - last_pgstat_start_time) < (unsigned int) PGSTAT_RESTART_INTERVAL) - return 0; + return; last_pgstat_start_time = curtime; - /* - * Okay, fork off the collector. - */ #ifdef EXEC_BACKEND - switch ((pgStatPid = pgstat_forkexec())) -#else - switch ((pgStatPid = fork_process())) -#endif - { - case -1: - ereport(LOG, - (errmsg("could not fork statistics collector: %m"))); - return 0; - -#ifndef EXEC_BACKEND - case 0: - /* in postmaster child ... */ - InitPostmasterChild(); - - /* Close the postmaster's sockets */ - ClosePostmasterPorts(false); - - /* Drop our connection to postmaster's shared memory, as well */ - dsm_detach_all(); - PGSharedMemoryDetach(); - - PgstatCollectorMain(0, NULL); - break; + pgstat_fork->av[ac++] = pstrdup("postgres"); + pgstat_fork->av[ac++] = pstrdup("--forkcol"); + pgstat_fork->av[ac++] = NULL; /* filled in by postmaster_forkexec */ #endif - default: - return (int) pgStatPid; - } + pgstat_fork->ac = ac; + Assert(pgstat_fork->ac < lengthof(*pgstat_fork->av)); - /* shouldn't get here */ - return 0; + pgstat_fork->type_desc = pstrdup("statistics collector"); + pgstat_fork->child_main = PgstatCollectorMain; } void @@ -4425,12 +4369,16 @@ pgstat_send_bgwriter(void) * ---------- */ NON_EXEC_STATIC void -PgstatCollectorMain(int argc, char *argv[]) +PgstatCollectorMain(pg_attribute_unused() int argc, pg_attribute_unused() char *argv[]) { int len; PgStat_Msg msg; int wr; + /* Drop our connection to postmaster's shared memory, as well */ + dsm_detach_all(); + PGSharedMemoryDetach(); + /* * Ignore all signals usually bound to some action in the postmaster, * except SIGHUP and SIGQUIT. Note we don't need a SIGUSR1 handler to diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c index 35cd1479b9..37a36387a3 100644 --- a/src/backend/postmaster/postmaster.c +++ b/src/backend/postmaster/postmaster.c @@ -549,6 +549,7 @@ static void ShmemBackendArrayRemove(Backend *bn); #define StartWalReceiver() StartChildProcess(WalReceiverFork) #define StartAutoVacLauncher() StartChildProcess(AutoVacLauncherFork) #define StartAutoVacWorker() StartChildProcess(AutoVacWorkerFork) +#define pgstat_start() StartChildProcess(PgstatCollectorFork) /* Macros to check exit status of a child process */ #define EXIT_STATUS_0(st) ((st) == 0) @@ -5459,7 +5460,9 @@ StartChildProcess(ForkProcType type) case AutoVacWorkerFork: PrepAutoVacProcessFork(fork_data); break; - + case PgstatCollectorFork: + PrepPgstatCollectorFork(fork_data); + break; default: break; } diff --git a/src/include/pgstat.h b/src/include/pgstat.h index fe076d823d..00e95caa60 100644 --- a/src/include/pgstat.h +++ b/src/include/pgstat.h @@ -15,6 +15,7 @@ #include "libpq/pqcomm.h" #include "port/atomics.h" #include "portability/instr_time.h" +#include "postmaster/fork_process.h" #include "postmaster/pgarch.h" #include "storage/proc.h" #include "utils/hsearch.h" @@ -1235,10 +1236,11 @@ extern Size BackendStatusShmemSize(void); extern void CreateSharedBackendStatus(void); extern void pgstat_init(void); -extern int pgstat_start(void); extern void pgstat_reset_all(void); extern void allow_immediate_pgstat_restart(void); +extern void PrepPgstatCollectorFork(ForkProcData *pgstat_fork); + #ifdef EXEC_BACKEND extern void PgstatCollectorMain(int argc, char *argv[]) pg_attribute_noreturn(); #endif diff --git a/src/include/postmaster/fork_process.h b/src/include/postmaster/fork_process.h index 1f319fc98f..16ca8be968 100644 --- a/src/include/postmaster/fork_process.h +++ b/src/include/postmaster/fork_process.h @@ -26,6 +26,7 @@ typedef enum WalReceiverFork, /* end of Auxiliary Process Forks */ AutoVacLauncherFork, AutoVacWorkerFork, + PgstatCollectorFork, NUMFORKPROCTYPES /* Must be last! */ } ForkProcType; -- 2.23.0