From 388491f1e5e63fe97c7cca26d18b64321973d423 Mon Sep 17 00:00:00 2001 From: Heikki Linnakangas Date: Sun, 3 Dec 2023 16:38:54 +0200 Subject: [PATCH] Pass BackgroundWorker entry in the parameter file in EXEC_BACKEND mode The BackgroundWorker struct is now passed the same way as the Port struct. Seems more consistent. This makes it possible to move InitProcess later in SubPostmasterMain (in next commit), as we no longer need to access shared memory to read background worker entry. Reviewed-by: Tristan Partin, Andres Freund, Alexander Lakhin Discussion: https://www.postgresql.org/message-id/7a59b073-5b5b-151e-7ed3-8b01ff7ce9ef@iki.fi --- src/backend/postmaster/bgworker.c | 23 +--- src/backend/postmaster/postmaster.c | 120 ++++++++++++-------- src/include/postmaster/bgworker_internals.h | 4 - 3 files changed, 75 insertions(+), 72 deletions(-) diff --git a/src/backend/postmaster/bgworker.c b/src/backend/postmaster/bgworker.c index 911bf24a7c..d936986c2b 100644 --- a/src/backend/postmaster/bgworker.c +++ b/src/backend/postmaster/bgworker.c @@ -350,7 +350,7 @@ BackgroundWorkerStateChange(bool allow_new_workers) */ rw = MemoryContextAllocExtended(PostmasterContext, sizeof(RegisteredBgWorker), - MCXT_ALLOC_NO_OOM); + MCXT_ALLOC_NO_OOM | MCXT_ALLOC_ZERO); if (rw == NULL) { ereport(LOG, @@ -631,27 +631,6 @@ ResetBackgroundWorkerCrashTimes(void) } } -#ifdef EXEC_BACKEND -/* - * In EXEC_BACKEND mode, workers use this to retrieve their details from - * shared memory. - */ -BackgroundWorker * -BackgroundWorkerEntry(int slotno) -{ - static BackgroundWorker myEntry; - BackgroundWorkerSlot *slot; - - Assert(slotno < BackgroundWorkerData->total_slots); - slot = &BackgroundWorkerData->slot[slotno]; - Assert(slot->in_use); - - /* must copy this in case we don't intend to retain shmem access */ - memcpy(&myEntry, &slot->worker, sizeof myEntry); - return &myEntry; -} -#endif - /* * Complain about the BackgroundWorker definition using error level elevel. * Return true if it looks ok, false if not (unless elevel >= ERROR, in diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c index 39fdcff3e2..92e51bd54d 100644 --- a/src/backend/postmaster/postmaster.c +++ b/src/backend/postmaster/postmaster.c @@ -476,7 +476,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[], Port *port, BackgroundWorker *worker); /* Type for a socket that can be inherited to a client process */ #ifdef WIN32 @@ -495,8 +495,13 @@ typedef int InheritableSocket; */ typedef struct { + bool has_port; Port port; InheritableSocket portsocket; + + bool has_bgworker; + BackgroundWorker bgworker; + char DataDir[MAXPGPATH]; int32 MyCancelKey; int MyPMChildSlot; @@ -542,13 +547,13 @@ typedef struct char pkglib_path[MAXPGPATH]; } BackendParameters; -static void read_backend_variables(char *id, Port *port); -static void restore_backend_variables(BackendParameters *param, Port *port); +static void read_backend_variables(char *id, Port **port, BackgroundWorker **worker); +static void restore_backend_variables(BackendParameters *param, Port **port, BackgroundWorker **worker); #ifndef WIN32 -static bool save_backend_variables(BackendParameters *param, Port *port); +static bool save_backend_variables(BackendParameters *param, Port *port, BackgroundWorker *worker); #else -static bool save_backend_variables(BackendParameters *param, Port *port, +static bool save_backend_variables(BackendParameters *param, Port *port, BackgroundWorker *worker, HANDLE childProcess, pid_t childPid); #endif @@ -4441,11 +4446,7 @@ 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); + return internal_forkexec(argc, argv, NULL, NULL); } /* @@ -4470,7 +4471,7 @@ backend_forkexec(Port *port) av[ac] = NULL; Assert(ac < lengthof(av)); - return internal_forkexec(ac, av, port); + return internal_forkexec(ac, av, port, NULL); } #ifndef WIN32 @@ -4482,7 +4483,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[], Port *port, BackgroundWorker *worker) { static unsigned long tmpBackendFileNum = 0; pid_t pid; @@ -4498,7 +4499,7 @@ internal_forkexec(int argc, char *argv[], Port *port) */ memset(¶m, 0, sizeof(BackendParameters)); - if (!save_backend_variables(¶m, port)) + if (!save_backend_variables(¶m, port, worker)) return -1; /* log made by save_backend_variables */ /* Calculate name for temp file */ @@ -4582,7 +4583,7 @@ internal_forkexec(int argc, char *argv[], Port *port) * file is complete. */ static pid_t -internal_forkexec(int argc, char *argv[], Port *port) +internal_forkexec(int argc, char *argv[], Port *port, BackgroundWorker *worker) { int retry_count = 0; STARTUPINFO si; @@ -4679,7 +4680,7 @@ retry: return -1; } - if (!save_backend_variables(param, port, pi.hProcess, pi.dwProcessId)) + if (!save_backend_variables(param, port, worker, pi.hProcess, pi.dwProcessId)) { /* * log made by save_backend_variables, but we have to clean up the @@ -4796,7 +4797,8 @@ retry: void SubPostmasterMain(int argc, char *argv[]) { - Port port; + Port *port; + BackgroundWorker *worker; /* In EXEC_BACKEND case we will not have inherited these settings */ IsPostmasterEnvironment = true; @@ -4810,8 +4812,7 @@ SubPostmasterMain(int argc, char *argv[]) elog(FATAL, "invalid subpostmaster invocation"); /* Read in the variables file */ - memset(&port, 0, sizeof(Port)); - read_backend_variables(argv[2], &port); + read_backend_variables(argv[2], &port, &worker); /* Close the postmaster's sockets (as soon as we know them) */ ClosePostmasterPorts(strcmp(argv[1], "--forklog") == 0); @@ -4839,7 +4840,7 @@ SubPostmasterMain(int argc, char *argv[]) strcmp(argv[1], "--forkavlauncher") == 0 || strcmp(argv[1], "--forkavworker") == 0 || strcmp(argv[1], "--forkaux") == 0 || - strncmp(argv[1], "--forkbgworker=", 15) == 0) + strcmp(argv[1], "--forkbgworker") == 0) PGSharedMemoryReAttach(); else PGSharedMemoryNoReAttach(); @@ -4912,7 +4913,7 @@ SubPostmasterMain(int argc, char *argv[]) * PGPROC slots, we have already initialized libpq and are able to * report the error to the client. */ - BackendInitialize(&port); + BackendInitialize(port); /* Restore basic shared memory pointers */ InitShmemAccess(UsedShmemSegAddr); @@ -4924,7 +4925,7 @@ SubPostmasterMain(int argc, char *argv[]) AttachSharedMemoryStructs(); /* And run the backend */ - BackendRun(&port); /* does not return */ + BackendRun(port); /* does not return */ } if (strcmp(argv[1], "--forkaux") == 0) { @@ -4970,10 +4971,8 @@ SubPostmasterMain(int argc, char *argv[]) AutoVacWorkerMain(argc - 2, argv + 2); /* does not return */ } - if (strncmp(argv[1], "--forkbgworker=", 15) == 0) + if (strcmp(argv[1], "--forkbgworker") == 0) { - int shmem_slot; - /* do this as early as possible; in particular, before InitProcess() */ IsBackgroundWorker = true; @@ -4986,10 +4985,7 @@ SubPostmasterMain(int argc, char *argv[]) /* Attach process to shared data structures */ AttachSharedMemoryStructs(); - /* Fetch MyBgworkerEntry from shared memory */ - shmem_slot = atoi(argv[1] + 15); - MyBgworkerEntry = BackgroundWorkerEntry(shmem_slot); - + MyBgworkerEntry = worker; BackgroundWorkerMain(); } if (strcmp(argv[1], "--forklog") == 0) @@ -5648,22 +5644,19 @@ BackgroundWorkerUnblockSignals(void) #ifdef EXEC_BACKEND static pid_t -bgworker_forkexec(int shmem_slot) +bgworker_forkexec(BackgroundWorker *worker) { 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++] = "--forkbgworker"; + av[ac++] = NULL; /* filled in by internal_forkexec */ av[ac] = NULL; Assert(ac < lengthof(av)); - return postmaster_forkexec(ac, av); + return internal_forkexec(ac, av, NULL, worker); } #endif @@ -5704,7 +5697,7 @@ do_start_bgworker(RegisteredBgWorker *rw) rw->rw_worker.bgw_name))); #ifdef EXEC_BACKEND - switch ((worker_pid = bgworker_forkexec(rw->rw_shmem_slot))) + switch ((worker_pid = bgworker_forkexec(&rw->rw_worker))) #else switch ((worker_pid = fork_process())) #endif @@ -6037,16 +6030,36 @@ static void read_inheritable_socket(SOCKET *dest, InheritableSocket *src); /* Save critical backend variables into the BackendParameters struct */ #ifndef WIN32 static bool -save_backend_variables(BackendParameters *param, Port *port) +save_backend_variables(BackendParameters *param, Port *port, BackgroundWorker *worker) #else static bool -save_backend_variables(BackendParameters *param, Port *port, +save_backend_variables(BackendParameters *param, Port *port, BackgroundWorker *worker, HANDLE childProcess, pid_t childPid) #endif { - memcpy(¶m->port, port, sizeof(Port)); - if (!write_inheritable_socket(¶m->portsocket, port->sock, childPid)) - return false; + if (port) + { + memcpy(¶m->port, port, sizeof(Port)); + if (!write_inheritable_socket(¶m->portsocket, port->sock, childPid)) + return false; + param->has_port = true; + } + else + { + memset(¶m->port, 0, sizeof(Port)); + param->has_port = false; + } + + if (worker) + { + memcpy(¶m->bgworker, worker, sizeof(BackgroundWorker)); + param->has_bgworker = true; + } + else + { + memset(¶m->bgworker, 0, sizeof(BackgroundWorker)); + param->has_bgworker = false; + } strlcpy(param->DataDir, DataDir, MAXPGPATH); @@ -6202,7 +6215,7 @@ read_inheritable_socket(SOCKET *dest, InheritableSocket *src) #endif static void -read_backend_variables(char *id, Port *port) +read_backend_variables(char *id, Port **port, BackgroundWorker **worker) { BackendParameters param; @@ -6269,15 +6282,30 @@ read_backend_variables(char *id, Port *port) } #endif - restore_backend_variables(¶m, port); + restore_backend_variables(¶m, port, worker); } /* Restore critical backend variables from the BackendParameters struct */ static void -restore_backend_variables(BackendParameters *param, Port *port) +restore_backend_variables(BackendParameters *param, Port **port, BackgroundWorker **worker) { - memcpy(port, ¶m->port, sizeof(Port)); - read_inheritable_socket(&port->sock, ¶m->portsocket); + if (param->has_port) + { + *port = (Port *) MemoryContextAlloc(TopMemoryContext, sizeof(Port)); + memcpy(*port, ¶m->port, sizeof(Port)); + read_inheritable_socket(&(*port)->sock, ¶m->portsocket); + } + else + *port = NULL; + + if (param->has_bgworker) + { + *worker = (BackgroundWorker *) + MemoryContextAlloc(TopMemoryContext, sizeof(BackgroundWorker)); + memcpy(*worker, ¶m->bgworker, sizeof(BackgroundWorker)); + } + else + *worker = NULL; SetDataDir(param->DataDir); diff --git a/src/include/postmaster/bgworker_internals.h b/src/include/postmaster/bgworker_internals.h index 09df054fcc..323f1e0729 100644 --- a/src/include/postmaster/bgworker_internals.h +++ b/src/include/postmaster/bgworker_internals.h @@ -57,8 +57,4 @@ extern void ResetBackgroundWorkerCrashTimes(void); /* Entry point for background worker processes */ extern void BackgroundWorkerMain(void) pg_attribute_noreturn(); -#ifdef EXEC_BACKEND -extern BackgroundWorker *BackgroundWorkerEntry(int slotno); -#endif - #endif /* BGWORKER_INTERNALS_H */