From 0e2fd19c6122258a054bce13ffd4c641e210395a Mon Sep 17 00:00:00 2001 From: Daniel Gustafsson Date: Wed, 17 Jun 2026 14:55:57 +0200 Subject: [PATCH] Fix comments on data checksum cost settings The cost parameters for the data checksums worker can be updated by the user issuing a repeated enable checksum command, but the comments on the struct members hadn't been updated to reflect this and were out of date. Another part of the same comment needed better wording to be readable. Also wrap the reading of the parameters in a lock, there is no live bug due to not using a lock but it's still the right thing to do. Reported-by: Heikki Linnakangas Discussion: https://postgr.es/m/2176020b-ecbc-438b-9fc3-9c3593d9e6fc@iki.fi --- src/backend/postmaster/datachecksum_state.c | 25 ++++++++++++--------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/src/backend/postmaster/datachecksum_state.c b/src/backend/postmaster/datachecksum_state.c index a6fdcf114ec..dabfa55c6a6 100644 --- a/src/backend/postmaster/datachecksum_state.c +++ b/src/backend/postmaster/datachecksum_state.c @@ -306,15 +306,13 @@ typedef struct DataChecksumsStateStruct pid_t worker_pid; /* - * These fields indicate the target state that the launcher is currently - * working towards. They can be different from the corresponding launch_* + * These fields indicate the target state that the worker is currently + * running with. They can be different from the corresponding launch_* * fields, if a new pg_enable/disable_data_checksums() call was made while - * the launcher/worker was already running. - * - * The below members are set when the launcher starts, and are only - * accessed read-only by the single worker. Thus, we can access these - * without a lock. If multiple workers, or dynamic cost parameters, are - * supported at some point then this would need to be revisited. + * the launcher/worker was already running. The worker will periodically + * check if new cost settings have been requested, and if so will copy + * them from the launch_* fields and reset cost throttling to match the + * new values. */ DataChecksumsWorkerOperation operation; int cost_delay; @@ -1509,6 +1507,7 @@ DataChecksumsWorkerMain(Datum arg) BufferAccessStrategy strategy; bool aborted = false; int64 rels_done; + bool process_shared; #ifdef USE_INJECTION_POINTS bool retried = false; #endif @@ -1533,9 +1532,13 @@ DataChecksumsWorkerMain(Datum arg) /* * Get a list of all temp tables present as we start in this database. We * need to wait until they are all gone until we are done, since we cannot - * access these relations and modify them. + * access these relations and modify them. For the list of relations to + * process once the temp relations are gone, check if shared catalogs have + * been processed already. */ InitialTempTableList = BuildRelationList(true, false); + LWLockAcquire(DataChecksumsWorkerLock, LW_EXCLUSIVE); + process_shared = DataChecksumState->process_shared_catalogs; /* * Enable vacuum cost delay, if any. While this process isn't doing any @@ -1549,6 +1552,7 @@ DataChecksumsWorkerMain(Datum arg) */ VacuumCostDelay = DataChecksumState->cost_delay; VacuumCostLimit = DataChecksumState->cost_limit; + LWLockRelease(DataChecksumsWorkerLock); VacuumUpdateCosts(); VacuumCostBalance = 0; @@ -1557,8 +1561,7 @@ DataChecksumsWorkerMain(Datum arg) */ strategy = GetAccessStrategy(BAS_VACUUM); - RelationList = BuildRelationList(false, - DataChecksumState->process_shared_catalogs); + RelationList = BuildRelationList(false, process_shared); /* Update the total number of relations to be processed in this DB. */ { -- 2.39.3 (Apple Git-146)