From 585b3e6506e3d183219b72506e69213775d3eb2a Mon Sep 17 00:00:00 2001 From: Mike Palmiotto Date: Wed, 4 Mar 2020 02:04:50 +0000 Subject: [PATCH 05/11] Drop MyBackend and handle backend fork failure --- src/backend/postmaster/bgworker.c | 4 +- src/backend/postmaster/postmaster.c | 185 ++++++++++++-------- src/backend/postmaster/startup.c | 2 +- src/include/postmaster/bgworker_internals.h | 3 +- src/include/postmaster/postmaster.h | 2 +- src/include/postmaster/startup.h | 2 +- src/include/postmaster/subprocess.h | 2 +- 7 files changed, 121 insertions(+), 79 deletions(-) diff --git a/src/backend/postmaster/bgworker.c b/src/backend/postmaster/bgworker.c index 7993a0e360..1ac262fae4 100644 --- a/src/backend/postmaster/bgworker.c +++ b/src/backend/postmaster/bgworker.c @@ -137,7 +137,7 @@ static const struct static bgworker_main_type LookupBackgroundWorkerFunction(const char *libraryname, const char *funcname); static bool assign_backendlist_entry(RegisteredBgWorker *rw); -bool BackgroundWorkerCleanup(int argc, char *argv[]); +bool BackgroundWorkerCleanup(int child_errno); void BackgroundWorkerParentMain(int argc, char *argv[]); /* @@ -934,7 +934,7 @@ BackgroundWorkerMain(pg_attribute_unused() int argc, pg_attribute_unused() char } bool -BackgroundWorkerCleanup(pg_attribute_unused() int argc, pg_attribute_unused() char *argv[]) +BackgroundWorkerCleanup(int child_errno) { RegisteredBgWorker *rw = CurrentBgWorker; diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c index ffc3afa9d2..55f4067f79 100644 --- a/src/backend/postmaster/postmaster.c +++ b/src/backend/postmaster/postmaster.c @@ -145,7 +145,6 @@ static Backend *ShmemBackendArray; BackgroundWorker *MyBgworkerEntry = NULL; RegisteredBgWorker *CurrentBgWorker = NULL; -static Backend *MyBackend; static int child_errno; /* Struct containing postmaster subprocess control info */ @@ -517,6 +516,68 @@ HANDLE PostmasterHandle; static Port *ConnProcPort = NULL; +/* + * BackendPrep + * + * Do all of the pre-fork setup for a new Backend. +*/ +int +BackendPrep(int argc, char *argv[]) +{ + /* + * Create backend data structure. Better before the fork() so we can + * handle failure cleanly. + */ + Backend *bn; + + bn = (Backend *) malloc(sizeof(Backend)); + if (!bn) + { + ereport(LOG, + (errcode(ERRCODE_OUT_OF_MEMORY), + errmsg("out of memory"))); + } + + /* + * 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"))); + } + + bn->cancel_key = MyCancelKey; + + /* Pass down canAcceptConnections state */ + ConnProcPort->canAcceptConnections = canAcceptConnections(BACKEND_TYPE_NORMAL); + bn->dead_end = (ConnProcPort->canAcceptConnections != CAC_OK && + ConnProcPort->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; + + /* + * Push the Backend to the stack as-is so it can be retreived by the parent + * on the other side of the fork/exec. They will modify this entry in place. + */ + dlist_push_head(&BackendList, &bn->elem); + + return 0; +} + /* * BackendMain * @@ -546,23 +607,39 @@ void BackendMain(pg_attribute_unused() int argc, pg_attribute_unused() char *arg */ void BackendParentMain(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))); + dlist_mutable_iter iter; /* - * Everything's been successful, it's safe to add this backend to our list - * of backends. + * Find our backend node by popping the element with a matching child slot + * off the list. This was pushed to the stack (incomplete) by BackendPrep. + * We need to fix it up and push back. */ - MyBackend->pid = MyChildProcPid; - MyBackend->bkend_type = BACKEND_TYPE_NORMAL; /* Can change later to WALSND */ - dlist_push_head(&BackendList, &MyBackend->elem); + dlist_foreach_modify(iter, &BackendList) + { + Backend *bp = dlist_container(Backend, elem, iter.cur); + + /* MyPMChildSlot is guaranteed to match the child MyPMChildSlot */ + if (bp->child_slot == MyPMChildSlot) + { + /* + * Everything's been successful, it's safe to add this backend to our list + * of backends. + */ + bp->pid = MyChildProcPid; + bp->bkend_type = BACKEND_TYPE_NORMAL; /* Can change later to WALSND */ #ifdef EXEC_BACKEND - if (!MyBackend->dead_end) - ShmemBackendArrayAdd(MyBackend); + if (!bp->dead_end) + ShmemBackendArrayAdd(bp); #endif + break; /* There is only one entry and we've found it */ + } + } + + /* in parent, successful fork */ + ereport(DEBUG2, + (errmsg_internal("forked new backend, pid=%d socket=%d", + (int) MyChildProcPid, (int) MyProcPort->sock))); } /* @@ -571,71 +648,37 @@ void BackendParentMain(pg_attribute_unused() int argc, pg_attribute_unused() cha * Backend cleanup in case a failure occurs forking a new Backend. */ bool -BackendCleanup(int argc, char *argv[]) +BackendCleanup(int child_errno) { - if (!MyBackend->dead_end) - (void) ReleasePostmasterChildSlot(MyBackend->child_slot); - free(MyBackend); - - report_fork_failure_to_client(MyProcPort, child_errno); - - /* Don't panic */ - return false; -} + dlist_mutable_iter iter; -/* - * BackendPrep - * - * Prepare a ForkProcType struct for starting a Backend. - * This does all prep related to av parameters and error messages. -*/ -int -BackendPrep(int argc, char *argv[]) -{ - /* - * Create backend data structure. Better before the fork() so we can - * handle failure cleanly. - */ - MyBackend = (Backend *) malloc(sizeof(Backend)); - if (!MyBackend) + dlist_foreach_modify(iter, &BackendList) { - ereport(LOG, - (errcode(ERRCODE_OUT_OF_MEMORY), - errmsg("out of memory"))); - } + Backend *bp = dlist_container(Backend, elem, iter.cur); - /* - * 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"))); + if (bp->child_slot == MyPMChildSlot) + { + if (!bp->dead_end) + { + if (!ReleasePostmasterChildSlot(bp->child_slot)) + { + /* Could not release child slot! Panic! */ + return true; + } +#ifdef EXEC_BACKEND + ShmemBackendArrayRemove(bp); +#endif + } + dlist_delete(iter.cur); + free(bp); + break; + } } - MyBackend->cancel_key = MyCancelKey; - - /* Pass down canAcceptConnections state */ - ConnProcPort->canAcceptConnections = canAcceptConnections(BACKEND_TYPE_NORMAL); - 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; + report_fork_failure_to_client(MyProcPort, child_errno); - return 0; + /* Don't panic */ + return false; } /* @@ -5429,7 +5472,7 @@ StartSubprocess(SubprocessType type) * Panic if the cleanup function tells us to. Otherwise, just bail * with -1. */ - if (MySubprocess->cleanup(argc, argv)) + if (MySubprocess->cleanup(child_errno)) ExitPostmaster(1); else return -1; diff --git a/src/backend/postmaster/startup.c b/src/backend/postmaster/startup.c index 8957b341b9..975492e674 100644 --- a/src/backend/postmaster/startup.c +++ b/src/backend/postmaster/startup.c @@ -128,7 +128,7 @@ HandleStartupProcInterrupts(void) } bool -StartupCleanup(int argc, char *argv[]) +StartupCleanup(int child_errno) { /* Panic! */ return true; diff --git a/src/include/postmaster/bgworker_internals.h b/src/include/postmaster/bgworker_internals.h index 366bbcf1e3..aac31980fc 100644 --- a/src/include/postmaster/bgworker_internals.h +++ b/src/include/postmaster/bgworker_internals.h @@ -58,8 +58,7 @@ extern void ResetBackgroundWorkerCrashTimes(void); extern int BgWorkerPrep(int argc, char *argv[]); extern void BackgroundWorkerMain(int argc, char *argv[]); extern void BackgroundWorkerParentMain(int argc, char *argv[]); -extern bool BackgroundWorkerCleanup(int argc, char *argv[]); - +extern bool BackgroundWorkerCleanup(int child_errno); #ifdef EXEC_BACKEND extern BackgroundWorker *BackgroundWorkerEntry(int slotno); diff --git a/src/include/postmaster/postmaster.h b/src/include/postmaster/postmaster.h index d4c0f46220..f13fcc880a 100644 --- a/src/include/postmaster/postmaster.h +++ b/src/include/postmaster/postmaster.h @@ -94,7 +94,7 @@ extern bool RandomCancelKey(int32 *cancel_key); extern int BackendPrep(int argc, char *argv[]); extern void BackendMain(int argc, char *argv[]); extern void BackendParentMain(int argc, char *argv[]); -extern bool BackendCleanup(int argc, char *argv[]); +extern bool BackendCleanup(int child_errno); extern void BackgroundWorkerMain(int argc, char *argv[]); #ifdef EXEC_BACKEND diff --git a/src/include/postmaster/startup.h b/src/include/postmaster/startup.h index 14a646136f..6a8108dfe0 100644 --- a/src/include/postmaster/startup.h +++ b/src/include/postmaster/startup.h @@ -20,6 +20,6 @@ extern void ResetPromoteTriggered(void); /* Startup subprocess functions */ extern void StartupProcessMain(int argc, char *argv[]) pg_attribute_noreturn(); -extern bool StartupCleanup(int argc, char *argv[]); +extern bool StartupCleanup(int child_errno); #endif /* _STARTUP_H */ diff --git a/src/include/postmaster/subprocess.h b/src/include/postmaster/subprocess.h index b65fc75475..2634bbd142 100644 --- a/src/include/postmaster/subprocess.h +++ b/src/include/postmaster/subprocess.h @@ -39,7 +39,7 @@ typedef enum typedef int (*SubprocessInit) (int argc, char *argv[]); typedef void (*SubprocessEntryPoint) (int argc, char *argv[]); -typedef bool (*SubprocessCleanup) (int argc, char *argv[]); +typedef bool (*SubprocessCleanup) (int child_errno); typedef void (*SubprocessParent) (int argc, char *argv[]); /* Current subprocess initializer */ -- 2.21.0