From: Daniel Gustafsson Date: Thu, 18 Jun 2026 21:16:35 +0000 (+0200) Subject: Fix comments on data checksum cost settings X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=8d22f5232458278aa62a1769758a8aa6b1dfe290;p=thirdparty%2Fpostgresql.git 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. Author: Daniel Gustafsson Reviewed-by: Heikki Linnakangas Reported-by: Heikki Linnakangas Discussion: https://postgr.es/m/2176020b-ecbc-438b-9fc3-9c3593d9e6fc@iki.fi --- diff --git a/src/backend/postmaster/datachecksum_state.c b/src/backend/postmaster/datachecksum_state.c index a6fdcf114ec..04f1a268845 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 @@ -1532,10 +1531,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. + * need to wait until they are all gone before we exit. For the list of + * relations to enable checksums in, 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 +1551,7 @@ DataChecksumsWorkerMain(Datum arg) */ VacuumCostDelay = DataChecksumState->cost_delay; VacuumCostLimit = DataChecksumState->cost_limit; + LWLockRelease(DataChecksumsWorkerLock); VacuumUpdateCosts(); VacuumCostBalance = 0; @@ -1557,8 +1560,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. */ {