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