DataChecksumsStateStruct cost_delay fields and locking

Hi,

The comment in the DataChecksumsStateStruct says:

> 
> 	/*
> 	 * These fields indicate the target state that the launcher is currently
> 	 * working towards. 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.
> 	 */
> 	DataChecksumsWorkerOperation operation;
> 	int			cost_delay;
> 	int			cost_limit;

The "only accessed read-only by the single worker" part isn't true, 
because DataChecksumsWorkerMain() does this:

> 		/*
> 		 * Check if the cost settings changed during runtime and if so, update
> 		 * to reflect the new values and signal that the access strategy needs
> 		 * to be refreshed.
> 		 */
> 		LWLockAcquire(DataChecksumsWorkerLock, LW_EXCLUSIVE);
> 		if ((DataChecksumState->launch_cost_delay != DataChecksumState->cost_delay)
> 			|| (DataChecksumState->launch_cost_limit != DataChecksumState->cost_limit))
> 		{
> 			costs_updated = true;
> 			VacuumCostDelay = DataChecksumState->launch_cost_delay;
> 			VacuumCostLimit = DataChecksumState->launch_cost_limit;
> 			VacuumUpdateCosts();
> 
> 			DataChecksumState->cost_delay = DataChecksumState->launch_cost_delay;
> 			DataChecksumState->cost_limit = DataChecksumState->launch_cost_limit;
> 		}
> 		else
> 			costs_updated = false;
> 		LWLockRelease(DataChecksumsWorkerLock);

So contrary to the comment, the cost_delay fields are updated by the worker.

Some other observations:

- at the beginning of DataChecksumsWorkerMain(), it reads 
DataChecksumState->cost_delay and cost_limit without holding the lock, 
and also DataChecksumState->process_shared_catalogs. I think that's OK, 
but I would just add locking to it, to remove all doubt.

- the comment says "These fields indicate the target state that the 
launcher is currently working towards". It took me a moment to 
understand what the means. At first, I thought that it means that it's 
the target state of the launcher, but not necessarily what it's 
currently using. That's wrong. Those are in fact the values that the 
*worker* is *currently* using. launch_cost_delay/limit are the "target" 
values, which will eventually be copied into the current 
cost_delay/limit values. Perhaps change "... launcher is currently 
working towards" into "... worker is currently running with" or 
something like that.

- Heikki