From dfb5814036358afa44670f377ef9768f782753a1 Mon Sep 17 00:00:00 2001 From: Mike Palmiotto Date: Wed, 11 Mar 2020 00:24:42 +0000 Subject: [PATCH v2 05/12] Add PgstatCollectorType to subprocess struct Introduce two new fields to handle subprocess control: 1) needs_shmem controls whether or not the subprocess needs to attach to shmem. Up until now, all subprocesses needed to attach. This field paves the way for easier handling of subprocesses which do not. 2) fork_prep allows StartSubprocess to do some preparation for fork/exec. This is necessary to split out any functionality that previously happened in Start functions prior to fork/exec. --- src/backend/postmaster/pgstat.c | 86 +++-------------------------- src/backend/postmaster/postmaster.c | 39 +++++++------ src/backend/postmaster/subprocess.c | 28 ++++++++++ src/include/pgstat.h | 4 +- src/include/postmaster/subprocess.h | 8 ++- 5 files changed, 65 insertions(+), 100 deletions(-) diff --git a/src/backend/postmaster/pgstat.c b/src/backend/postmaster/pgstat.c index f9287b7942..6d4f2ace56 100644 --- a/src/backend/postmaster/pgstat.c +++ b/src/backend/postmaster/pgstat.c @@ -48,9 +48,9 @@ #include "pg_trace.h" #include "pgstat.h" #include "postmaster/autovacuum.h" -#include "postmaster/fork_process.h" #include "postmaster/interrupt.h" #include "postmaster/postmaster.h" +#include "postmaster/subprocess.h" #include "replication/walsender.h" #include "storage/backendid.h" #include "storage/dsm.h" @@ -275,11 +275,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_beshutdown_hook(int code, Datum arg); static PgStat_StatDBEntry *pgstat_get_db_entry(Oid databaseid, bool create); @@ -685,46 +680,15 @@ pgstat_reset_all(void) pgstat_reset_remove_files(PGSTAT_STAT_PERMANENT_DIRECTORY); } -#ifdef EXEC_BACKEND - /* - * pgstat_forkexec() - + * PgstatCollectorPrep * - * 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() - - * - * 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. + * Called from StartSubprocess to prepare a new collector fork */ int -pgstat_start(void) +PgstatCollectorPrep(int argc, char *argv[]) { time_t curtime; - pid_t pgStatPid; /* * Check that the socket is there, else pgstat_init failed and we can do @@ -740,46 +704,10 @@ pgstat_start(void) * from the postmaster main loop, we will get another chance later. */ curtime = time(NULL); - if ((unsigned int) (curtime - last_pgstat_start_time) < + if ((unsigned int) (curtime - last_pgstat_start_time) >= (unsigned int) PGSTAT_RESTART_INTERVAL) - return 0; - last_pgstat_start_time = curtime; + 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; -#endif - - default: - return (int) pgStatPid; - } - - /* shouldn't get here */ return 0; } @@ -4326,7 +4254,7 @@ pgstat_send_bgwriter(void) * The argc/argv parameters are valid only in EXEC_BACKEND case. * ---------- */ -NON_EXEC_STATIC void +void PgstatCollectorMain(int argc, char *argv[]) { int len; diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c index 21bbbf8194..e2ad879452 100644 --- a/src/backend/postmaster/postmaster.c +++ b/src/backend/postmaster/postmaster.c @@ -1781,7 +1781,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 = StartSubprocess(PgstatCollectorType); /* If we have lost the archiver, try to start a new one. */ if (PgArchPID == 0 && PgArchStartupAllowed()) @@ -3057,7 +3057,7 @@ reaper(SIGNAL_ARGS) if (PgArchStartupAllowed() && PgArchPID == 0) PgArchPID = pgarch_start(); if (PgStatPID == 0) - PgStatPID = pgstat_start(); + PgStatPID = StartSubprocess(PgstatCollectorType); /* workers may be scheduled to start now */ maybe_start_bgworkers(); @@ -3219,7 +3219,7 @@ reaper(SIGNAL_ARGS) LogChildExit(LOG, _("statistics collector process"), pid, exitstatus); if (pmState == PM_RUN || pmState == PM_HOT_STANDBY) - PgStatPID = pgstat_start(); + PgStatPID = StartSubprocess(PgstatCollectorType); continue; } @@ -5069,12 +5069,6 @@ SubPostmasterMain(int argc, char *argv[]) PgArchiverMain(argc, argv); /* does not return */ } - if (strcmp(argv[1], "--forkcol") == 0) - { - /* Do not want to attach to shared memory */ - - PgstatCollectorMain(argc, argv); /* does not return */ - } if (strcmp(argv[1], "--forklog") == 0) { /* Do not want to attach to shared memory */ @@ -5097,14 +5091,17 @@ SubPostmasterMain(int argc, char *argv[]) } else { - /* Restore basic shared memory pointers */ - InitShmemAccess(UsedShmemSegAddr); + if (MySubprocess->needs_shmem) + { + /* Restore basic shared memory pointers */ + InitShmemAccess(UsedShmemSegAddr); - /* Need a PGPROC to run CreateSharedMemoryAndSemaphores */ - InitProcess(); + /* Need a PGPROC to run CreateSharedMemoryAndSemaphores */ + InitProcess(); - /* Attach process to shared data structures */ - CreateSharedMemoryAndSemaphores(); + /* Attach process to shared data structures */ + CreateSharedMemoryAndSemaphores(); + } MySubprocess->entrypoint(argc - 2, argv + 2); } @@ -5224,7 +5221,7 @@ sigusr1_handler(SIGNAL_ARGS) * Likewise, start other special children as needed. */ Assert(PgStatPID == 0); - PgStatPID = pgstat_start(); + PgStatPID = StartSubprocess(PgstatCollectorType); ereport(LOG, (errmsg("database system is ready to accept read only connections"))); @@ -5475,6 +5472,16 @@ StartSubprocess(SubprocessType type) MemoryContextDelete(PostmasterContext); PostmasterContext = NULL; + /* + * Drop connection to postmaster's shared memory if the subprocess + * doesn't need it. + */ + if (!MySubprocess->needs_shmem) + { + dsm_detach_all(); + PGSharedMemoryDetach(); + } + if (MySubprocess->needs_aux_proc) AuxiliaryProcessMain(argc, argv); else diff --git a/src/backend/postmaster/subprocess.c b/src/backend/postmaster/subprocess.c index 0fbd067310..ee44ef0789 100644 --- a/src/backend/postmaster/subprocess.c +++ b/src/backend/postmaster/subprocess.c @@ -12,6 +12,7 @@ */ #include "postgres.h" #include "bootstrap/bootstrap.h" +#include "pgstat.h" #include "postmaster/autovacuum.h" #include "postmaster/bgwriter.h" #include "postmaster/postmaster.h" @@ -28,6 +29,8 @@ static PgSubprocess process_types[] = { .name = "boot", .desc = "checker", .needs_aux_proc = true, + .needs_shmem = true, + .fork_prep = NULL, .entrypoint = CheckerModeMain, .fork_failure = NULL }, @@ -35,6 +38,8 @@ static PgSubprocess process_types[] = { .name = "boot", .desc = "bootstrap", .needs_aux_proc = true, + .needs_shmem = true, + .fork_prep = NULL, .entrypoint = BootstrapModeMain, .fork_failure = NULL }, @@ -42,6 +47,8 @@ static PgSubprocess process_types[] = { .name = "boot", .desc = "startup", .needs_aux_proc = true, + .needs_shmem = true, + .fork_prep = NULL, .entrypoint = StartupProcessMain, .fork_failure = StartupForkFailure }, @@ -49,6 +56,8 @@ static PgSubprocess process_types[] = { .name = "boot", .desc = "background writer", .needs_aux_proc = true, + .needs_shmem = true, + .fork_prep = NULL, .entrypoint = BackgroundWriterMain, .fork_failure = NULL }, @@ -56,6 +65,8 @@ static PgSubprocess process_types[] = { .name = "boot", .desc = "checkpointer", .needs_aux_proc = true, + .needs_shmem = true, + .fork_prep = NULL, .entrypoint = CheckpointerMain, .fork_failure = NULL }, @@ -63,6 +74,8 @@ static PgSubprocess process_types[] = { .name = "boot", .desc = "wal writer", .needs_aux_proc = true, + .needs_shmem = true, + .fork_prep = NULL, .entrypoint = WalWriterMain, .fork_failure = NULL }, @@ -70,6 +83,8 @@ static PgSubprocess process_types[] = { .name = "boot", .desc = "wal receiver", .needs_aux_proc = true, + .needs_shmem = true, + .fork_prep = NULL, .entrypoint = WalReceiverMain, .fork_failure = NULL }, @@ -77,6 +92,8 @@ static PgSubprocess process_types[] = { .name = "avlauncher", .desc = "autovacuum launcher", .needs_aux_proc = false, + .needs_shmem = true, + .fork_prep = NULL, .entrypoint = AutoVacLauncherMain, .fork_failure = NULL }, @@ -84,8 +101,19 @@ static PgSubprocess process_types[] = { .name = "avworker", .desc = "autovacuum worker", .needs_aux_proc = false, + .needs_shmem = true, + .fork_prep = NULL, .entrypoint = AutoVacWorkerMain, .fork_failure = NULL + }, + { + .name = "col", + .desc = "statistics collector", + .needs_aux_proc = false, + .needs_shmem = false, + .fork_prep = PgstatCollectorPrep, + .entrypoint = PgstatCollectorMain, + .fork_failure = NULL } }; diff --git a/src/include/pgstat.h b/src/include/pgstat.h index 1a19921f80..92943f5cff 100644 --- a/src/include/pgstat.h +++ b/src/include/pgstat.h @@ -1224,10 +1224,8 @@ extern int pgstat_start(void); extern void pgstat_reset_all(void); extern void allow_immediate_pgstat_restart(void); -#ifdef EXEC_BACKEND +extern int PgstatCollectorPrep(int argc, char *argv[]); extern void PgstatCollectorMain(int argc, char *argv[]) pg_attribute_noreturn(); -#endif - /* ---------- * Functions called from backends diff --git a/src/include/postmaster/subprocess.h b/src/include/postmaster/subprocess.h index e11c24cc62..a7ab036b7b 100644 --- a/src/include/postmaster/subprocess.h +++ b/src/include/postmaster/subprocess.h @@ -25,12 +25,14 @@ typedef enum WalReceiverType, /* end of Auxiliary Process Forks */ AutoVacuumLauncherType, AutoVacuumWorkerType, + PgstatCollectorType, NUMSUBPROCESSTYPES /* Must be last! */ } SubprocessType; -typedef void (*SubprocessEntryPoint) (int argc, char *argv[]); -typedef bool (*SubprocessForkFailure) (int fork_errno); +typedef int (*SubprocessPrep) (int argc, char *argv[]); +typedef void (*SubprocessEntryPoint) (int argc, char *argv[]); +typedef bool (*SubprocessForkFailure) (int fork_errno); /* Current subprocess initializer */ extern void InitMySubprocess(SubprocessType type); @@ -40,6 +42,8 @@ typedef struct PgSubprocess const char *name; const char *desc; bool needs_aux_proc; + bool needs_shmem; + SubprocessPrep fork_prep; SubprocessEntryPoint entrypoint; SubprocessForkFailure fork_failure; } PgSubprocess; -- 2.17.0