v1-0001-Inherit-pg_control-snapshot-into-EXEC_BACKEND-sub.patch

application/octet-stream

Filename: v1-0001-Inherit-pg_control-snapshot-into-EXEC_BACKEND-sub.patch
Type: application/octet-stream
Part: 0
Message: Re: Incorrect checksum in control file with pg_rewind test

Patch

Same data as JSON: GET /api/v1/attachments/:id/patch the parsed metadata as JSON — format, series position, per-file stats; never the diff bytes. API reference →
Format: format-patch
Series: patch v1-0001
Subject: Inherit pg_control snapshot into EXEC_BACKEND sub-processes
File+
src/backend/access/transam/xlog.c 60 29
src/backend/postmaster/launch_backend.c 29 3
src/backend/postmaster/postmaster.c 2 2
src/backend/tcop/postgres.c 1 1
src/include/access/xlog.h 3 1
From 78a5a6a977352b2d6de9851299db7e3ca47080cc Mon Sep 17 00:00:00 2001
From: Alexander Korotkov <akorotkov@postgresql.org>
Date: Tue, 21 Apr 2026 14:55:48 +0300
Subject: [PATCH v1] Inherit pg_control snapshot into EXEC_BACKEND
 sub-processes

Under EXEC_BACKEND, each sub-process re-reads pg_control from disk in
SubPostmasterMain() via LocalProcessControlFile().  That read is not
serialized against update_controlfile(), which writes the file
non-atomically in the startup process or in the checkpointer.  The
sub-process can therefore observe a torn image and fail the CRC check.

Also different sub-processes can end up with different versions of
LocalControlFile.  It doesn't seems to cause material bug yet, but it would
be better to stick fork and EXEC_BACKEND builds to the same behavior.

This commit reproduces fork behavior on  EXEC_BACKEND by passing
the postmaster's LocalControlFile copy through BackendParameters and
having the sub-process consume that snapshot instead of re-reading
from disk.

Now, LocalProcessControlFile() have a new an optional 'source' argument:
when NULL it reads from the disk as before; when non-NULL it installs the
provided snapshot and runs the same CRC, compatibility, and GUC
setup steps.  GetLocalControlFileCopy() is a small accessor that
copies out the postmaster's LocalControlFile, so launch_backend.c
does not reach into xlog.c globals directly.

Discussion: https://www.postgresql.org/message-id/f59335a4-83ff-438a-a30e-7cf2200276b6%40postgrespro.ru
---
 src/backend/access/transam/xlog.c       | 89 +++++++++++++++++--------
 src/backend/postmaster/launch_backend.c | 32 ++++++++-
 src/backend/postmaster/postmaster.c     |  4 +-
 src/backend/tcop/postgres.c             |  2 +-
 src/include/access/xlog.h               |  4 +-
 5 files changed, 95 insertions(+), 36 deletions(-)

diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index f85b5286086..fa953f5a43a 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -724,7 +724,7 @@ static void UpdateMinRecoveryPoint(XLogRecPtr lsn, bool force);
 static bool PerformRecoveryXLogAction(void);
 static void InitControlFile(uint64 sysidentifier, uint32 data_checksum_version);
 static void WriteControlFile(void);
-static void ReadControlFile(void);
+static void ReadControlFile(ControlFileData *source);
 static void UpdateControlFile(void);
 static char *str_time(pg_time_t tnow, char *buf, size_t bufsize);
 
@@ -4407,42 +4407,56 @@ WriteControlFile(void)
 }
 
 static void
