v4-0002-Pass-BackgroundWorker-entry-in-the-parameter-file.patch

From f4c1efd7a565d0f5856dd0e4fb7bcfb583190185 Mon Sep 17 00:00:00 2001
From: Heikki Linnakangas <heikki.linnakangas@iki.fi>
Date: Fri, 1 Dec 2023 11:04:01 +0200
Subject: [PATCH v4 2/4] Pass BackgroundWorker entry in the parameter file in
 EXEC_BACKEND mode

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
Discussion: https://www.postgresql.org/message-id/7a59b073-5b5b-151e-7ed3-8b01ff7ce9ef@iki.fi
---
 src/backend/postmaster/bgworker.c           | 21 -----------
 src/backend/postmaster/postmaster.c         | 40 +++++++++++++--------
 src/include/postmaster/bgworker_internals.h |  4 ---
 3 files changed, 25 insertions(+), 40 deletions(-)

diff --git a/src/backend/postmaster/bgworker.c b/src/backend/postmaster/bgworker.c
index 911bf24a7cb..c4e8cdb45d2 100644
--- a/src/backend/postmaster/bgworker.c
+++ b/src/backend/postmaster/bgworker.c
@@ -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 b03e88e6702..135dc5a5f49 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -540,6 +540,8 @@ typedef struct
 #endif
 	char		my_exec_path[MAXPGPATH];
 	char		pkglib_path[MAXPGPATH];
+
+	BackgroundWorker MyBgworkerEntry;
 } BackendParameters;
 
 static void read_backend_variables(char *id, Port *port);
@@ -4831,7 +4833,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();
@@ -4962,10 +4964,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;
 
@@ -4978,10 +4978,6 @@ 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);
-
 		BackgroundWorkerMain();
 	}
 	if (strcmp(argv[1], "--forklog") == 0)
@@ -5640,22 +5636,24 @@ 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);
+	pid_t		result;
 
 	av[ac++] = "postgres";
-	av[ac++] = forkav;
+	av[ac++] = "--forkbgworker";
 	av[ac++] = NULL;			/* filled in by postmaster_forkexec */
 	av[ac] = NULL;
 
 	Assert(ac < lengthof(av));
 
-	return postmaster_forkexec(ac, av);
+	MyBgworkerEntry = worker;
+	result = postmaster_forkexec(ac, av);
+	MyBgworkerEntry = NULL;
+
+	return result;
 }
 #endif
 
@@ -5696,7 +5694,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
@@ -6096,6 +6094,11 @@ save_backend_variables(BackendParameters *param, Port *port,
 
 	strlcpy(param->pkglib_path, pkglib_path, MAXPGPATH);
 
+	if (MyBgworkerEntry)
+		memcpy(&param->MyBgworkerEntry, MyBgworkerEntry, sizeof(BackgroundWorker));
+	else
+		memset(&param->MyBgworkerEntry, 0, sizeof(BackgroundWorker));
+
 	return true;
 }
 
@@ -6324,6 +6327,13 @@ restore_backend_variables(BackendParameters *param, Port *port)
 
 	strlcpy(pkglib_path, param->pkglib_path, MAXPGPATH);
 
+	if (param->MyBgworkerEntry.bgw_name[0] != '\0')
+	{
+		MyBgworkerEntry = (BackgroundWorker *)
+			MemoryContextAlloc(TopMemoryContext, sizeof(BackgroundWorker));
+		memcpy(MyBgworkerEntry, &param->MyBgworkerEntry, sizeof(BackgroundWorker));
+	}
+
 	/*
 	 * We need to restore fd.c's counts of externally-opened FDs; to avoid
 	 * confusion, be sure to do this after restoring max_safe_fds.  (Note:
diff --git a/src/include/postmaster/bgworker_internals.h b/src/include/postmaster/bgworker_internals.h
index 09df054fcce..323f1e07291 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 */
-- 
2.39.2