v20260319-0003-A-stab-at-simplified-database-processing-l.patch

application/octet-stream

Filename: v20260319-0003-A-stab-at-simplified-database-processing-l.patch
Type: application/octet-stream
Part: 2
Message: Re: Changing the state of data checksums in a running cluster

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 v20260319-0003
Subject: A stab at simplified database processing logic
File+
src/backend/access/transam/xlog.c 6 0
src/backend/commands/dbcommands.c 6 0
src/backend/postmaster/datachecksum_state.c 59 73
src/include/access/xlog.h 1 0
src/include/postmaster/datachecksum_state.h 1 0
From 952c3de9dd0f15d8d6a4371568015d539521c58a Mon Sep 17 00:00:00 2001
From: Daniel Gustafsson <dgustafsson@postgresql.org>
Date: Fri, 20 Mar 2026 00:08:29 +0100
Subject: [PATCH v20260319 3/3] A stab at simplified database processing logic

The previous coding extracted a list of databases, and proceeded to
process that list.  Once done, it would extract a new list and redo
it all for any new database created concurrently.  This was done to
ensure that a database created from a template which hadnt yet been
processed would be covered.  Since the wal_log strategy for create
database dirties all buffers during thec copy process, this was all
doing double work.  Simplify the logic by only processing a single
list and prohibit the file_copy strategy.
---
 src/backend/access/transam/xlog.c           |   6 +
 src/backend/commands/dbcommands.c           |   6 +
 src/backend/postmaster/datachecksum_state.c | 132 +++++++++-----------
 src/include/access/xlog.h                   |   1 +
 src/include/postmaster/datachecksum_state.h |   1 +
 5 files changed, 73 insertions(+), 73 deletions(-)

diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 883904d390f..5f486200ea3 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -4669,6 +4669,12 @@ DataChecksumsNeedWrite(void)
 			LocalDataChecksumState == PG_DATA_CHECKSUM_INPROGRESS_OFF);
 }
 
+bool
+DataChecksumsInProgress(void)
+{
+	return LocalDataChecksumState == PG_DATA_CHECKSUM_INPROGRESS_ON;
+}
+
 /*
  * DataChecksumsNeedVerify
  *		Returns whether data checksums must be verified or not
diff --git a/src/backend/commands/dbcommands.c b/src/backend/commands/dbcommands.c
index 9b18bb4a17e..c8706e16070 100644
--- a/src/backend/commands/dbcommands.c
+++ b/src/backend/commands/dbcommands.c
@@ -1044,7 +1044,13 @@ createdb(ParseState *pstate, const CreatedbStmt *stmt)
 		if (pg_strcasecmp(strategy, "wal_log") == 0)
 			dbstrategy = CREATEDB_WAL_LOG;
 		else if (pg_strcasecmp(strategy, "file_copy") == 0)
+		{
+			if (DataChecksumsInProgress())
+				ereport(ERROR,
+						errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+						errmsg("create database strategy \"%s\" not allowed when data checksums are bing enabled", strategy));
 			dbstrategy = CREATEDB_FILE_COPY;
+		}
 		else
 			ereport(ERROR,
 					(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
diff --git a/src/backend/postmaster/datachecksum_state.c b/src/backend/postmaster/datachecksum_state.c
index 917cbb7f354..477151091a1 100644
--- a/src/backend/postmaster/datachecksum_state.c
+++ b/src/backend/postmaster/datachecksum_state.c
@@ -396,6 +396,7 @@ static volatile sig_atomic_t launcher_running = false;
 static DataChecksumsWorkerOperation operation;
 
 /* Prototypes */
+static bool DatabaseExists(Oid dboid);
 static List *BuildDatabaseList(void);
 static List *BuildRelationList(bool temp_relations, bool include_shared);
 static void FreeDatabaseList(List *dblist);
@@ -852,7 +853,15 @@ ProcessDatabase(DataChecksumsWorkerDatabase *db)
 				errmsg("could not start background worker for enabling data checksums in database \"%s\"",
 					   db->dbname),
 				errhint("More details on the error might be found in the server log."));
