On Thu, Sep 26, 2019 at 6:03 PM Mike Palmiotto <mike.palmio...@crunchydata.com> wrote: > > On Thu, Sep 26, 2019 at 10:56 AM Alvaro Herrera > <alvhe...@2ndquadrant.com> wrote: > <snip> > > > > Well, I think it would be easier to manage as split patches, yeah. > > I think it'd be infrastructure that needs to be carefully reviewed, > > while the other ones are mostly boilerplate. If I were the committer > > for it, I would push that initial patch first immediately followed by > > conversion of some process that's heavily exercised in buildfarm, wait > > until lack of trouble is evident, followed by a trickle of pushes to > > adapt the other processes. > > Thanks for the feedback! I've rebased and tested on my F30 box with > and without EXEC_BACKEND. Just working on splitting out the patches > now and will post the new patchset as soon as that's done (hopefully > sometime tomorrow).
Attached is the reworked and rebased patch set. I put the hook on top and a separate commit for each process type. Note that avworker and avlauncher were intentionally left together. Let me know if you think those should be split out as well. Tested again with and without EXEC_BACKEND. Note that you'll need to set randomize_va_space to 0 to disable ASLR in order to avoid occasional failure in the EXEC_BACKEND case. Please let me know if anything else jumps out to you. Thanks, -- Mike Palmiotto https://crunchydata.com
From 205ea86dde898f7edac327d46b2b43b04721aadc Mon Sep 17 00:00:00 2001 From: Mike Palmiotto <mike.palmio...@crunchydata.com> Date: Mon, 30 Sep 2019 14:42:53 -0400 Subject: [PATCH 7/8] Add backends to process centralization --- src/backend/postmaster/postmaster.c | 309 +++++++++++++------------- src/include/postmaster/fork_process.h | 5 + src/include/postmaster/postmaster.h | 1 - 3 files changed, 160 insertions(+), 155 deletions(-) diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c index 8f862fcd64..b55cc4556d 100644 --- a/src/backend/postmaster/postmaster.c +++ b/src/backend/postmaster/postmaster.c @@ -144,6 +144,7 @@ static Backend *ShmemBackendArray; BackgroundWorker *MyBgworkerEntry = NULL; RegisteredBgWorker *CurrentBgWorker = NULL; +static Backend *MyBackend; static int child_errno; /* The socket number we are listening for connections on */ @@ -356,7 +357,6 @@ static void BackendInitialize(Port *port); static void BackendRun(Port *port) pg_attribute_noreturn(); static void ExitPostmaster(int status) pg_attribute_noreturn(); static int ServerLoop(void); -static int BackendStartup(Port *port); static int ProcessStartupPacket(Port *port, bool secure_done); static void SendNegotiateProtocolVersion(List *unrecognized_protocol_options); static void processCancelRequest(Port *port, void *pkt); @@ -409,7 +409,6 @@ typedef struct } win32_deadchild_waitinfo; #endif /* WIN32 */ -static pid_t backend_forkexec(Port *port); static pid_t internal_forkexec(int argc, char *argv[]); /* Type for a socket that can be inherited to a client process */ @@ -505,6 +504,7 @@ static void ShmemBackendArrayRemove(Backend *bn); #define pgarch_start() StartChildProcess(PgArchiverFork) #define SysLogger_Start() StartChildProcess(SysLoggerFork) #define do_start_bgworker() StartChildProcess(BgWorkerFork) +#define BackendStartup() StartChildProcess(BackendFork) /* Macros to check exit status of a child process */ #define EXIT_STATUS_0(st) ((st) == 0) @@ -524,6 +524,141 @@ HANDLE PostmasterHandle; static Port *ConnProcPort = NULL; +/* + * BackendMain + * + * Child code when forking a Backend. + */ +static void BackendMain(pg_attribute_unused() int argc, pg_attribute_unused() char *argv[]) +{ + /* + * Perform additional initialization and collect startup packet. + * + * We want to do this before InitProcess() for a couple of reasons: 1. + * so that we aren't eating up a PGPROC slot while waiting on the + * client. 2. so that if InitProcess() fails due to being out of + * PGPROC slots, we have already initialized libpq and are able to + * report the error to the client. + */ + BackendInitialize(ConnProcPort); + +#ifdef EXEC_BACKEND + shmemSetup(false); +#endif + + /* And run the backend */ + BackendRun(ConnProcPort); /* does not return */ +} + +/* + * BackendPostmasterMain + * + * Parent code when forking a Backend. + */ +static void BackendPostmasterMain(pg_attribute_unused() int argc, pg_attribute_unused() char *argv[]) +{ + /* in parent, successful fork */ + ereport(DEBUG2, + (errmsg_internal("forked new backend, pid=%d socket=%d", + (int) MyChildProcPid, (int) MyProcPort->sock))); + + /* + * Everything's been successful, it's safe to add this backend to our list + * of backends. + */ + MyBackend->pid = MyChildProcPid; + MyBackend->bkend_type = BACKEND_TYPE_NORMAL; /* Can change later to WALSND */ + dlist_push_head(&BackendList, &MyBackend->elem); + +#ifdef EXEC_BACKEND + if (!MyBackend->dead_end) + ShmemBackendArrayAdd(MyBackend); +#endif +} + +/* + * BackendFailFork + * + * Backend cleanup in case a failure occurs forking a new Backend. + */ +static void BackendFailFork(pg_attribute_unused() int argc, pg_attribute_unused() char *argv[]) +{ + if (!MyBackend->dead_end) + (void) ReleasePostmasterChildSlot(MyBackend->child_slot); + free(MyBackend); + + report_fork_failure_to_client(MyProcPort, child_errno); +} + +/* + * PrepBackendFork + * + * Prepare a ForkProcType struct for starting a Backend. + * This does all prep related to av parameters and error messages. +*/ +static void +PrepBackendFork(ForkProcData *backend_fork) +{ + int ac = 0; + + /* + * Create backend data structure. Better before the fork() so we can + * handle failure cleanly. + */ + MyBackend = (Backend *) malloc(sizeof(Backend)); + if (!MyBackend) + { + ereport(LOG, + (errcode(ERRCODE_OUT_OF_MEMORY), + errmsg("out of memory"))); + } + +#ifdef EXEC_BACKEND + backend_fork->av[ac++] = pstrdup("postgres"); + backend_fork->av[ac++] = pstrdup("--forkbackend"); + backend_fork->av[ac++] = NULL; +#endif + backend_fork->av[ac] = NULL; + backend_fork->ac = ac; + + Assert(ac < lengthof(*backend_fork->av)); + + /* + * Compute the cancel key that will be assigned to this backend. The + * backend will have its own copy in the forked-off process' value of + * MyCancelKey, so that it can transmit the key to the frontend. + */ + if (!RandomCancelKey(&MyCancelKey)) + { + free(MyBackend); + ereport(LOG, + (errcode(ERRCODE_INTERNAL_ERROR), + errmsg("could not generate random cancel key"))); + } + + MyBackend->cancel_key = MyCancelKey; + + /* Pass down canAcceptConnections state */ + ConnProcPort->canAcceptConnections = canAcceptConnections(); + MyBackend->dead_end = (ConnProcPort->canAcceptConnections != CAC_OK && + ConnProcPort->canAcceptConnections != CAC_WAITBACKUP); + + /* + * Unless it's a dead_end child, assign it a child slot number + */ + if (!MyBackend->dead_end) + MyBackend->child_slot = MyPMChildSlot = AssignPostmasterChildSlot(); + else + MyBackend->child_slot = 0; + + /* Hasn't asked to be notified about any bgworkers yet */ + MyBackend->bgworker_notify = false; + + backend_fork->postmaster_main = BackendPostmasterMain; + backend_fork->child_main = BackendMain; + backend_fork->fail_main = BackendFailFork; +} + /* * PrepAuxProcessFork * @@ -1729,7 +1864,7 @@ ServerLoop(void) ConnProcPort = ConnCreate(ListenSocket[i]); if (ConnProcPort) { - BackendStartup(ConnProcPort); + BackendStartup(); /* * We no longer need the open socket or port structure @@ -4085,122 +4220,6 @@ TerminateChildren(int signal) signal_child(PgStatPID, signal); } -/* - * BackendStartup -- start backend process - * - * returns: STATUS_ERROR if the fork failed, STATUS_OK otherwise. - * - * Note: if you change this code, also consider StartAutovacuumWorker. - */ -static int -BackendStartup(Port *port) -{ - Backend *bn; /* for backend cleanup */ - pid_t pid; - - /* - * Create backend data structure. Better before the fork() so we can - * handle failure cleanly. - */ - bn = (Backend *) malloc(sizeof(Backend)); - if (!bn) - { - ereport(LOG, - (errcode(ERRCODE_OUT_OF_MEMORY), - errmsg("out of memory"))); - return STATUS_ERROR; - } - - /* - * Compute the cancel key that will be assigned to this backend. The - * backend will have its own copy in the forked-off process' value of - * MyCancelKey, so that it can transmit the key to the frontend. - */ - if (!RandomCancelKey(&MyCancelKey)) - { - free(bn); - ereport(LOG, - (errcode(ERRCODE_INTERNAL_ERROR), - errmsg("could not generate random cancel key"))); - return STATUS_ERROR; - } - - bn->cancel_key = MyCancelKey; - - /* Pass down canAcceptConnections state */ - port->canAcceptConnections = canAcceptConnections(); - bn->dead_end = (port->canAcceptConnections != CAC_OK && - port->canAcceptConnections != CAC_WAITBACKUP); - - /* - * Unless it's a dead_end child, assign it a child slot number - */ - if (!bn->dead_end) - bn->child_slot = MyPMChildSlot = AssignPostmasterChildSlot(); - else - bn->child_slot = 0; - - /* Hasn't asked to be notified about any bgworkers yet */ - bn->bgworker_notify = false; - -#ifdef EXEC_BACKEND - pid = backend_forkexec(port); -#else /* !EXEC_BACKEND */ - pid = fork_process(); - if (pid == 0) /* child */ - { - free(bn); - - /* Detangle from postmaster */ - InitPostmasterChild(); - - /* Close the postmaster's sockets */ - ClosePostmasterPorts(false); - - /* Perform additional initialization and collect startup packet */ - BackendInitialize(port); - - /* And run the backend */ - BackendRun(port); - } -#endif /* EXEC_BACKEND */ - - if (pid < 0) - { - /* in parent, fork failed */ - int save_errno = errno; - - if (!bn->dead_end) - (void) ReleasePostmasterChildSlot(bn->child_slot); - free(bn); - errno = save_errno; - ereport(LOG, - (errmsg("could not fork new process for connection: %m"))); - report_fork_failure_to_client(port, save_errno); - return STATUS_ERROR; - } - - /* in parent, successful fork */ - ereport(DEBUG2, - (errmsg_internal("forked new backend, pid=%d socket=%d", - (int) pid, (int) port->sock))); - - /* - * Everything's been successful, it's safe to add this backend to our list - * of backends. - */ - bn->pid = pid; - bn->bkend_type = BACKEND_TYPE_NORMAL; /* Can change later to WALSND */ - dlist_push_head(&BackendList, &bn->elem); - -#ifdef EXEC_BACKEND - if (!bn->dead_end) - ShmemBackendArrayAdd(bn); -#endif - - return STATUS_OK; -} - /* * Try to report backend fork() failure to client before we close the * connection. Since we do not care to risk blocking the postmaster on @@ -4440,7 +4459,7 @@ BackendRun(Port *port) maxac * sizeof(char *)); ac = 0; - av[ac++] = "postgres"; + av[ac++] = pstrdup("postgres"); /* * Pass any backend switches specified with -o on the postmaster's own @@ -4489,38 +4508,13 @@ BackendRun(Port *port) * child process. */ pid_t -postmaster_forkexec(int argc, char *argv[]) +postmaster_forkexec(ForkProcData *fork_data) { /* This entry point passes dummy values for the Port variables */ if (!ConnProcPort) ConnProcPort = palloc0(sizeof(*ConnProcPort)); - return internal_forkexec(argc, argv); -} - -/* - * backend_forkexec -- fork/exec off a backend process - * - * Some operating systems (WIN32) don't have fork() so we have to simulate - * it by storing parameters that need to be passed to the child and - * then create a new child process. - * - * returns the pid of the fork/exec'd process, or -1 on failure - */ -static pid_t -backend_forkexec(Port *port) -{ - char *av[4]; - int ac = 0; - - av[ac++] = "postgres"; - av[ac++] = "--forkbackend"; - av[ac++] = NULL; /* filled in by internal_forkexec */ - - av[ac] = NULL; - Assert(ac < lengthof(av)); - - return internal_forkexec(ac, av); + return internal_forkexec(fork_data->ac, fork_data->av); } #ifndef WIN32 @@ -5413,6 +5407,13 @@ StartChildProcess(ForkProcType type) break; case PgArchiverFork: PrepPgArchiverFork(fork_data); + break; + case SysLoggerFork: + if (!Logging_collector) + return 0; + + PrepSysLoggerFork(fork_data); + break; case BgWorkerFork: if (PrepBgWorkerFork(fork_data)) { @@ -5422,11 +5423,8 @@ StartChildProcess(ForkProcType type) return -1; } break; - case SysLoggerFork: - if (!Logging_collector) - return 0; - - PrepSysLoggerFork(fork_data); + case BackendFork: + PrepBackendFork(fork_data); break; default: break; @@ -5439,7 +5437,7 @@ StartChildProcess(ForkProcType type) return 0; #ifdef EXEC_BACKEND - pid = postmaster_forkexec(fork_data->ac, fork_data->av); + pid = postmaster_forkexec(fork_data); #else pid = fork_process(); #endif @@ -5453,10 +5451,11 @@ StartChildProcess(ForkProcType type) InitPostmasterChild(); /* Release postmaster's working memory context */ - if (type != SysLoggerFork) + if (type != SysLoggerFork && type != BackendFork) { /* Close the postmaster's sockets */ ClosePostmasterPorts(false); + if (type == BgWorkerFork) { /* @@ -5504,6 +5503,7 @@ StartChildProcess(ForkProcType type) ExitPostmaster(1); break; case BgWorkerFork: + case BackendFork: fork_data->fail_main(fork_data->ac, fork_data->av); return -1; default: @@ -5518,6 +5518,7 @@ StartChildProcess(ForkProcType type) { case SysLoggerFork: case BgWorkerFork: + case BackendFork: fork_data->postmaster_main(fork_data->ac, fork_data->av); break; default: diff --git a/src/include/postmaster/fork_process.h b/src/include/postmaster/fork_process.h index b5d5457ce7..631a2113b5 100644 --- a/src/include/postmaster/fork_process.h +++ b/src/include/postmaster/fork_process.h @@ -30,6 +30,7 @@ typedef enum PgArchiverFork, SysLoggerFork, BgWorkerFork, + BackendFork, NUMFORKPROCTYPES /* Must be last! */ } ForkProcType; @@ -53,4 +54,8 @@ typedef struct ForkProcData extern pid_t fork_process(void); +#ifdef EXEC_BACKEND +extern pid_t postmaster_forkexec(ForkProcData *fork_data); +#endif + #endif /* FORK_PROCESS_H */ diff --git a/src/include/postmaster/postmaster.h b/src/include/postmaster/postmaster.h index 9962bdd4be..a52952b74e 100644 --- a/src/include/postmaster/postmaster.h +++ b/src/include/postmaster/postmaster.h @@ -92,7 +92,6 @@ extern bool PostmasterMarkPIDForWorkerNotify(int); extern bool RandomCancelKey(int32 *cancel_key); #ifdef EXEC_BACKEND -extern pid_t postmaster_forkexec(int argc, char *argv[]); extern void SubPostmasterMain(int argc, char *argv[]) pg_attribute_noreturn(); extern Size ShmemBackendArraySize(void); -- 2.23.0
From 718c6598e9e02e1ce9ade99afb3c8948c67ef5ae Mon Sep 17 00:00:00 2001 From: Mike Palmiotto <mike.palmio...@crunchydata.com> Date: Tue, 19 Feb 2019 15:29:33 +0000 Subject: [PATCH 8/8] Add a hook to allow extensions to set worker metadata This patch implements a hook that allows extensions to modify a worker process' metadata in backends. Additional work done by Yuli Khodorkovskiy <y...@crunchydata.com> Original discussion here: https://www.postgresql.org/message-id/flat/CAMN686FE0OdZKp9YPO%3DhtC6LnA6aW4r-%2Bjq%3D3Q5RAoFQgW8EtA%40mail.gmail.com --- src/backend/postmaster/fork_process.c | 11 +++++++++++ src/include/postmaster/fork_process.h | 4 ++++ 2 files changed, 15 insertions(+) diff --git a/src/backend/postmaster/fork_process.c b/src/backend/postmaster/fork_process.c index a9138f589e..964c04d846 100644 --- a/src/backend/postmaster/fork_process.c +++ b/src/backend/postmaster/fork_process.c @@ -21,6 +21,14 @@ #include <openssl/rand.h> #endif +/* + * This hook allows plugins to get control of the child process following + * a successful fork_process(). It can be used to perform some action in + * extensions to set metadata in backends (including special backends) upon + * setting of the session user. + */ +ForkProcess_post_hook_type ForkProcess_post_hook = NULL; + #ifndef WIN32 /* * Wrapper for fork(). Return values are the same as those for fork(): @@ -113,6 +121,9 @@ fork_process(void) #ifdef USE_OPENSSL RAND_cleanup(); #endif + + if (ForkProcess_post_hook) + (*ForkProcess_post_hook) (); } return result; diff --git a/src/include/postmaster/fork_process.h b/src/include/postmaster/fork_process.h index 631a2113b5..d756fcead7 100644 --- a/src/include/postmaster/fork_process.h +++ b/src/include/postmaster/fork_process.h @@ -58,4 +58,8 @@ extern pid_t fork_process(void); extern pid_t postmaster_forkexec(ForkProcData *fork_data); #endif +/* Hook for plugins to get control after a successful fork_process() */ +typedef void (*ForkProcess_post_hook_type) (); +extern PGDLLIMPORT ForkProcess_post_hook_type ForkProcess_post_hook; + #endif /* FORK_PROCESS_H */ -- 2.23.0
From ad1004aea268d329d4ad03a73fc817f51449aa5b Mon Sep 17 00:00:00 2001 From: Mike Palmiotto <mike.palmio...@crunchydata.com> Date: Sun, 29 Sep 2019 17:17:16 -0400 Subject: [PATCH 6/8] Add bgworker to process centralization --- src/backend/postmaster/bgworker.c | 138 ++++++++- src/backend/postmaster/postmaster.c | 316 +++++--------------- src/include/miscadmin.h | 1 + src/include/postmaster/bgworker.h | 4 + src/include/postmaster/bgworker_internals.h | 3 +- src/include/postmaster/fork_process.h | 1 + src/include/postmaster/postmaster.h | 53 ++++ 7 files changed, 269 insertions(+), 247 deletions(-) diff --git a/src/backend/postmaster/bgworker.c b/src/backend/postmaster/bgworker.c index b66b517aca..8c60a13cd1 100644 --- a/src/backend/postmaster/bgworker.c +++ b/src/backend/postmaster/bgworker.c @@ -36,6 +36,7 @@ #include "utils/ascii.h" #include "utils/ps_status.h" #include "utils/timeout.h" +#include "utils/timestamp.h" /* * The postmaster's list of registered background workers, in private memory. @@ -134,7 +135,110 @@ static const struct /* Private functions. */ static bgworker_main_type LookupBackgroundWorkerFunction(const char *libraryname, const char *funcname); +static bool assign_backendlist_entry(RegisteredBgWorker *rw); +NON_EXEC_STATIC void BackgroundWorkerFailFork(int argc, char *argv[]); +NON_EXEC_STATIC void BackgroundWorkerPostmasterMain(int argc, char *argv[]); +/* + * Allocate the Backend struct for a connected background worker, but don't + * add it to the list of backends just yet. + * + * On failure, return false without changing any worker state. + * + * Some info from the Backend is copied into the passed rw. + */ +static bool +assign_backendlist_entry(RegisteredBgWorker *rw) +{ + Backend *bn; + + /* + * Compute the cancel key that will be assigned to this session. We + * probably don't need cancel keys for background workers, but we'd better + * have something random in the field to prevent unfriendly people from + * sending cancels to them. + */ + if (!RandomCancelKey(&MyCancelKey)) + { + ereport(LOG, + (errcode(ERRCODE_INTERNAL_ERROR), + errmsg("could not generate random cancel key"))); + return false; + } + + bn = malloc(sizeof(Backend)); + if (bn == NULL) + { + ereport(LOG, + (errcode(ERRCODE_OUT_OF_MEMORY), + errmsg("out of memory"))); + return false; + } + + bn->cancel_key = MyCancelKey; + bn->child_slot = MyPMChildSlot = AssignPostmasterChildSlot(); + bn->bkend_type = BACKEND_TYPE_BGWORKER; + bn->dead_end = false; + bn->bgworker_notify = false; + + rw->rw_backend = bn; + rw->rw_child_slot = bn->child_slot; + + return true; +} + +/* + * PrepBgWorkerFork + * + * Postmaster subroutine to prepare a bgworker subprocess. + * + * Returns 0 on success or -1 on failure. + * + * Note: if fail, we will be called again from the postmaster main loop. + */ +int +PrepBgWorkerFork(ForkProcData *bgworker_fork) +{ + int ac = 0; + RegisteredBgWorker *rw = CurrentBgWorker; + + Assert(CurrentBgWorker); + Assert(rw->rw_pid == 0); + + /* + * Allocate and assign the Backend element. Note we must do this before + * forking, so that we can handle out of memory properly. + * + * Treat failure as though the worker had crashed. That way, the + * postmaster will wait a bit before attempting to start it again; if it + * tried again right away, most likely it'd find itself repeating the + * out-of-memory or fork failure condition. + */ + if (!assign_backendlist_entry(rw)) + { + rw->rw_crashed_at = GetCurrentTimestamp(); + return -1; + } + + ereport(DEBUG1, + (errmsg("starting background worker process \"%s\"", + rw->rw_worker.bgw_name))); + +#ifdef EXEC_BACKEND + bgworker_fork->av[ac++] = pstrdup("postgres"); + bgworker_fork->av[ac++] = psprintf("--forkbgworker=%d", rw->rw_shmem_slot); + bgworker_fork->av[ac++] = NULL; /* filled in by postmaster_forkexec */ + bgworker_fork->av[ac] = NULL; +#endif + Assert(bgworker_fork->ac < lengthof(*bgworker_fork->av)); + bgworker_fork->ac = ac; + bgworker_fork->type_desc = pstrdup("background worker"); + bgworker_fork->child_main = BackgroundWorkerMain; + bgworker_fork->fail_main = BackgroundWorkerFailFork; + bgworker_fork->postmaster_main = BackgroundWorkerPostmasterMain; + + return 0; +} /* * Calculate shared memory needed. @@ -698,7 +802,7 @@ bgworker_sigusr1_handler(SIGNAL_ARGS) * postmaster. */ void -StartBackgroundWorker(void) +BackgroundWorkerMain(pg_attribute_unused() int argc, pg_attribute_unused() char *argv[]) { sigjmp_buf local_sigjmp_buf; BackgroundWorker *worker = MyBgworkerEntry; @@ -837,6 +941,38 @@ StartBackgroundWorker(void) proc_exit(0); } +NON_EXEC_STATIC void +BackgroundWorkerFailFork(pg_attribute_unused() int argc, pg_attribute_unused() char *argv[]) +{ + RegisteredBgWorker *rw = CurrentBgWorker; + + /* in postmaster, fork failed ... */ + ereport(LOG, + (errmsg("could not fork worker process: %m"))); + + /* undo what assign_backendlist_entry did */ + ReleasePostmasterChildSlot(rw->rw_child_slot); + rw->rw_child_slot = 0; + free(rw->rw_backend); + rw->rw_backend = NULL; + /* mark entry as crashed, so we'll try again later */ + rw->rw_crashed_at = GetCurrentTimestamp(); +} + +NON_EXEC_STATIC void +BackgroundWorkerPostmasterMain(pg_attribute_unused() int argc, pg_attribute_unused() char *argv[]) +{ + RegisteredBgWorker *rw = CurrentBgWorker; + + rw->rw_pid = MyChildProcPid; + rw->rw_backend->pid = rw->rw_pid; + ReportBackgroundWorkerPID(rw); + /* add new worker to lists of backends */ + dlist_push_head(&BackendList, &(rw->rw_backend->elem)); +#ifdef EXEC_BACKEND + ShmemBackendArrayAdd(rw->rw_backend); +#endif +} /* * Register a new static background worker. * diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c index 5d78db067f..8f862fcd64 100644 --- a/src/backend/postmaster/postmaster.c +++ b/src/backend/postmaster/postmaster.c @@ -136,62 +136,15 @@ #include "storage/spin.h" #endif - -/* - * Possible types of a backend. Beyond being the possible bkend_type values in - * struct bkend, these are OR-able request flag bits for SignalSomeChildren() - * and CountChildren(). - */ -#define BACKEND_TYPE_NORMAL 0x0001 /* normal backend */ -#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_WORKER (BACKEND_TYPE_AUTOVAC | BACKEND_TYPE_BGWORKER) - -/* - * List of active backends (or child processes anyway; we don't actually - * know whether a given child has become a backend or is still in the - * authorization phase). This is used mainly to keep track of how many - * children we have and send them appropriate signals when necessary. - * - * "Special" children such as the startup, bgwriter and autovacuum launcher - * tasks are not in this list. Autovacuum worker and walsender are in it. - * Also, "dead_end" children are in it: these are children launched just for - * the purpose of sending a friendly rejection message to a would-be client. - * We must track them because they are attached to shared memory, but we know - * they will never become live backends. dead_end children are not assigned a - * PMChildSlot. - * - * Background workers are in this list, too. - */ -typedef struct bkend -{ - pid_t pid; /* process id of backend */ - int32 cancel_key; /* cancel key for cancels for this backend */ - int child_slot; /* PMChildSlot for this backend, if any */ - - /* - * Flavor of backend or auxiliary process. Note that BACKEND_TYPE_WALSND - * backends initially announce themselves as BACKEND_TYPE_NORMAL, so if - * bkend_type is normal, you should check for a recent transition. - */ - int bkend_type; - bool dead_end; /* is it going to send an error and quit? */ - bool bgworker_notify; /* gets bgworker start/stop notifications */ - dlist_node elem; /* list link in BackendList */ -} Backend; - -static dlist_head BackendList = DLIST_STATIC_INIT(BackendList); +dlist_head BackendList = DLIST_STATIC_INIT(BackendList); #ifdef EXEC_BACKEND static Backend *ShmemBackendArray; #endif BackgroundWorker *MyBgworkerEntry = NULL; - - +RegisteredBgWorker *CurrentBgWorker = NULL; +static int child_errno; /* The socket number we are listening for connections on */ int PostPortNumber; @@ -410,7 +363,6 @@ static void processCancelRequest(Port *port, void *pkt); static int initMasks(fd_set *rmask); static void report_fork_failure_to_client(Port *port, int errnum); static CAC_state canAcceptConnections(void); -static bool RandomCancelKey(int32 *cancel_key); static void signal_child(pid_t pid, int signal); static bool SignalSomeChildren(int signal, int targets); static void TerminateChildren(int signal); @@ -421,7 +373,6 @@ static void shmemSetup(bool aux_process); #define SignalChildren(sig) SignalSomeChildren(sig, BACKEND_TYPE_ALL) static int CountChildren(int target); -static bool assign_backendlist_entry(RegisteredBgWorker *rw); static void maybe_start_bgworkers(void); static bool CreateOptsFile(int argc, char *argv[], char *fullprogname); static pid_t StartChildProcess(ForkProcType type); @@ -538,10 +489,11 @@ static bool save_backend_variables(BackendParameters *param, Port *port, HANDLE childProcess, pid_t childPid); #endif -static void ShmemBackendArrayAdd(Backend *bn); static void ShmemBackendArrayRemove(Backend *bn); #endif /* EXEC_BACKEND */ +#define BGWORKER_LEN 15 + #define StartupDataBase() StartChildProcess(StartupFork) #define StartBackgroundWriter() StartChildProcess(BgWriterFork) #define StartCheckpointer() StartChildProcess(CheckpointerFork) @@ -552,6 +504,7 @@ static void ShmemBackendArrayRemove(Backend *bn); #define pgstat_start() StartChildProcess(PgstatCollectorFork) #define pgarch_start() StartChildProcess(PgArchiverFork) #define SysLogger_Start() StartChildProcess(SysLoggerFork) +#define do_start_bgworker() StartChildProcess(BgWorkerFork) /* Macros to check exit status of a child process */ #define EXIT_STATUS_0(st) ((st) == 0) @@ -4967,7 +4920,7 @@ SubPostmasterMain(int argc, char *argv[]) strcmp(argv[1], "--forkavlauncher") == 0 || strcmp(argv[1], "--forkavworker") == 0 || strcmp(argv[1], "--forkboot") == 0 || - strncmp(argv[1], "--forkbgworker=", 15) == 0) + strncmp(argv[1], "--forkbgworker=", BGWORKER_LEN) == 0) PGSharedMemoryReAttach(); else PGSharedMemoryNoReAttach(); @@ -5082,27 +5035,20 @@ SubPostmasterMain(int argc, char *argv[]) shmemSetup(true); AutoVacWorkerMain(argc - 2, argv + 2); /* does not return */ } - if (strncmp(argv[1], "--forkbgworker=", 15) == 0) + if (strncmp(argv[1], "--forkbgworker=", BGWORKER_LEN) == 0) { int shmem_slot; /* do this as early as possible; in particular, before InitProcess() */ IsBackgroundWorker = true; - /* Restore basic shared memory pointers */ - InitShmemAccess(UsedShmemSegAddr); - - /* Need a PGPROC to run CreateSharedMemoryAndSemaphores */ - InitProcess(); - - /* Attach process to shared data structures */ - CreateSharedMemoryAndSemaphores(); + shmemSetup(false); /* Fetch MyBgworkerEntry from shared memory */ - shmem_slot = atoi(argv[1] + 15); + shmem_slot = atoi(argv[1] + BGWORKER_LEN); MyBgworkerEntry = BackgroundWorkerEntry(shmem_slot); - StartBackgroundWorker(); + BackgroundWorkerMain(argc, argv); } if (strcmp(argv[1], "--forkarch") == 0) { @@ -5377,7 +5323,7 @@ StartupPacketTimeoutHandler(void) /* * Generate a random cancel key. */ -static bool +bool RandomCancelKey(int32 *cancel_key) { return pg_strong_random(cancel_key, sizeof(int32)); @@ -5467,6 +5413,14 @@ StartChildProcess(ForkProcType type) break; case PgArchiverFork: PrepPgArchiverFork(fork_data); + case BgWorkerFork: + if (PrepBgWorkerFork(fork_data)) + { + /* We failed to assign a backendlist entry + * so tell postmaster to try again later. + */ + return -1; + } break; case SysLoggerFork: if (!Logging_collector) @@ -5490,6 +5444,9 @@ StartChildProcess(ForkProcType type) pid = fork_process(); #endif + /* some processes like backends and bgworkers need the pid */ + MyChildProcPid = pid; + if (pid == 0) /* child */ { #ifndef EXEC_BACKEND @@ -5498,8 +5455,19 @@ StartChildProcess(ForkProcType type) /* Release postmaster's working memory context */ if (type != SysLoggerFork) { - /* Close the postmaster's sockets */ - ClosePostmasterPorts(false); + /* Close the postmaster's sockets */ + ClosePostmasterPorts(false); + if (type == BgWorkerFork) + { + /* + * Before blowing away PostmasterContext, save this bgworker's + * data where it can find it. + */ + MyBgworkerEntry = (BackgroundWorker *) + MemoryContextAlloc(TopMemoryContext, sizeof(BackgroundWorker)); + memcpy(MyBgworkerEntry, &(CurrentBgWorker->rw_worker), sizeof(BackgroundWorker)); + + } MemoryContextSwitchTo(TopMemoryContext); MemoryContextDelete(PostmasterContext); @@ -5521,21 +5489,41 @@ StartChildProcess(ForkProcType type) else if (pid < 0) { /* in parent, fork failed */ - int save_errno = errno; + child_errno = errno; - errno = save_errno; - ereport(LOG, - (errmsg("could not fork %s process: %m", fork_data->type_desc))); + ereport(LOG, + (errmsg("could not fork %s process: %m", fork_data->type_desc))); - /* - * fork failure is fatal during startup, but there's no need to choke - * immediately if starting other child types fails. - */ - if (MyForkProcType == StartupFork) - ExitPostmaster(1); + switch (MyForkProcType) + { + /* + * fork failure is fatal during startup, but there's no need to choke + * immediately if starting other child types fails. + */ + case StartupFork: + ExitPostmaster(1); + break; + case BgWorkerFork: + fork_data->fail_main(fork_data->ac, fork_data->av); + return -1; + default: + break; + } return 0; } + else /* parent */ + { + switch (MyForkProcType) + { + case SysLoggerFork: + case BgWorkerFork: + fork_data->postmaster_main(fork_data->ac, fork_data->av); + break; + default: + break; + } + } /* * in parent, successful fork @@ -5770,123 +5758,6 @@ BackgroundWorkerUnblockSignals(void) PG_SETMASK(&UnBlockSig); } -#ifdef EXEC_BACKEND -static pid_t -bgworker_forkexec(int shmem_slot) -{ - char *av[10]; - int ac = 0; - char forkav[MAXPGPATH]; - - snprintf(forkav, MAXPGPATH, "--forkbgworker=%d", shmem_slot); - - av[ac++] = "postgres"; - av[ac++] = forkav; - av[ac++] = NULL; /* filled in by postmaster_forkexec */ - av[ac] = NULL; - - Assert(ac < lengthof(av)); - - return postmaster_forkexec(ac, av); -} -#endif - -/* - * Start a new bgworker. - * Starting time conditions must have been checked already. - * - * Returns true on success, false on failure. - * In either case, update the RegisteredBgWorker's state appropriately. - * - * This code is heavily based on autovacuum.c, q.v. - */ -static bool -do_start_bgworker(RegisteredBgWorker *rw) -{ - pid_t worker_pid; - - Assert(rw->rw_pid == 0); - - /* - * Allocate and assign the Backend element. Note we must do this before - * forking, so that we can handle out of memory properly. - * - * Treat failure as though the worker had crashed. That way, the - * postmaster will wait a bit before attempting to start it again; if it - * tried again right away, most likely it'd find itself repeating the - * out-of-memory or fork failure condition. - */ - if (!assign_backendlist_entry(rw)) - { - rw->rw_crashed_at = GetCurrentTimestamp(); - return false; - } - - ereport(DEBUG1, - (errmsg("starting background worker process \"%s\"", - rw->rw_worker.bgw_name))); - -#ifdef EXEC_BACKEND - switch ((worker_pid = bgworker_forkexec(rw->rw_shmem_slot))) -#else - switch ((worker_pid = fork_process())) -#endif - { - case -1: - /* in postmaster, fork failed ... */ - ereport(LOG, - (errmsg("could not fork worker process: %m"))); - /* undo what assign_backendlist_entry did */ - ReleasePostmasterChildSlot(rw->rw_child_slot); - rw->rw_child_slot = 0; - free(rw->rw_backend); - rw->rw_backend = NULL; - /* mark entry as crashed, so we'll try again later */ - rw->rw_crashed_at = GetCurrentTimestamp(); - break; - -#ifndef EXEC_BACKEND - case 0: - /* in postmaster child ... */ - InitPostmasterChild(); - - /* Close the postmaster's sockets */ - ClosePostmasterPorts(false); - - /* - * Before blowing away PostmasterContext, save this bgworker's - * data where it can find it. - */ - MyBgworkerEntry = (BackgroundWorker *) - MemoryContextAlloc(TopMemoryContext, sizeof(BackgroundWorker)); - memcpy(MyBgworkerEntry, &rw->rw_worker, sizeof(BackgroundWorker)); - - /* Release postmaster's working memory context */ - MemoryContextSwitchTo(TopMemoryContext); - MemoryContextDelete(PostmasterContext); - PostmasterContext = NULL; - - StartBackgroundWorker(); - - exit(1); /* should not get here */ - break; -#endif - default: - /* in postmaster, fork successful ... */ - rw->rw_pid = worker_pid; - rw->rw_backend->pid = rw->rw_pid; - ReportBackgroundWorkerPID(rw); - /* add new worker to lists of backends */ - dlist_push_head(&BackendList, &rw->rw_backend->elem); -#ifdef EXEC_BACKEND - ShmemBackendArrayAdd(rw->rw_backend); -#endif - return true; - } - - return false; -} - /* * Does the current postmaster state require starting a worker with the * specified start_time? @@ -5927,54 +5798,6 @@ bgworker_should_start_now(BgWorkerStartTime start_time) return false; } -/* - * Allocate the Backend struct for a connected background worker, but don't - * add it to the list of backends just yet. - * - * On failure, return false without changing any worker state. - * - * Some info from the Backend is copied into the passed rw. - */ -static bool -assign_backendlist_entry(RegisteredBgWorker *rw) -{ - Backend *bn; - - /* - * Compute the cancel key that will be assigned to this session. We - * probably don't need cancel keys for background workers, but we'd better - * have something random in the field to prevent unfriendly people from - * sending cancels to them. - */ - if (!RandomCancelKey(&MyCancelKey)) - { - ereport(LOG, - (errcode(ERRCODE_INTERNAL_ERROR), - errmsg("could not generate random cancel key"))); - return false; - } - - bn = malloc(sizeof(Backend)); - if (bn == NULL) - { - ereport(LOG, - (errcode(ERRCODE_OUT_OF_MEMORY), - errmsg("out of memory"))); - return false; - } - - bn->cancel_key = MyCancelKey; - bn->child_slot = MyPMChildSlot = AssignPostmasterChildSlot(); - bn->bkend_type = BACKEND_TYPE_BGWORKER; - bn->dead_end = false; - bn->bgworker_notify = false; - - rw->rw_backend = bn; - rw->rw_child_slot = bn->child_slot; - - return true; -} - /* * If the time is right, start background worker(s). * @@ -6079,7 +5902,8 @@ maybe_start_bgworkers(void) * crashed, but there's no need because the next run of this * function will do that. */ - if (!do_start_bgworker(rw)) + CurrentBgWorker = rw; + if (do_start_bgworker() <= 0) { StartWorkerNeeded = true; return; @@ -6202,6 +6026,7 @@ save_backend_variables(BackendParameters *param, Port *port, param->max_safe_fds = max_safe_fds; param->MaxBackends = MaxBackends; + param->proc_type = MyForkProcType; #ifdef WIN32 param->PostmasterHandle = PostmasterHandle; @@ -6437,6 +6262,7 @@ restore_backend_variables(BackendParameters *param, Port *port) max_safe_fds = param->max_safe_fds; MaxBackends = param->MaxBackends; + MyForkProcType = param->proc_type; #ifdef WIN32 PostmasterHandle = param->PostmasterHandle; @@ -6472,7 +6298,7 @@ ShmemBackendArrayAllocation(void) memset(ShmemBackendArray, 0, size); } -static void +void ShmemBackendArrayAdd(Backend *bn) { /* The array slot corresponding to my PMChildSlot should be free */ diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h index bc6e03fbc7..da9c4319b9 100644 --- a/src/include/miscadmin.h +++ b/src/include/miscadmin.h @@ -163,6 +163,7 @@ extern PGDLLIMPORT int max_worker_processes; extern PGDLLIMPORT int max_parallel_workers; extern PGDLLIMPORT int MyProcPid; +extern PGDLLIMPORT int MyChildProcPid; extern PGDLLIMPORT pg_time_t MyStartTime; extern PGDLLIMPORT TimestampTz MyStartTimestamp; extern PGDLLIMPORT struct Port *MyProcPort; diff --git a/src/include/postmaster/bgworker.h b/src/include/postmaster/bgworker.h index a8864946cb..363e03aca4 100644 --- a/src/include/postmaster/bgworker.h +++ b/src/include/postmaster/bgworker.h @@ -41,6 +41,8 @@ #ifndef BGWORKER_H #define BGWORKER_H +#include "postmaster/fork_process.h" + /*--------------------------------------------------------------------- * External module API. *--------------------------------------------------------------------- @@ -158,4 +160,6 @@ extern void BackgroundWorkerInitializeConnectionByOid(Oid dboid, Oid useroid, ui extern void BackgroundWorkerBlockSignals(void); extern void BackgroundWorkerUnblockSignals(void); +extern int PrepBgWorkerFork(ForkProcData *bgworker_fork); + #endif /* BGWORKER_H */ diff --git a/src/include/postmaster/bgworker_internals.h b/src/include/postmaster/bgworker_internals.h index 0809e0af53..b6d86b503e 100644 --- a/src/include/postmaster/bgworker_internals.h +++ b/src/include/postmaster/bgworker_internals.h @@ -43,6 +43,7 @@ typedef struct RegisteredBgWorker } RegisteredBgWorker; extern slist_head BackgroundWorkerList; +extern RegisteredBgWorker *CurrentBgWorker; extern Size BackgroundWorkerShmemSize(void); extern void BackgroundWorkerShmemInit(void); @@ -54,7 +55,7 @@ extern void BackgroundWorkerStopNotifications(pid_t pid); extern void ResetBackgroundWorkerCrashTimes(void); /* Function to start a background worker, called from postmaster.c */ -extern void StartBackgroundWorker(void) pg_attribute_noreturn(); +extern void BackgroundWorkerMain(int argc, char *argv[]); #ifdef EXEC_BACKEND extern BackgroundWorker *BackgroundWorkerEntry(int slotno); diff --git a/src/include/postmaster/fork_process.h b/src/include/postmaster/fork_process.h index 32e10e3629..b5d5457ce7 100644 --- a/src/include/postmaster/fork_process.h +++ b/src/include/postmaster/fork_process.h @@ -29,6 +29,7 @@ typedef enum PgstatCollectorFork, PgArchiverFork, SysLoggerFork, + BgWorkerFork, NUMFORKPROCTYPES /* Must be last! */ } ForkProcType; diff --git a/src/include/postmaster/postmaster.h b/src/include/postmaster/postmaster.h index b692d8be11..9962bdd4be 100644 --- a/src/include/postmaster/postmaster.h +++ b/src/include/postmaster/postmaster.h @@ -13,6 +13,41 @@ #ifndef _POSTMASTER_H #define _POSTMASTER_H +#include "lib/ilist.h" + +/* + * List of active backends (or child processes anyway; we don't actually + * know whether a given child has become a backend or is still in the + * authorization phase). This is used mainly to keep track of how many + * children we have and send them appropriate signals when necessary. + * + * "Special" children such as the startup, bgwriter and autovacuum launcher + * tasks are not in this list. Autovacuum worker and walsender are in it. + * Also, "dead_end" children are in it: these are children launched just for + * the purpose of sending a friendly rejection message to a would-be client. + * We must track them because they are attached to shared memory, but we know + * they will never become live backends. dead_end children are not assigned a + * PMChildSlot. + * + * Background workers are in this list, too. + */ +typedef struct bkend +{ + pid_t pid; /* process id of backend */ + int32 cancel_key; /* cancel key for cancels for this backend */ + int child_slot; /* PMChildSlot for this backend, if any */ + + /* + * Flavor of backend or auxiliary process. Note that BACKEND_TYPE_WALSND + * backends initially announce themselves as BACKEND_TYPE_NORMAL, so if + * bkend_type is normal, you should check for a recent transition. + */ + int bkend_type; + bool dead_end; /* is it going to send an error and quit? */ + bool bgworker_notify; /* gets bgworker start/stop notifications */ + dlist_node elem; /* list link in BackendList */ +} Backend; + /* GUC options */ extern bool EnableSSL; extern int ReservedBackends; @@ -54,14 +89,19 @@ extern int MaxLivePostmasterChildren(void); extern bool PostmasterMarkPIDForWorkerNotify(int); +extern bool RandomCancelKey(int32 *cancel_key); + #ifdef EXEC_BACKEND extern pid_t postmaster_forkexec(int argc, char *argv[]); extern void SubPostmasterMain(int argc, char *argv[]) pg_attribute_noreturn(); extern Size ShmemBackendArraySize(void); extern void ShmemBackendArrayAllocation(void); +extern void ShmemBackendArrayAdd(Backend *bn); #endif +extern PGDLLIMPORT dlist_head BackendList; + /* * Note: MAX_BACKENDS is limited to 2^18-1 because that's the width reserved * for buffer references in buf_internals.h. This limitation could be lifted @@ -74,4 +114,17 @@ extern void ShmemBackendArrayAllocation(void); */ #define MAX_BACKENDS 0x3FFFF +/* + * Possible types of a backend. Beyond being the possible bkend_type values in + * struct bkend, these are OR-able request flag bits for SignalSomeChildren() + * and CountChildren(). + */ +#define BACKEND_TYPE_NORMAL 0x0001 /* normal backend */ +#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_WORKER (BACKEND_TYPE_AUTOVAC | BACKEND_TYPE_BGWORKER) + #endif /* _POSTMASTER_H */ -- 2.23.0
From 85cac3f12ff70df613859d01dca8d971701442f6 Mon Sep 17 00:00:00 2001 From: Mike Palmiotto <mike.palmio...@crunchydata.com> Date: Fri, 27 Sep 2019 20:34:17 -0400 Subject: [PATCH 5/8] Add syslogger to process centralization --- src/backend/postmaster/postmaster.c | 17 ++- src/backend/postmaster/syslogger.c | 152 ++++++++++---------------- src/include/postmaster/fork_process.h | 1 + src/include/postmaster/syslogger.h | 3 + 4 files changed, 79 insertions(+), 94 deletions(-) diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c index 6bdd57d1e2..5d78db067f 100644 --- a/src/backend/postmaster/postmaster.c +++ b/src/backend/postmaster/postmaster.c @@ -551,6 +551,7 @@ static void ShmemBackendArrayRemove(Backend *bn); #define StartAutoVacWorker() StartChildProcess(AutoVacWorkerFork) #define pgstat_start() StartChildProcess(PgstatCollectorFork) #define pgarch_start() StartChildProcess(PgArchiverFork) +#define SysLogger_Start() StartChildProcess(SysLoggerFork) /* Macros to check exit status of a child process */ #define EXIT_STATUS_0(st) ((st) == 0) @@ -5467,6 +5468,12 @@ StartChildProcess(ForkProcType type) case PgArchiverFork: PrepPgArchiverFork(fork_data); break; + case SysLoggerFork: + if (!Logging_collector) + return 0; + + PrepSysLoggerFork(fork_data); + break; default: break; } @@ -5488,13 +5495,21 @@ StartChildProcess(ForkProcType type) #ifndef EXEC_BACKEND InitPostmasterChild(); + /* Release postmaster's working memory context */ + if (type != SysLoggerFork) + { /* Close the postmaster's sockets */ ClosePostmasterPorts(false); - /* Release postmaster's working memory context */ MemoryContextSwitchTo(TopMemoryContext); MemoryContextDelete(PostmasterContext); + PostmasterContext = NULL; + } + else + { + ClosePostmasterPorts(true); + } #endif /* Call the process's main subroutine */ fork_data->child_main (fork_data->ac, fork_data->av); diff --git a/src/backend/postmaster/syslogger.c b/src/backend/postmaster/syslogger.c index bb2baff763..37baad6450 100644 --- a/src/backend/postmaster/syslogger.c +++ b/src/backend/postmaster/syslogger.c @@ -48,6 +48,7 @@ #include "storage/pg_shmem.h" #include "tcop/tcopprot.h" #include "utils/guc.h" +#include "utils/memutils.h" #include "utils/ps_status.h" #include "utils/timestamp.h" @@ -134,10 +135,12 @@ static volatile sig_atomic_t rotation_requested = false; /* Local subroutines */ #ifdef EXEC_BACKEND -static pid_t syslogger_forkexec(void); static void syslogger_parseArgs(int argc, char *argv[]); #endif + NON_EXEC_STATIC void SysLoggerMain(int argc, char *argv[]) pg_attribute_noreturn(); +NON_EXEC_STATIC void SysLoggerPostmasterMain(int argc, char *argv[]); + static void process_pipe_input(char *logbuffer, int *bytes_in_logbuffer); static void flush_pipe_input(char *logbuffer, int *bytes_in_logbuffer); static FILE *logfile_open(const char *filename, const char *mode, @@ -171,12 +174,16 @@ SysLoggerMain(int argc, char *argv[]) pg_time_t now; WaitEventSet *wes; - now = MyStartTime; - #ifdef EXEC_BACKEND syslogger_parseArgs(argc, argv); +#else + /* Drop our connection to postmaster's shared memory, as well */ + dsm_detach_all(); + PGSharedMemoryDetach(); #endif /* EXEC_BACKEND */ + now = MyStartTime; + am_syslogger = true; init_ps_display("logger", "", "", ""); @@ -540,16 +547,42 @@ SysLoggerMain(int argc, char *argv[]) } /* - * Postmaster subroutine to start a syslogger subprocess. + * Helper function for setting up a file number buffer + */ +static char * +setup_file_buff(FILE *file) +{ + char *filenobuf; + + /* static variables (those not passed by write_backend_variables) */ +#ifndef WIN32 + if (file != NULL) + filenobuf = psprintf("%d", fileno(file)); + else + filenobuf = pstrdup("-1"); +#else /* WIN32 */ + if (file != NULL) + filenobuf = psprintf("%ld", (long) _get_osfhandle(_fileno(file))); + else + filenobuf = pstrdup("0"); +#endif /* WIN32 */ + + return filenobuf; +} + +/* + * PrepSysLoggerFork + * + * Postmaster subroutine to prepare a syslogger subprocess. */ -int -SysLogger_Start(void) +void +PrepSysLoggerFork(ForkProcData *logger_fork) { - pid_t sysloggerPid; + int ac = 0; char *filename; + MemoryContext cxt; - if (!Logging_collector) - return 0; + cxt = MemoryContextSwitchTo(PostmasterContext); /* * If first time through, create the pipe which will receive stderr @@ -626,37 +659,29 @@ SysLogger_Start(void) pfree(filename); } -#ifdef EXEC_BACKEND - switch ((sysloggerPid = syslogger_forkexec())) -#else - switch ((sysloggerPid = fork_process())) -#endif - { - case -1: - ereport(LOG, - (errmsg("could not fork system logger: %m"))); - return 0; + MemoryContextSwitchTo(cxt); -#ifndef EXEC_BACKEND - case 0: - /* in postmaster child ... */ - InitPostmasterChild(); + logger_fork->av[ac++] = pstrdup("postgres"); + logger_fork->av[ac++] = pstrdup("--forklog"); + logger_fork->av[ac++] = NULL; /* filled in by postmaster_forkexec */ - /* Close the postmaster's sockets */ - ClosePostmasterPorts(true); + logger_fork->av[ac++] = setup_file_buff(syslogFile); + logger_fork->av[ac++] = setup_file_buff(csvlogFile); - /* Drop our connection to postmaster's shared memory, as well */ - dsm_detach_all(); - PGSharedMemoryDetach(); + logger_fork->av[ac] = NULL; - /* do the work */ - SysLoggerMain(0, NULL); - break; -#endif + logger_fork->ac = ac; + Assert(logger_fork->ac < lengthof(*logger_fork->av)); + + logger_fork->child_main = SysLoggerMain; + logger_fork->postmaster_main = SysLoggerPostmasterMain; + logger_fork->type_desc = pstrdup("system logger"); +} - default: - /* success, in postmaster */ +NON_EXEC_STATIC void +SysLoggerPostmasterMain(int argc, char *argv[]) +{ /* now we redirect stderr, if not done already */ if (!redirection_done) { @@ -723,70 +748,11 @@ SysLogger_Start(void) fclose(csvlogFile); csvlogFile = NULL; } - return (int) sysloggerPid; - } - - /* we should never reach here */ - return 0; } #ifdef EXEC_BACKEND -/* - * syslogger_forkexec() - - * - * Format up the arglist for, then fork and exec, a syslogger process - */ -static pid_t -syslogger_forkexec(void) -{ - char *av[10]; - int ac = 0; - char filenobuf[32]; - char csvfilenobuf[32]; - - av[ac++] = "postgres"; - av[ac++] = "--forklog"; - av[ac++] = NULL; /* filled in by postmaster_forkexec */ - - /* static variables (those not passed by write_backend_variables) */ -#ifndef WIN32 - if (syslogFile != NULL) - snprintf(filenobuf, sizeof(filenobuf), "%d", - fileno(syslogFile)); - else - strcpy(filenobuf, "-1"); -#else /* WIN32 */ - if (syslogFile != NULL) - snprintf(filenobuf, sizeof(filenobuf), "%ld", - (long) _get_osfhandle(_fileno(syslogFile))); - else - strcpy(filenobuf, "0"); -#endif /* WIN32 */ - av[ac++] = filenobuf; - -#ifndef WIN32 - if (csvlogFile != NULL) - snprintf(csvfilenobuf, sizeof(csvfilenobuf), "%d", - fileno(csvlogFile)); - else - strcpy(csvfilenobuf, "-1"); -#else /* WIN32 */ - if (csvlogFile != NULL) - snprintf(csvfilenobuf, sizeof(csvfilenobuf), "%ld", - (long) _get_osfhandle(_fileno(csvlogFile))); - else - strcpy(csvfilenobuf, "0"); -#endif /* WIN32 */ - av[ac++] = csvfilenobuf; - - av[ac] = NULL; - Assert(ac < lengthof(av)); - - return postmaster_forkexec(ac, av); -} - /* * syslogger_parseArgs() - * diff --git a/src/include/postmaster/fork_process.h b/src/include/postmaster/fork_process.h index 86bad60cea..32e10e3629 100644 --- a/src/include/postmaster/fork_process.h +++ b/src/include/postmaster/fork_process.h @@ -28,6 +28,7 @@ typedef enum AutoVacWorkerFork, PgstatCollectorFork, PgArchiverFork, + SysLoggerFork, NUMFORKPROCTYPES /* Must be last! */ } ForkProcType; diff --git a/src/include/postmaster/syslogger.h b/src/include/postmaster/syslogger.h index 3a61104573..1d0b125141 100644 --- a/src/include/postmaster/syslogger.h +++ b/src/include/postmaster/syslogger.h @@ -14,6 +14,7 @@ #include <limits.h> /* for PIPE_BUF */ +#include "postmaster/fork_process.h" /* * Primitive protocol structure for writing to syslogger pipe(s). The idea @@ -83,6 +84,8 @@ extern int SysLogger_Start(void); extern void write_syslogger_file(const char *buffer, int count, int dest); +extern void PrepSysLoggerFork(ForkProcData *logger_fork); + #ifdef EXEC_BACKEND extern void SysLoggerMain(int argc, char *argv[]) pg_attribute_noreturn(); #endif -- 2.23.0
From 367a6a98bd2b8ea5c810bd081027806c6045dfd8 Mon Sep 17 00:00:00 2001 From: Mike Palmiotto <mike.palmio...@crunchydata.com> Date: Fri, 27 Sep 2019 19:33:15 -0400 Subject: [PATCH 4/8] Add archiver to process centralization --- src/backend/postmaster/pgarch.c | 89 ++++++--------------------- src/backend/postmaster/postmaster.c | 4 ++ src/include/postmaster/fork_process.h | 1 + src/include/postmaster/pgarch.h | 5 +- 4 files changed, 28 insertions(+), 71 deletions(-) diff --git a/src/backend/postmaster/pgarch.c b/src/backend/postmaster/pgarch.c index f84f882c4c..53661004fd 100644 --- a/src/backend/postmaster/pgarch.c +++ b/src/backend/postmaster/pgarch.c @@ -92,10 +92,6 @@ static volatile sig_atomic_t ready_to_stop = false; * Local function forward declarations * ---------- */ -#ifdef EXEC_BACKEND -static pid_t pgarch_forkexec(void); -#endif - NON_EXEC_STATIC void PgArchiverMain(int argc, char *argv[]) pg_attribute_noreturn(); static void pgarch_exit(SIGNAL_ARGS); static void ArchSigHupHandler(SIGNAL_ARGS); @@ -115,7 +111,7 @@ static void pgarch_archiveDone(char *xlog); */ /* - * pgarch_start + * PrepPgArchiverFork * * Called from postmaster at startup or after an existing archiver * died. Attempt to fire up a fresh archiver process. @@ -124,17 +120,17 @@ static void pgarch_archiveDone(char *xlog); * * Note: if fail, we will be called again from the postmaster main loop. */ -int -pgarch_start(void) +void +PrepPgArchiverFork(ForkProcData *pgarch_fork) { + int ac = 0; time_t curtime; - pid_t pgArchPid; /* * Do nothing if no archiver needed */ if (!XLogArchivingActive()) - return 0; + return; /* * Do nothing if too soon since last archiver start. This is a safety @@ -145,42 +141,20 @@ pgarch_start(void) curtime = time(NULL); if ((unsigned int) (curtime - last_pgarch_start_time) < (unsigned int) PGARCH_RESTART_INTERVAL) - return 0; + return; last_pgarch_start_time = curtime; -#ifdef EXEC_BACKEND - switch ((pgArchPid = pgarch_forkexec())) -#else - switch ((pgArchPid = fork_process())) -#endif - { - case -1: - ereport(LOG, - (errmsg("could not fork archiver: %m"))); - return 0; - -#ifndef EXEC_BACKEND - case 0: - /* in postmaster child ... */ - InitPostmasterChild(); - - /* Close the postmaster's sockets */ - ClosePostmasterPorts(false); + pgarch_fork->av[ac++] = pstrdup("postgres"); - /* Drop our connection to postmaster's shared memory, as well */ - dsm_detach_all(); - PGSharedMemoryDetach(); - - PgArchiverMain(0, NULL); - break; +#ifdef EXEC_BACKEND + pgarch_fork->av[ac++] = pstrdup("--forkarch"); + pgarch_fork->av[ac++] = NULL; /* filled in by postmaster_forkexec */ #endif + pgarch_fork->ac = ac; - default: - return (int) pgArchPid; - } - - /* shouldn't get here */ - return 0; + Assert(pgarch_fork->ac < lengthof(*pgarch_fork->av)); + pgarch_fork->child_main = PgArchiverMain; + pgarch_fork->type_desc = pstrdup("archiver"); } /* ------------------------------------------------------------ @@ -189,42 +163,19 @@ pgarch_start(void) */ -#ifdef EXEC_BACKEND - -/* - * pgarch_forkexec() - - * - * Format up the arglist for, then fork and exec, archive process - */ -static pid_t -pgarch_forkexec(void) -{ - char *av[10]; - int ac = 0; - - av[ac++] = "postgres"; - - av[ac++] = "--forkarch"; - - av[ac++] = NULL; /* filled in by postmaster_forkexec */ - - av[ac] = NULL; - Assert(ac < lengthof(av)); - - return postmaster_forkexec(ac, av); -} -#endif /* EXEC_BACKEND */ - - /* * PgArchiverMain * * The argc/argv parameters are valid only in EXEC_BACKEND case. However, * since we don't use 'em, it hardly matters... */ -NON_EXEC_STATIC void -PgArchiverMain(int argc, char *argv[]) +void +PgArchiverMain(pg_attribute_unused() int argc, pg_attribute_unused() char *arg[]) { + /* 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 for SIGHUP, SIGTERM, SIGUSR1, SIGUSR2, and SIGQUIT. diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c index 37a36387a3..6bdd57d1e2 100644 --- a/src/backend/postmaster/postmaster.c +++ b/src/backend/postmaster/postmaster.c @@ -550,6 +550,7 @@ static void ShmemBackendArrayRemove(Backend *bn); #define StartAutoVacLauncher() StartChildProcess(AutoVacLauncherFork) #define StartAutoVacWorker() StartChildProcess(AutoVacWorkerFork) #define pgstat_start() StartChildProcess(PgstatCollectorFork) +#define pgarch_start() StartChildProcess(PgArchiverFork) /* Macros to check exit status of a child process */ #define EXIT_STATUS_0(st) ((st) == 0) @@ -5463,6 +5464,9 @@ StartChildProcess(ForkProcType type) case PgstatCollectorFork: PrepPgstatCollectorFork(fork_data); break; + case PgArchiverFork: + PrepPgArchiverFork(fork_data); + break; default: break; } diff --git a/src/include/postmaster/fork_process.h b/src/include/postmaster/fork_process.h index 16ca8be968..86bad60cea 100644 --- a/src/include/postmaster/fork_process.h +++ b/src/include/postmaster/fork_process.h @@ -27,6 +27,7 @@ typedef enum AutoVacLauncherFork, AutoVacWorkerFork, PgstatCollectorFork, + PgArchiverFork, NUMFORKPROCTYPES /* Must be last! */ } ForkProcType; diff --git a/src/include/postmaster/pgarch.h b/src/include/postmaster/pgarch.h index 2474eac26a..ce23e39c8b 100644 --- a/src/include/postmaster/pgarch.h +++ b/src/include/postmaster/pgarch.h @@ -13,6 +13,8 @@ #ifndef _PGARCH_H #define _PGARCH_H +#include "postmaster/fork_process.h" + /* ---------- * Archiver control info. * @@ -30,10 +32,9 @@ * Functions called from postmaster * ---------- */ -extern int pgarch_start(void); - #ifdef EXEC_BACKEND extern void PgArchiverMain(int argc, char *argv[]) pg_attribute_noreturn(); #endif +extern void PrepPgArchiverFork(ForkProcData *pgarch_fork); #endif /* _PGARCH_H */ -- 2.23.0
From a6f005d9ea81e1a989353f320149e12281e9fb04 Mon Sep 17 00:00:00 2001 From: Mike Palmiotto <mike.palmio...@crunchydata.com> Date: Fri, 27 Sep 2019 16:45:57 -0400 Subject: [PATCH 2/8] Add centralized autovac launcher/worker --- src/backend/postmaster/autovacuum.c | 157 ++++---------------------- src/backend/postmaster/postmaster.c | 67 ++++++----- src/include/postmaster/autovacuum.h | 9 +- src/include/postmaster/fork_process.h | 2 + 4 files changed, 70 insertions(+), 165 deletions(-) diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c index 073f313337..5c3ac1568f 100644 --- a/src/backend/postmaster/autovacuum.c +++ b/src/backend/postmaster/autovacuum.c @@ -134,8 +134,8 @@ int Log_autovacuum_min_duration = -1; #define MAX_AUTOVAC_SLEEPTIME 300 /* seconds */ /* Flags to tell if we are in an autovacuum process */ -static bool am_autovacuum_launcher = false; -static bool am_autovacuum_worker = false; +bool am_autovacuum_launcher = false; +bool am_autovacuum_worker = false; /* Flags set by signal handlers */ static volatile sig_atomic_t got_SIGHUP = false; @@ -303,10 +303,6 @@ static WorkerInfo MyWorkerInfo = NULL; /* PID of launcher, valid only in worker while shutting down */ int AutovacuumLauncherPid = 0; -#ifdef EXEC_BACKEND -static pid_t avlauncher_forkexec(void); -static pid_t avworker_forkexec(void); -#endif NON_EXEC_STATIC void AutoVacWorkerMain(int argc, char *argv[]) pg_attribute_noreturn(); NON_EXEC_STATIC void AutoVacLauncherMain(int argc, char *argv[]) pg_attribute_noreturn(); @@ -347,88 +343,53 @@ static void avl_sigusr2_handler(SIGNAL_ARGS); static void avl_sigterm_handler(SIGNAL_ARGS); static void autovac_refresh_stats(void); - - /******************************************************************** * AUTOVACUUM LAUNCHER CODE ********************************************************************/ -#ifdef EXEC_BACKEND /* - * forkexec routine for the autovacuum launcher process. + * PrepAutoVacProcessFork * * Format up the arglist, then fork and exec. */ -static pid_t -avlauncher_forkexec(void) +void +PrepAutoVacProcessFork(ForkProcData *autovac_fork) { - char *av[10]; int ac = 0; - av[ac++] = "postgres"; - av[ac++] = "--forkavlauncher"; - av[ac++] = NULL; /* filled in by postmaster_forkexec */ - av[ac] = NULL; - - Assert(ac < lengthof(av)); - - return postmaster_forkexec(ac, av); -} - /* - * We need this set from the outside, before InitProcess is called + * Set up command-line arguments for subprocess */ -void -AutovacuumLauncherIAm(void) -{ - am_autovacuum_launcher = true; -} -#endif + autovac_fork->av[ac++] = pstrdup("postgres"); -/* - * Main entry point for autovacuum launcher process, to be called from the - * postmaster. - */ -int -StartAutoVacLauncher(void) + if (MyForkProcType == AutoVacLauncherFork) { - pid_t AutoVacPID; - + autovac_fork->type_desc = pstrdup("autovacuum launcher"); + autovac_fork->child_main = AutoVacLauncherMain; #ifdef EXEC_BACKEND - switch ((AutoVacPID = avlauncher_forkexec())) -#else - switch ((AutoVacPID = fork_process())) + autovac_fork->av[ac++] = pstrdup("--forkavlauncher"); + autovac_fork->av[ac++] = NULL; /* filled in by postmaster_forkexec */ #endif + } + else if (MyForkProcType == AutoVacWorkerFork) { - case -1: - ereport(LOG, - (errmsg("could not fork autovacuum launcher process: %m"))); - return 0; - -#ifndef EXEC_BACKEND - case 0: - /* in postmaster child ... */ - InitPostmasterChild(); - - /* Close the postmaster's sockets */ - ClosePostmasterPorts(false); - - AutoVacLauncherMain(0, NULL); - break; + autovac_fork->type_desc = pstrdup("autovacuum worker"); + autovac_fork->child_main = AutoVacWorkerMain; +#ifdef EXEC_BACKEND + autovac_fork->av[ac++] = pstrdup("--forkavworker"); + autovac_fork->av[ac++] = NULL; /* filled in by postmaster_forkexec */ #endif - default: - return (int) AutoVacPID; } - /* shouldn't get here */ - return 0; + autovac_fork->ac = ac; + Assert(autovac_fork->ac < lengthof(*autovac_fork->av)); } /* * Main loop for the autovacuum launcher process. */ NON_EXEC_STATIC void -AutoVacLauncherMain(int argc, char *argv[]) +AutoVacLauncherMain(pg_attribute_unused() int argc, pg_attribute_unused() char *argv[]) { sigjmp_buf local_sigjmp_buf; @@ -1428,83 +1389,11 @@ avl_sigterm_handler(SIGNAL_ARGS) * AUTOVACUUM WORKER CODE ********************************************************************/ -#ifdef EXEC_BACKEND -/* - * forkexec routines for the autovacuum worker. - * - * Format up the arglist, then fork and exec. - */ -static pid_t -avworker_forkexec(void) -{ - char *av[10]; - int ac = 0; - - av[ac++] = "postgres"; - av[ac++] = "--forkavworker"; - av[ac++] = NULL; /* filled in by postmaster_forkexec */ - av[ac] = NULL; - - Assert(ac < lengthof(av)); - - return postmaster_forkexec(ac, av); -} - -/* - * We need this set from the outside, before InitProcess is called - */ -void -AutovacuumWorkerIAm(void) -{ - am_autovacuum_worker = true; -} -#endif - -/* - * Main entry point for autovacuum worker process. - * - * This code is heavily based on pgarch.c, q.v. - */ -int -StartAutoVacWorker(void) -{ - pid_t worker_pid; - -#ifdef EXEC_BACKEND - switch ((worker_pid = avworker_forkexec())) -#else - switch ((worker_pid = fork_process())) -#endif - { - case -1: - ereport(LOG, - (errmsg("could not fork autovacuum worker process: %m"))); - return 0; - -#ifndef EXEC_BACKEND - case 0: - /* in postmaster child ... */ - InitPostmasterChild(); - - /* Close the postmaster's sockets */ - ClosePostmasterPorts(false); - - AutoVacWorkerMain(0, NULL); - break; -#endif - default: - return (int) worker_pid; - } - - /* shouldn't get here */ - return 0; -} - /* * AutoVacWorkerMain */ NON_EXEC_STATIC void -AutoVacWorkerMain(int argc, char *argv[]) +AutoVacWorkerMain(pg_attribute_unused() int argc, pg_attribute_unused() char *argv[]) { sigjmp_buf local_sigjmp_buf; Oid dbid; diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c index e0d11cc6ab..35cd1479b9 100644 --- a/src/backend/postmaster/postmaster.c +++ b/src/backend/postmaster/postmaster.c @@ -414,6 +414,9 @@ static bool RandomCancelKey(int32 *cancel_key); static void signal_child(pid_t pid, int signal); static bool SignalSomeChildren(int signal, int targets); static void TerminateChildren(int signal); +#ifdef EXEC_BACKEND +static void shmemSetup(bool aux_process); +#endif #define SignalChildren(sig) SignalSomeChildren(sig, BACKEND_TYPE_ALL) @@ -544,6 +547,8 @@ static void ShmemBackendArrayRemove(Backend *bn); #define StartCheckpointer() StartChildProcess(CheckpointerFork) #define StartWalWriter() StartChildProcess(WalWriterFork) #define StartWalReceiver() StartChildProcess(WalReceiverFork) +#define StartAutoVacLauncher() StartChildProcess(AutoVacLauncherFork) +#define StartAutoVacWorker() StartChildProcess(AutoVacWorkerFork) /* Macros to check exit status of a child process */ #define EXIT_STATUS_0(st) ((st) == 0) @@ -4858,6 +4863,29 @@ retry: } #endif /* WIN32 */ +/* + * shmemSetup + * + * Helper function for a child to set up shmem before + * executing. + * + * aux_process - set to true if an auxiliary process. + */ +static void +shmemSetup(bool aux_process) +{ + /* Restore basic shared memory pointers */ + InitShmemAccess(UsedShmemSegAddr); + + /* Need a PGPROC to run CreateSharedMemoryAndSemaphores */ + if (aux_process) + InitAuxiliaryProcess(); + else + InitProcess(); + + /* Attach process to shared data structures */ + CreateSharedMemoryAndSemaphores(); +} /* * SubPostmasterMain -- Get the fork/exec'd process into a state equivalent @@ -4943,9 +4971,9 @@ SubPostmasterMain(int argc, char *argv[]) /* autovacuum needs this set before calling InitProcess */ if (strcmp(argv[1], "--forkavlauncher") == 0) - AutovacuumLauncherIAm(); + am_autovacuum_launcher = true; if (strcmp(argv[1], "--forkavworker") == 0) - AutovacuumWorkerIAm(); + am_autovacuum_worker = true; /* * Start our win32 signal implementation. This has to be done after we @@ -5038,41 +5066,17 @@ SubPostmasterMain(int argc, char *argv[]) } if (strcmp(argv[1], "--forkboot") == 0) { - /* Restore basic shared memory pointers */ - InitShmemAccess(UsedShmemSegAddr); - - /* Need a PGPROC to run CreateSharedMemoryAndSemaphores */ - InitAuxiliaryProcess(); - - /* Attach process to shared data structures */ - CreateSharedMemoryAndSemaphores(); - + shmemSetup(true); AuxiliaryProcessMain(argc - 2, argv + 2); /* does not return */ } if (strcmp(argv[1], "--forkavlauncher") == 0) { - /* Restore basic shared memory pointers */ - InitShmemAccess(UsedShmemSegAddr); - - /* Need a PGPROC to run CreateSharedMemoryAndSemaphores */ - InitProcess(); - - /* Attach process to shared data structures */ - CreateSharedMemoryAndSemaphores(); - + shmemSetup(true); AutoVacLauncherMain(argc - 2, argv + 2); /* does not return */ } if (strcmp(argv[1], "--forkavworker") == 0) { - /* Restore basic shared memory pointers */ - InitShmemAccess(UsedShmemSegAddr); - - /* Need a PGPROC to run CreateSharedMemoryAndSemaphores */ - InitProcess(); - - /* Attach process to shared data structures */ - CreateSharedMemoryAndSemaphores(); - + shmemSetup(true); AutoVacWorkerMain(argc - 2, argv + 2); /* does not return */ } if (strncmp(argv[1], "--forkbgworker=", 15) == 0) @@ -5450,6 +5454,11 @@ StartChildProcess(ForkProcType type) case WalReceiverFork: PrepAuxProcessFork(fork_data); break; + /* Non-Auxiliary Processes */ + case AutoVacLauncherFork: + case AutoVacWorkerFork: + PrepAutoVacProcessFork(fork_data); + break; default: break; diff --git a/src/include/postmaster/autovacuum.h b/src/include/postmaster/autovacuum.h index 8451e5d9e2..518775f528 100644 --- a/src/include/postmaster/autovacuum.h +++ b/src/include/postmaster/autovacuum.h @@ -15,6 +15,7 @@ #define AUTOVACUUM_H #include "storage/block.h" +#include "postmaster/fork_process.h" /* * Other processes can request specific work from autovacuum, identified by @@ -45,6 +46,10 @@ extern int AutovacuumLauncherPid; extern int Log_autovacuum_min_duration; +/* autovacuum identification */ +extern bool am_autovacuum_launcher; +extern bool am_autovacuum_worker; + /* Status inquiry functions */ extern bool AutoVacuumingActive(void); extern bool IsAutoVacuumLauncherProcess(void); @@ -58,6 +63,8 @@ extern void autovac_init(void); extern int StartAutoVacLauncher(void); extern int StartAutoVacWorker(void); +extern void PrepAutoVacProcessFork(ForkProcData *autovac_fork); + /* called from postmaster when a worker could not be forked */ extern void AutoVacWorkerFailed(void); @@ -67,8 +74,6 @@ extern void AutoVacuumUpdateDelay(void); #ifdef EXEC_BACKEND extern void AutoVacLauncherMain(int argc, char *argv[]) pg_attribute_noreturn(); extern void AutoVacWorkerMain(int argc, char *argv[]) pg_attribute_noreturn(); -extern void AutovacuumWorkerIAm(void); -extern void AutovacuumLauncherIAm(void); #endif extern bool AutoVacuumRequestWork(AutoVacuumWorkItemType type, diff --git a/src/include/postmaster/fork_process.h b/src/include/postmaster/fork_process.h index d912229055..1f319fc98f 100644 --- a/src/include/postmaster/fork_process.h +++ b/src/include/postmaster/fork_process.h @@ -24,6 +24,8 @@ typedef enum CheckpointerFork, WalWriterFork, WalReceiverFork, /* end of Auxiliary Process Forks */ + AutoVacLauncherFork, + AutoVacWorkerFork, NUMFORKPROCTYPES /* Must be last! */ } ForkProcType; -- 2.23.0
From efc6f0531b71847c6500062b5a0fe43331a6ea7e Mon Sep 17 00:00:00 2001 From: Mike Palmiotto <mike.palmio...@crunchydata.com> 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
From 2a3a35f2e80cb2badcb0efbce1bad2484e364b7b Mon Sep 17 00:00:00 2001 From: Mike Palmiotto <mike.palmio...@crunchydata.com> Date: Fri, 27 Sep 2019 12:28:19 -0400 Subject: [PATCH 1/8] Add ForkProcType infrastructure --- src/backend/bootstrap/bootstrap.c | 1 + src/backend/main/main.c | 7 + src/backend/postmaster/postmaster.c | 197 ++++++++++++++++---------- src/backend/utils/init/globals.c | 3 +- src/backend/utils/init/postinit.c | 2 +- src/include/bootstrap/bootstrap.h | 2 +- src/include/postmaster/fork_process.h | 33 +++++ 7 files changed, 167 insertions(+), 78 deletions(-) diff --git a/src/backend/bootstrap/bootstrap.c b/src/backend/bootstrap/bootstrap.c index 9238fbe98d..9f3dad1c6d 100644 --- a/src/backend/bootstrap/bootstrap.c +++ b/src/backend/bootstrap/bootstrap.c @@ -70,6 +70,7 @@ static void cleanup(void); */ AuxProcType MyAuxProcType = NotAnAuxProcess; /* declared in miscadmin.h */ +ForkProcType MyForkProcType = NoForkProcess; /* declared in fork_process.h */ Relation boot_reldesc; /* current relation descriptor */ diff --git a/src/backend/main/main.c b/src/backend/main/main.c index a9edbfd4a4..79942deb7d 100644 --- a/src/backend/main/main.c +++ b/src/backend/main/main.c @@ -199,7 +199,14 @@ main(int argc, char *argv[]) #endif if (argc > 1 && strcmp(argv[1], "--boot") == 0) + { + MemoryContext old_ctx; + + old_ctx = MemoryContextSwitchTo(TopMemoryContext); + AuxiliaryProcessMain(argc, argv); /* does not return */ + MemoryContextSwitchTo(old_ctx); + } else if (argc > 1 && strcmp(argv[1], "--describe-config") == 0) GucInfoMain(); /* does not return */ else if (argc > 1 && strcmp(argv[1], "--single") == 0) diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c index eb9e0221f8..e0d11cc6ab 100644 --- a/src/backend/postmaster/postmaster.c +++ b/src/backend/postmaster/postmaster.c @@ -421,7 +421,7 @@ static int CountChildren(int target); static bool assign_backendlist_entry(RegisteredBgWorker *rw); static void maybe_start_bgworkers(void); static bool CreateOptsFile(int argc, char *argv[], char *fullprogname); -static pid_t StartChildProcess(AuxProcType type); +static pid_t StartChildProcess(ForkProcType type); static void StartAutovacuumWorker(void); static void MaybeStartWalReceiver(void); static void InitPostmasterDeathWatchHandle(void); @@ -456,7 +456,7 @@ typedef struct #endif /* WIN32 */ static pid_t backend_forkexec(Port *port); -static pid_t internal_forkexec(int argc, char *argv[], Port *port); +static pid_t internal_forkexec(int argc, char *argv[]); /* Type for a socket that can be inherited to a client process */ #ifdef WIN32 @@ -522,6 +522,7 @@ typedef struct char my_exec_path[MAXPGPATH]; char pkglib_path[MAXPGPATH]; char ExtraOptions[MAXPGPATH]; + ForkProcType proc_type; } BackendParameters; static void read_backend_variables(char *id, Port *port); @@ -538,11 +539,11 @@ static void ShmemBackendArrayAdd(Backend *bn); static void ShmemBackendArrayRemove(Backend *bn); #endif /* EXEC_BACKEND */ -#define StartupDataBase() StartChildProcess(StartupProcess) -#define StartBackgroundWriter() StartChildProcess(BgWriterProcess) -#define StartCheckpointer() StartChildProcess(CheckpointerProcess) -#define StartWalWriter() StartChildProcess(WalWriterProcess) -#define StartWalReceiver() StartChildProcess(WalReceiverProcess) +#define StartupDataBase() StartChildProcess(StartupFork) +#define StartBackgroundWriter() StartChildProcess(BgWriterFork) +#define StartCheckpointer() StartChildProcess(CheckpointerFork) +#define StartWalWriter() StartChildProcess(WalWriterFork) +#define StartWalReceiver() StartChildProcess(WalReceiverFork) /* Macros to check exit status of a child process */ #define EXIT_STATUS_0(st) ((st) == 0) @@ -560,6 +561,60 @@ int postmaster_alive_fds[2] = {-1, -1}; HANDLE PostmasterHandle; #endif +static Port *ConnProcPort = NULL; + +/* + * PrepAuxProcessFork + * + * Prepare a ForkProcType struct for the auxiliary process specified by + * AuxProcType. This does all prep related to av parameters and error messages. + */ +static void +PrepAuxProcessFork(ForkProcData *aux_fork) +{ + int ac = 0; + + /* + * Set up command-line arguments for subprocess + */ + aux_fork->av[ac++] = pstrdup("postgres"); + +#ifdef EXEC_BACKEND + aux_fork->av[ac++] = pstrdup("--forkboot"); + aux_fork->av[ac++] = NULL; /* filled in by postmaster_forkexec */ +#endif + + aux_fork->av[ac++] = psprintf("-x%d", MyForkProcType); + + aux_fork->av[ac] = NULL; + Assert(ac < lengthof(*aux_fork->av)); + + aux_fork->ac = ac; + switch (MyForkProcType) + { + case StartupProcess: + aux_fork->type_desc = pstrdup("startup"); + break; + case BgWriterProcess: + aux_fork->type_desc = pstrdup("background writer"); + break; + case CheckpointerProcess: + aux_fork->type_desc = pstrdup("checkpointer"); + break; + case WalWriterProcess: + aux_fork->type_desc = pstrdup("WAL writer"); + break; + case WalReceiverProcess: + aux_fork->type_desc = pstrdup("WAL receiver"); + break; + default: + aux_fork->type_desc = pstrdup("child"); + break; + } + + aux_fork->child_main = AuxiliaryProcessMain; +} + /* * Postmaster main entry point */ @@ -1710,19 +1765,17 @@ ServerLoop(void) break; if (FD_ISSET(ListenSocket[i], &rmask)) { - Port *port; - - port = ConnCreate(ListenSocket[i]); - if (port) + ConnProcPort = ConnCreate(ListenSocket[i]); + if (ConnProcPort) { - BackendStartup(port); + BackendStartup(ConnProcPort); /* * We no longer need the open socket or port structure * in this process */ - StreamClose(port->sock); - ConnFree(port); + StreamClose(ConnProcPort->sock); + ConnFree(ConnProcPort); } } } @@ -4477,11 +4530,11 @@ BackendRun(Port *port) pid_t postmaster_forkexec(int argc, char *argv[]) { - Port port; - /* This entry point passes dummy values for the Port variables */ - memset(&port, 0, sizeof(port)); - return internal_forkexec(argc, argv, &port); + if (!ConnProcPort) + ConnProcPort = palloc0(sizeof(*ConnProcPort)); + + return internal_forkexec(argc, argv); } /* @@ -4506,7 +4559,7 @@ backend_forkexec(Port *port) av[ac] = NULL; Assert(ac < lengthof(av)); - return internal_forkexec(ac, av, port); + return internal_forkexec(ac, av); } #ifndef WIN32 @@ -4518,7 +4571,7 @@ backend_forkexec(Port *port) * - fork():s, and then exec():s the child process */ static pid_t -internal_forkexec(int argc, char *argv[], Port *port) +internal_forkexec(int argc, char *argv[]) { static unsigned long tmpBackendFileNum = 0; pid_t pid; @@ -4526,7 +4579,7 @@ internal_forkexec(int argc, char *argv[], Port *port) BackendParameters param; FILE *fp; - if (!save_backend_variables(¶m, port)) + if (!save_backend_variables(¶m, ConnProcPort)) return -1; /* log made by save_backend_variables */ /* Calculate name for temp file */ @@ -4836,9 +4889,12 @@ SubPostmasterMain(int argc, char *argv[]) if (argc < 3) elog(FATAL, "invalid subpostmaster invocation"); + Assert(!ConnProcPort); + /* Read in the variables file */ memset(&port, 0, sizeof(Port)); read_backend_variables(argv[2], &port); + ConnProcPort = &port; /* Close the postmaster's sockets (as soon as we know them) */ ClosePostmasterPorts(strcmp(argv[1], "--forklog") == 0); @@ -5360,7 +5416,6 @@ CountChildren(int target) return cnt; } - /* * StartChildProcess -- start an auxiliary process for the postmaster * @@ -5371,36 +5426,50 @@ CountChildren(int target) * to start subprocess. */ static pid_t -StartChildProcess(AuxProcType type) +StartChildProcess(ForkProcType type) { pid_t pid; - char *av[10]; - int ac = 0; - char typebuf[32]; - - /* - * Set up command-line arguments for subprocess - */ - av[ac++] = "postgres"; + ForkProcData *fork_data; + MemoryContext old_ctx; + + /* Keep fork data after splitting from postmaster */ + old_ctx = MemoryContextSwitchTo(TopMemoryContext); + fork_data = (ForkProcData *) palloc0(sizeof(ForkProcData)); + fork_data->av = palloc0(sizeof(char *) * 10); + MyForkProcType = type; + + switch(type) + { + /* Auxiliary Processes */ + case CheckerFork: + case BootstrapFork: + case StartupFork: + case BgWriterFork: + case CheckpointerFork: + case WalWriterFork: + case WalReceiverFork: + PrepAuxProcessFork(fork_data); + break; -#ifdef EXEC_BACKEND - av[ac++] = "--forkboot"; - av[ac++] = NULL; /* filled in by postmaster_forkexec */ -#endif + default: + break; + } - snprintf(typebuf, sizeof(typebuf), "-x%d", type); - av[ac++] = typebuf; + MemoryContextSwitchTo(old_ctx); - av[ac] = NULL; - Assert(ac < lengthof(av)); + /* Bail out if pre-conditions are not met */ + if (fork_data == NULL) + return 0; #ifdef EXEC_BACKEND - pid = postmaster_forkexec(ac, av); -#else /* !EXEC_BACKEND */ + pid = postmaster_forkexec(fork_data->ac, fork_data->av); +#else pid = fork_process(); +#endif if (pid == 0) /* child */ { +#ifndef EXEC_BACKEND InitPostmasterChild(); /* Close the postmaster's sockets */ @@ -5410,52 +5479,30 @@ StartChildProcess(AuxProcType type) MemoryContextSwitchTo(TopMemoryContext); MemoryContextDelete(PostmasterContext); PostmasterContext = NULL; +#endif + /* Call the process's main subroutine */ + fork_data->child_main (fork_data->ac, fork_data->av); - AuxiliaryProcessMain(ac, av); +#ifndef EXEC_BACKEND ExitPostmaster(0); +#endif } -#endif /* EXEC_BACKEND */ - - if (pid < 0) + else if (pid < 0) { /* in parent, fork failed */ - int save_errno = errno; + int save_errno = errno; - errno = save_errno; - switch (type) - { - case StartupProcess: - ereport(LOG, - (errmsg("could not fork startup process: %m"))); - break; - case BgWriterProcess: - ereport(LOG, - (errmsg("could not fork background writer process: %m"))); - break; - case CheckpointerProcess: - ereport(LOG, - (errmsg("could not fork checkpointer process: %m"))); - break; - case WalWriterProcess: - ereport(LOG, - (errmsg("could not fork WAL writer process: %m"))); - break; - case WalReceiverProcess: - ereport(LOG, - (errmsg("could not fork WAL receiver process: %m"))); - break; - default: - ereport(LOG, - (errmsg("could not fork process: %m"))); - break; - } + errno = save_errno; + ereport(LOG, + (errmsg("could not fork %s process: %m", fork_data->type_desc))); /* * fork failure is fatal during startup, but there's no need to choke * immediately if starting other child types fails. */ - if (type == StartupProcess) + if (MyForkProcType == StartupFork) ExitPostmaster(1); + return 0; } diff --git a/src/backend/utils/init/globals.c b/src/backend/utils/init/globals.c index 3bf96de256..51657924f1 100644 --- a/src/backend/utils/init/globals.c +++ b/src/backend/utils/init/globals.c @@ -38,9 +38,10 @@ volatile uint32 QueryCancelHoldoffCount = 0; volatile uint32 CritSectionCount = 0; int MyProcPid; +int MyChildProcPid; pg_time_t MyStartTime; TimestampTz MyStartTimestamp; -struct Port *MyProcPort; +struct Port *MyProcPort = NULL; int32 MyCancelKey; int MyPMChildSlot; diff --git a/src/backend/utils/init/postinit.c b/src/backend/utils/init/postinit.c index 29c5ec7b58..c0840c33a7 100644 --- a/src/backend/utils/init/postinit.c +++ b/src/backend/utils/init/postinit.c @@ -1113,7 +1113,7 @@ process_startup_options(Port *port, bool am_superuser) av = (char **) palloc(maxac * sizeof(char *)); ac = 0; - av[ac++] = "postgres"; + av[ac++] = pstrdup("postgres"); pg_split_opts(av, &ac, port->cmdline_options); diff --git a/src/include/bootstrap/bootstrap.h b/src/include/bootstrap/bootstrap.h index 85705602cb..3c53454069 100644 --- a/src/include/bootstrap/bootstrap.h +++ b/src/include/bootstrap/bootstrap.h @@ -15,7 +15,7 @@ #define BOOTSTRAP_H #include "nodes/execnodes.h" - +#include "postmaster/fork_process.h" /* * MAXATTR is the maximum number of attributes in a relation supported diff --git a/src/include/postmaster/fork_process.h b/src/include/postmaster/fork_process.h index eb3c8ec974..d912229055 100644 --- a/src/include/postmaster/fork_process.h +++ b/src/include/postmaster/fork_process.h @@ -12,6 +12,39 @@ #ifndef FORK_PROCESS_H #define FORK_PROCESS_H +#include "postmaster.h" + +typedef enum +{ + NoForkProcess = -1, + CheckerFork = 0, + BootstrapFork, + StartupFork, + BgWriterFork, + CheckpointerFork, + WalWriterFork, + WalReceiverFork, /* end of Auxiliary Process Forks */ + + NUMFORKPROCTYPES /* Must be last! */ +} ForkProcType; + +extern ForkProcType MyForkProcType; + +struct ForkProcData; + +typedef void (*ChildProcessMain) (int argc, char *argv[]); + +typedef struct ForkProcData +{ + char **av; + int ac; + int save_errno; /* for failure */ + char *type_desc; + ChildProcessMain postmaster_main; + ChildProcessMain child_main; + ChildProcessMain fail_main; +} ForkProcData; + extern pid_t fork_process(void); #endif /* FORK_PROCESS_H */ -- 2.23.0