v20250309-0003-sync-the-data_checksums-GUC-with-the-local.patch

From 9257d742294b27a4ee559fa31459fa0afa502b6b Mon Sep 17 00:00:00 2001
From: Tomas Vondra <tomas@vondra.me>
Date: Fri, 7 Mar 2025 23:07:44 +0100
Subject: [PATCH v20250309 3/5] sync the data_checksums GUC with the local
 variable

We now have three places that in some way express the state of data
checksums in the instance.

- control file / data_checksum_version

- LocalDataChecksumVersion as a local cache to reduce locking

- data_checksums backing the GUC

We need to keep the GUC variable in sync, to ensure we get the correct
value even early during startup (e.g. with -C).

Introduces a new SetLocalDataChecksumVersion() which sets the local
variable, and also updates the data_checksum GUC at the same time.
---
 src/backend/access/transam/xlog.c   | 51 +++++++++++++++++++++++++----
 src/backend/utils/misc/guc_tables.c |  3 +-
 src/include/access/xlog.h           |  1 +
 3 files changed, 47 insertions(+), 8 deletions(-)

diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index b5190a7e104..f137cdc6d42 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -657,6 +657,14 @@ static bool updateMinRecoveryPoint = true;
  */
 static uint32 LocalDataChecksumVersion = 0;
 
+/*
+ * Variable backing the GUC, keep it in sync with LocalDataChecksumVersion.
+ * See SetLocalDataChecksumVersion().
+ */
+int			data_checksums = 0;
+
+static void SetLocalDataChecksumVersion(uint32 data_checksum_version);
+
 /* For WALInsertLockAcquire/Release functions */
 static int	MyLockNo = 0;
 static bool holdingAllLocks = false;
@@ -4594,7 +4602,7 @@ ReadControlFile(void)
 
 	CalculateCheckpointSegments();
 
-	LocalDataChecksumVersion = ControlFile->data_checksum_version;
+	SetLocalDataChecksumVersion(ControlFile->data_checksum_version);
 }
 
 /*
@@ -4913,7 +4921,7 @@ SetDataChecksumsOff(void)
 bool
 AbsorbChecksumsOnInProgressBarrier(void)
 {
-	LocalDataChecksumVersion = PG_DATA_CHECKSUM_INPROGRESS_ON_VERSION;
+	SetLocalDataChecksumVersion(PG_DATA_CHECKSUM_INPROGRESS_ON_VERSION);
 	return true;
 }
 
@@ -4921,21 +4929,21 @@ bool
 AbsorbChecksumsOnBarrier(void)
 {
 	Assert(LocalDataChecksumVersion == PG_DATA_CHECKSUM_INPROGRESS_ON_VERSION);
-	LocalDataChecksumVersion = PG_DATA_CHECKSUM_VERSION;
+	SetLocalDataChecksumVersion(PG_DATA_CHECKSUM_VERSION);
 	return true;
 }
 
 bool
 AbsorbChecksumsOffInProgressBarrier(void)
 {
-	LocalDataChecksumVersion = PG_DATA_CHECKSUM_INPROGRESS_OFF_VERSION;
+	SetLocalDataChecksumVersion(PG_DATA_CHECKSUM_INPROGRESS_OFF_VERSION);
 	return true;
 }
 
 bool
 AbsorbChecksumsOffBarrier(void)
 {
-	LocalDataChecksumVersion = 0;
+	SetLocalDataChecksumVersion(0);
 	return true;
 }
 
@@ -4951,10 +4959,41 @@ void
 InitLocalControldata(void)
 {
 	LWLockAcquire(ControlFileLock, LW_EXCLUSIVE);
-	LocalDataChecksumVersion = ControlFile->data_checksum_version;
+	SetLocalDataChecksumVersion(ControlFile->data_checksum_version);
 	LWLockRelease(ControlFileLock);
 }
 
+/*
+ * XXX probably should be called in all places that modify the value of
+ * LocalDataChecksumVersion (to make sure data_checksums GUC is in sync)
+ *
+ * XXX aren't PG_DATA_ and DATA_ constants the same? why do we need both?
+ */
+void
+SetLocalDataChecksumVersion(uint32 data_checksum_version)
+{
+	LocalDataChecksumVersion = data_checksum_version;
+
+	switch (LocalDataChecksumVersion)
+	{
+		case PG_DATA_CHECKSUM_VERSION:
+			data_checksums = DATA_CHECKSUMS_ON;
+			break;
+
+		case PG_DATA_CHECKSUM_INPROGRESS_ON_VERSION:
+			data_checksums = DATA_CHECKSUMS_INPROGRESS_ON;
+			break;
+
+		case PG_DATA_CHECKSUM_INPROGRESS_OFF_VERSION:
+			data_checksums = DATA_CHECKSUMS_INPROGRESS_OFF;
+			break;
+
+		default:
+			data_checksums = DATA_CHECKSUMS_OFF;
+			break;
+	}
+}
+
 /* guc hook */
 const char *
 show_data_checksums(void)
diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c
index 05cba3a02e3..66abf056159 100644
--- a/src/backend/utils/misc/guc_tables.c
+++ b/src/backend/utils/misc/guc_tables.c
@@ -609,7 +609,6 @@ static int	shared_memory_size_mb;
 static int	shared_memory_size_in_huge_pages;
 static int	wal_block_size;
 static int	num_os_semaphores;
-static int	data_checksums;
 static bool integer_datetimes;
 
 #ifdef USE_ASSERT_CHECKING
@@ -5300,7 +5299,7 @@ struct config_enum ConfigureNamesEnum[] =
 		{"data_checksums", PGC_INTERNAL, PRESET_OPTIONS,
 			gettext_noop("Shows whether data checksums are turned on for this cluster."),
 			NULL,
-			GUC_NOT_IN_SAMPLE | GUC_DISALLOW_IN_FILE
+			GUC_NOT_IN_SAMPLE | GUC_DISALLOW_IN_FILE | GUC_RUNTIME_COMPUTED
 		},
 		&data_checksums,
 		DATA_CHECKSUMS_OFF, data_checksums_options,
diff --git a/src/include/access/xlog.h b/src/include/access/xlog.h
index 0bb32c9c0fc..aec3ea0bc63 100644
--- a/src/include/access/xlog.h
+++ b/src/include/access/xlog.h
@@ -56,6 +56,7 @@ extern PGDLLIMPORT int CommitDelay;
 extern PGDLLIMPORT int CommitSiblings;
 extern PGDLLIMPORT bool track_wal_io_timing;
 extern PGDLLIMPORT int wal_decode_buffer_size;
+extern PGDLLIMPORT int data_checksums;
 
 extern PGDLLIMPORT int CheckPointSegments;
 
-- 
2.48.1