-		return DATACHECKSUMSWORKER_FAILED;
+		/*
+		 * XXX: Heuristic to see if the database was dropped, and if so we can
+		 * ignore the error, else treat as fatal and error out.  This needs a
+		 * tighter check.
+		 */
+		if (DatabaseExists(db->dboid))
+			return DATACHECKSUMSWORKER_FAILED;
+		else
+			return DATACHECKSUMSWORKER_DROPDB;
 	}
 
 	/*
@@ -1170,9 +1179,8 @@ done:
  *		Compute the list of all databases and process checksums in each
  *
  * This will repeatedly generate a list of databases to process for enabling
- * checksums. Until no new databases are found, this will loop around computing
- * a new list and comparing it to the already seen ones.
- *
+ * checksums.  If a database encounters a failure then processing will end
+ * immediately and return an error.
  */
 static bool
 ProcessAllDatabases(void)
@@ -1180,7 +1188,6 @@ ProcessAllDatabases(void)
 	List	   *DatabaseList;
 	HTAB	   *ProcessedDatabases = NULL;
 	HASHCTL		hash_ctl;
-	bool		found_failed = false;
 	int			cumulative_total = 0;
 
 	/* Initialize a hash tracking all processed databases */
@@ -1195,16 +1202,7 @@ ProcessAllDatabases(void)
 	/* Set up so first run processes shared catalogs, not once in every db */
 	DataChecksumsWorkerShmem->process_shared_catalogs = true;
 
-	/*
-	 * Get a list of all databases to process. This may include databases that
-	 * were created during our runtime.  Since a database can be created as a
-	 * copy of any other database (which may not have existed in our last
-	 * run), we have to repeat this loop until no new databases show up in the
-	 * list.  Here the initial list for the loop processing is generated after
-	 * waiting for all existing transactions to finish to ensure that we can
-	 * see any database which was created even if the transaction in which it
-	 * was created started before checksums were being processed.
-	 */
+	/* Get a list of all databases to process */
 	WaitForAllTransactionsToFinish();
 	DatabaseList = BuildDatabaseList();
 
@@ -1303,6 +1301,18 @@ ProcessAllDatabases(void)
 				/* Abort flag set, so exit the whole process */
 				return false;
 			}
+			else if (result == DATACHECKSUMSWORKER_FAILED)
+			{
+				/*
+				 * Disable checksums on cluster, because we failed one of the
+				 * databases and this is an all or nothing process.
+				 */
+				SetDataChecksumsOff();
+				ereport(ERROR,
+						errcode(ERRCODE_INSUFFICIENT_RESOURCES),
+						errmsg("data checksums failed to get enabled in all databases, aborting"),
+						errhint("The server log might have more information on the cause of the error."));
+			}
 
 			entry = hash_search(ProcessedDatabases, &db->dboid, HASH_ENTER, &found);
 			entry->dboid = db->dboid;
@@ -1313,17 +1323,7 @@ ProcessAllDatabases(void)
 				entry->retries++;
 		}
 
-		elog(DEBUG1,
-			 "%i databases processed for data checksum enabling (%i this iteration), %s",
-			 cumulative_total, processed_databases,
-			 (processed_databases ? "process with restart" : "process completed"));
-
-		FreeDatabaseList(DatabaseList);
-
-		/*
-		 * If no databases were processed in this run of the loop, we have now
-		 * finished all databases and no concurrently created ones can exist.
-		 */
+		/* If no databases were processed in this iteration, exit the loop */
 		if (processed_databases == 0)
 			break;
 
@@ -1338,56 +1338,10 @@ ProcessAllDatabases(void)
 		 * enabled) will be committed.
 		 */
 		WaitForAllTransactionsToFinish();