-ReadControlFile(void)
+ReadControlFile(ControlFileData *source)
 {
 	pg_crc32c	crc;
-	int			fd;
 	char		wal_segsz_str[20];
-	int			r;
 
 	/*
-	 * Read data...
+	 * If a source snapshot was provided, use it instead of reading from disk.
+	 * This path is used under EXEC_BACKEND to inherit pg_control from the
+	 * postmaster via BackendParameters, matching fork semantics and avoiding
+	 * torn reads of the on-disk file.
 	 */
-	fd = BasicOpenFile(XLOG_CONTROL_FILE,
-					   O_RDWR | PG_BINARY);
-	if (fd < 0)
-		ereport(PANIC,
-				(errcode_for_file_access(),
-				 errmsg("could not open file \"%s\": %m",
-						XLOG_CONTROL_FILE)));
-
-	pgstat_report_wait_start(WAIT_EVENT_CONTROL_FILE_READ);
-	r = read(fd, ControlFile, sizeof(ControlFileData));
-	if (r != sizeof(ControlFileData))
+	if (source)
+	{
+		memcpy(ControlFile, source, sizeof(ControlFileData));
+	}
+	else
 	{
-		if (r < 0)
+		int			fd;
+		int			r;
+
+		/*
+		 * Read data...
+		 */
+		fd = BasicOpenFile(XLOG_CONTROL_FILE,
+						   O_RDWR | PG_BINARY);
+		if (fd < 0)
 			ereport(PANIC,
 					(errcode_for_file_access(),
-					 errmsg("could not read file \"%s\": %m",
+					 errmsg("could not open file \"%s\": %m",
 							XLOG_CONTROL_FILE)));
-		else
-			ereport(PANIC,
-					(errcode(ERRCODE_DATA_CORRUPTED),
-					 errmsg("could not read file \"%s\": read %d of %zu",
-							XLOG_CONTROL_FILE, r, sizeof(ControlFileData))));
-	}
-	pgstat_report_wait_end();
 
-	close(fd);
+		pgstat_report_wait_start(WAIT_EVENT_CONTROL_FILE_READ);
+		r = read(fd, ControlFile, sizeof(ControlFileData));
+		if (r != sizeof(ControlFileData))
+		{
+			if (r < 0)
+				ereport(PANIC,
+						(errcode_for_file_access(),
+						 errmsg("could not read file \"%s\": %m",
+								XLOG_CONTROL_FILE)));
+			else
+				ereport(PANIC,
+						(errcode(ERRCODE_DATA_CORRUPTED),
+						 errmsg("could not read file \"%s\": read %d of %zu",
+								XLOG_CONTROL_FILE, r, sizeof(ControlFileData))));
+		}
+		pgstat_report_wait_end();
+
+		close(fd);
+	}
 
 	/*
 	 * Check for expected pg_control format version.  If this is wrong, the
@@ -4640,6 +4654,19 @@ UpdateControlFile(void)
 	update_controlfile(DataDir, ControlFile, true);
 }
 
+/*
+ * Copy the postmaster's process-local pg_control image into *dest.  This is
+ * the same in-memory copy that a forked backend inherits in its address
+ * space, and it is used under EXEC_BACKEND to hand the image to sub-processes
+ * via BackendParameters instead of having them re-read pg_control from disk.
+ */
+void
+GetLocalControlFileCopy(ControlFileData *dest)
+{
+	Assert(LocalControlFile != NULL);
+	memcpy(dest, LocalControlFile, sizeof(ControlFileData));
+}
+
 /*
  * Returns the unique system identifier from control file.
  */
@@ -5264,14 +5291,18 @@ show_effective_wal_level(void)
  *
  * reset just controls whether previous contents are to be expected (in the
  * reset case, there's a dangling pointer into old shared memory), or not.
+ *
+ * If source is non-NULL, its contents are used instead of reading from disk.
+ * This supports EXEC_BACKEND sub-processes that inherit pg_control from the
+ * postmaster via BackendParameters.
  */
 void
-LocalProcessControlFile(bool reset)
+LocalProcessControlFile(bool reset, struct ControlFileData *source)
 {
 	Assert(reset || ControlFile == NULL);
 	LocalControlFile = palloc_object(ControlFileData);
 	ControlFile = LocalControlFile;
-	ReadControlFile();
+	ReadControlFile(source);
 	SetLocalDataChecksumState(ControlFile->data_checksum_version);
 }
 
@@ -5611,7 +5642,7 @@ BootStrapXLOG(uint32 data_checksum_version)
 	 * Force control file to be read - in contrast to normal processing we'd
 	 * otherwise never run the checks and GUC related initializations therein.
 	 */
-	ReadControlFile();
+	ReadControlFile(NULL);
 }
 
 static char *
diff --git a/src/backend/postmaster/launch_backend.c b/src/backend/postmaster/launch_backend.c
index 8f3cfea880c..75c6853dece 100644
--- a/src/backend/postmaster/launch_backend.c
+++ b/src/backend/postmaster/launch_backend.c
@@ -56,6 +56,7 @@
 #include "utils/memutils.h"
 
 #ifdef EXEC_BACKEND
+#include "catalog/pg_control.h"
 #include "nodes/queryjumble.h"
 #include "portability/instr_time.h"
 #include "storage/pg_shmem.h"
@@ -141,6 +142,13 @@ typedef struct
 	ClientSocket client_sock;
 	InheritableSocket inh_sock;
 
+	/*
+	 * Snapshot of pg_control taken by the postmaster.  Inherited by the
+	 * sub-process in place of re-reading the file from disk, to avoid torn
+	 * reads and to match fork() inheritance semantics.
+	 */
+	ControlFileData ControlFile;
+
 	/*
 	 * Extra startup data, content depends on the child process.
 	 */
@@ -150,6 +158,13 @@ typedef struct
 
 #define SizeOfBackendParameters(startup_data_len) (offsetof(BackendParameters, startup_data) + startup_data_len)
 
+/*
+ * Snapshot of pg_control inherited from the postmaster.  Populated from
+ * BackendParameters in restore_backend_variables() and then consumed by the
+ * call to LocalProcessControlFile() in SubPostmasterMain().
+ */
+static ControlFileData InheritedControlFile;
+
 static void read_backend_variables(char *id, void **startup_data, size_t *startup_data_len);
 static void restore_backend_variables(BackendParameters *param);
 
@@ -662,10 +677,11 @@ SubPostmasterMain(int argc, char *argv[])
 	checkDataDir();
 
 	/*
-	 * (re-)read control file, as it contains config. The postmaster will
-	 * already have read this, but this process doesn't know about that.
+	 * Set up control file from the snapshot the postmaster captured for us.
+	 * Reading pg_control from disk here would race with concurrent updates
+	 * from the startup process or checkpointer.
 	 */
-	LocalProcessControlFile(false);
+	LocalProcessControlFile(false, &InheritedControlFile);
 
 	RegisterBuiltinShmemCallbacks();
 
@@ -772,6 +788,14 @@ save_backend_variables(BackendParameters *param,
 
 	strlcpy(param->pkglib_path, pkglib_path, MAXPGPATH);
 
+	/*
+	 * Hand the postmaster's process-local pg_control image to the
+	 * sub-process.  This mirrors what fork() inheritance does on platforms
+	 * without EXEC_BACKEND, and it avoids re-reading pg_control from disk
+	 * concurrently with the startup process or checkpointer updating it.
+	 */
+	GetLocalControlFileCopy(&param->ControlFile);
+
 	param->startup_data_len = startup_data_len;
 	if (startup_data_len > 0)
 		memcpy(param->startup_data, startup_data, startup_data_len);
@@ -1029,6 +1053,8 @@ restore_backend_variables(BackendParameters *param)
 
 	strlcpy(pkglib_path, param->pkglib_path, MAXPGPATH);
 
+	memcpy(&InheritedControlFile, &param->ControlFile, sizeof(ControlFileData));
+
 	/*
 	 * 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/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index b6fd332f196..9c92ac09bd3 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -916,7 +916,7 @@ PostmasterMain(int argc, char *argv[])
 	 * processes will inherit the correct function pointer and not need to
 	 * repeat the test.
 	 */
-	LocalProcessControlFile(false);
+	LocalProcessControlFile(false, NULL);
 
 	/*
 	 * Register the apply launcher.  It's probably a good idea to call this
@@ -3265,7 +3265,7 @@ PostmasterStateMachine(void)
 		shmem_exit(1);
 
 		/* re-read control file into local memory */
-		LocalProcessControlFile(true);
+		LocalProcessControlFile(true, NULL);
 
 		/*
 		 * Re-initialize shared memory and semaphores.  Note: We don't call
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index 2c1f14b7889..a35d99649e4 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -4175,7 +4175,7 @@ PostgresSingleUserMain(int argc, char *argv[],
 	CreateDataDirLockFile(false);
 
 	/* read control file (error checking and contains config ) */
-	LocalProcessControlFile(false);
+	LocalProcessControlFile(false, NULL);
 
 	/* Register the shared memory needs of all core subsystems. */
 	RegisterBuiltinShmemCallbacks();
diff --git a/src/include/access/xlog.h b/src/include/access/xlog.h
index 437b4f32349..aab638e6a10 100644
--- a/src/include/access/xlog.h
+++ b/src/include/access/xlog.h
@@ -261,7 +261,9 @@ extern bool GetDefaultCharSignedness(void);
 extern XLogRecPtr GetFakeLSNForUnloggedRel(void);
 extern void BootStrapXLOG(uint32 data_checksum_version);
 extern void InitializeWalConsistencyChecking(void);
-extern void LocalProcessControlFile(bool reset);
+struct ControlFileData;
+extern void LocalProcessControlFile(bool reset, struct ControlFileData *source);
+extern void GetLocalControlFileCopy(struct ControlFileData *dest);
 extern WalLevel GetActiveWalLevelOnStandby(void);
 extern void StartupXLOG(void);
 extern void ShutdownXLOG(int code, Datum arg);
-- 
2.39.5 (Apple Git-154)