-		DatabaseList = BuildDatabaseList();
-	}
-
-	/*
-	 * ProcessedDatabases now has all databases and the results of their
-	 * processing. Failure to enable checksums for a database can be because
-	 * they actually failed for some reason, or because the database was
-	 * dropped between us getting the database list and trying to process it.
-	 * Get a fresh list of databases to detect the second case where the
-	 * database was dropped before we had started processing it. If a database
-	 * still exists, but enabling checksums failed then we fail the entire
-	 * checksum enablement process and exit with an error.
-	 */
-	DatabaseList = BuildDatabaseList();
-
-	foreach_ptr(DataChecksumsWorkerDatabase, db, DatabaseList)
-	{
-		DataChecksumsWorkerResultEntry *entry;
-		bool		found;
-
-		entry = hash_search(ProcessedDatabases, (void *) &db->dboid,
-							HASH_FIND, &found);
-
-		/*
-		 * We are only interested in the processed databases which failed, and
-		 * where the failed database still exists.  This indicates that
-		 * enabling checksums actually failed, and not that the failure was
-		 * due to the db being concurrently dropped.
-		 */
-		if (found && entry->result == DATACHECKSUMSWORKER_FAILED)
-		{
-			ereport(WARNING,
-					errmsg("failed to enable data checksums in \"%s\"", db->dbname));
-			found_failed = found;
-			continue;
-		}
 	}
 
 	FreeDatabaseList(DatabaseList);
 
-	if (found_failed)
-	{
-		/* Disable checksums on cluster, because we failed */
-		SetDataChecksumsOff();
-		ereport(ERROR,
-				errcode(ERRCODE_INSUFFICIENT_RESOURCES),
-				errmsg("data checksums failed to get enabled in all databases, aborting"),
-				errhint("The server log might have more information on the cause of the error."));
-	}
-
 	pgstat_progress_update_param(PROGRESS_DATACHECKSUMS_PHASE,
 								 PROGRESS_DATACHECKSUMS_PHASE_WAITING_BARRIER);
 	return true;
@@ -1421,11 +1375,43 @@ DataChecksumsWorkerShmemInit(void)
 		ShmemInitStruct("DataChecksumsWorker Data",
 						DataChecksumsWorkerShmemSize(),
 						&found);
-
 	if (!found)
 		MemSet(DataChecksumsWorkerShmem, 0, DataChecksumsWorkerShmemSize());
 }
 
+/*
+ * DatabaseExists
+ *
+ * Scans the system catalog to check if a database with the given Oid exist
+ * and returns true if it is found, else false.
+ */
+static bool
+DatabaseExists(Oid dboid)
+{
+	Relation	rel;
+	ScanKeyData	skey;
+	SysScanDesc scan;
+	bool	found;
+	HeapTuple tuple;
+
+	StartTransactionCommand();
+
+	rel = table_open(DatabaseRelationId, AccessShareLock);
+	ScanKeyInit(&skey,
+				Anum_pg_database_oid,
+				BTEqualStrategyNumber, F_OIDEQ,
+				dboid);
+	scan = systable_beginscan(rel, DatabaseOidIndexId, true, SnapshotSelf,
+							  1, &skey);
+	tuple = systable_getnext(scan);
+	found = HeapTupleIsValid(tuple);
+
+	systable_endscan(scan);
+	table_close(rel, AccessShareLock);
+
+	return found;
+}
+
 /*
  * BuildDatabaseList
  *		Compile a list of all currently available databases in the cluster
diff --git a/src/include/access/xlog.h b/src/include/access/xlog.h
index 684c1a9b20a..6e917a82072 100644
--- a/src/include/access/xlog.h
+++ b/src/include/access/xlog.h
@@ -249,6 +249,7 @@ extern uint64 GetSystemIdentifier(void);
 extern char *GetMockAuthenticationNonce(void);
 extern bool DataChecksumsNeedWrite(void);
 extern bool DataChecksumsNeedVerify(void);
+extern bool DataChecksumsInProgress(void);
 extern void SetDataChecksumsOnInProgress(void);
 extern void SetDataChecksumsOn(void);
 extern void SetDataChecksumsOff(void);
diff --git a/src/include/postmaster/datachecksum_state.h b/src/include/postmaster/datachecksum_state.h
index 5601241a803..2428ca6581c 100644
--- a/src/include/postmaster/datachecksum_state.h
+++ b/src/include/postmaster/datachecksum_state.h
@@ -39,6 +39,7 @@ typedef enum
 	DATACHECKSUMSWORKER_ABORTED,
 	DATACHECKSUMSWORKER_FAILED,
 	DATACHECKSUMSWORKER_RETRYDB,
+	DATACHECKSUMSWORKER_DROPDB,
 } DataChecksumsWorkerResult;
 
 /* Prototypes for data checksum state manipulation */
-- 
2.39.3 (Apple Git-146)