Re: Changing the state of data checksums in a running cluster

It'd be good to print a LOG message when the checksums state changes, to
have a trail in the log of when checksums were enabled/disabled.
Something like:

LOG: enabling checksums was requested, starting checksum calculation
...
LOG: checksums calculation finished, checksums are now enabled

On 02/04/2026 02:01, Daniel Gustafsson wrote:
> + 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."));
> + }

This got me thinking, what happens if the the data checksums launcher
encounters some other error, for example if you SIGTERM it? The system
is left in 'inprogress-on' state, but because the launcher is gone it
will never finish and 'pg_stat_progress_data_checksums' will be empty.

Perhaps launcher_exit() should call SetDataChecksumsOff()?

> /*
> * TODO: how to really handle the worker still running when the
> * launcher exits?
> */
> if (DataChecksumsWorkerShmem->worker_running)
> ereport(LOG,
> errmsg("data checksums launcher exiting while worker is still running"));

That TODO should be addressed somehow.

> + /*
> + * As of now we only update the block counter for main forks in order
> + * to not cause too frequent calls. TODO: investigate whether we
> + * should do it more frequent?
> + */
> + if (forkNum == MAIN_FORKNUM)
> + pgstat_progress_update_param(PROGRESS_DATACHECKSUMS_BLOCKS_DONE,
> + (blknum + 1));

We're updating it for every block in the main fork, but not at all for
other forks? What a bizarre way of avoiding too frequent calls :-). I
think you could just call this on every page,
pgstat_progress_update_param() is supposed to be very fast. For
comparison, e.g. index build calls it for every tuple.

> +/*
> + * Configuration of conditions which must match when absorbing a procsignal
> + * barrier during data checksum enable/disable operations. A single function
> + * is used for absorbing all barriers, and the set of conditions to use is
> + * looked up in the checksum_barriers struct. The struct member for the target
> + * state defines which state the backend must currently be in, and which it
> + * must not be in.
> + *
> + * The reason for this explicit checking is to ensure that processing cannot
> + * be started such that it breaks the assumptions of the state machine.
> + *
> + * MAX_BARRIER_CONDITIONS must match largest number of sets in barrier_eq and
> + * barrier_ne in the below checksum_barriers definition.
> + */
> +#define MAX_BARRIER_CONDITIONS 2
> +typedef struct ChecksumBarrierCondition
> +{
> + /* The target state of the barrier */
> + int target;
> + /* A set of states in which at least one MUST match the current state */
> + int barrier_eq[MAX_BARRIER_CONDITIONS];
> + /* The number of elements in the barrier_eq set */
> + int barrier_eq_sz;
> + /* A set of states which all MUST NOT match the current state */
> + int barrier_ne[MAX_BARRIER_CONDITIONS];
> + /* The number of elements in the barrier_ne set */
> + int barrier_ne_sz;
> +} ChecksumBarrierCondition;
> +
> +static const ChecksumBarrierCondition checksum_barriers[4] =
> +{
> + /*
> + * When disabling checksums, either inprogress state is Ok but checksums
> + * must not be in the enabled state.
> + */
> + {
> + .target = PG_DATA_CHECKSUM_OFF,
> + .barrier_eq = {PG_DATA_CHECKSUM_INPROGRESS_ON, PG_DATA_CHECKSUM_INPROGRESS_OFF},
> + .barrier_eq_sz = 2,
> + .barrier_ne = {PG_DATA_CHECKSUM_VERSION},
> + .barrier_ne_sz = 1
> + },
> + /* When enabling the current state must be inprogress-on */
> + {
> + .target = PG_DATA_CHECKSUM_VERSION,
> + .barrier_eq = {PG_DATA_CHECKSUM_INPROGRESS_ON},
> + .barrier_eq_sz = 1,
> + {0}, 0
> + },
> +
> + /*
> + * When moving to inprogress-on the current state cannot enabled, but when
> + * moving to inprogress-off the current state must be enabled.
> + */
> + {
> + .target = PG_DATA_CHECKSUM_INPROGRESS_ON,
> + {0}, 0,
> + .barrier_ne = {PG_DATA_CHECKSUM_VERSION},
> + .barrier_ne_sz = 1
> + },
> + {
> + .target = PG_DATA_CHECKSUM_INPROGRESS_OFF,
> + .barrier_eq = {PG_DATA_CHECKSUM_VERSION},
> + .barrier_eq_sz = 1,
> + {0}, 0
> + },
> +};

I find this to still be a pretty complicated and unclear way of
representing the allowed transitions. There are only 16 possible
transitions, and only 6 of them are allowed. How about listing the
allowed ones directly:

/* Allowed transitions: from, to */
{
/*
* Disabling checksums: If checksums are currently enabled,
* disabling must go through the 'inprogress-off' state.
*/
{PG_DATA_CHECKSUM_VERSION, PG_DATA_CHECKSUM_INPROGRESS_OFF},
{PG_DATA_CHECKSUM_INPROGRESS_OFF, PG_DATA_CHECKSUM_OFF},

/*
* If checksums are in the process of being enabled, but are
* not yet being verified, we can abort by going back to 'off'
* state.
*/
{PG_DATA_CHECKSUM_INPROGRESS_ON, PG_DATA_CHECKSUM_OFF},

/*
* Enabling checksums must normally go through the 'inprogress-on'
* state.
*/
{PG_DATA_CHECKSUM_OFF, PG_DATA_CHECKSUM_INPROGRESS_ON},
{PG_DATA_CHECKSUM_INPROGRESS_ON, PG_DATA_CHECKSUM_VERSION},

/*
* If checksums are being disabled but all backends are still
* computing checksums, we can go straight back to 'on'
*/
{PG_DATA_CHECKSUM_INPROGRESS_OFF, PG_DATA_CHECKSUM_VERSION},
}

> +/*
> + * Signaling between backends calling pg_enable/disable_data_checksums, the
> + * checksums launcher process, and the checksums worker process.
> + *
> + * This struct is protected by DataChecksumsWorkerLock
> + */
> +typedef struct DataChecksumsWorkerShmemStruct
> +{
> + /*
> + * These are set by pg_{enable|disable|verify}_data_checksums, to tell the
> + * launcher what the target state is.
> + */
> + DataChecksumsWorkerOperation launch_operation;
> + int launch_cost_delay;
> + int launch_cost_limit;

The naming feels a little weird with this struct. It's called
"DataChecksumsWorkerShmemStruct", but it's also accessed by the backends
calling pg_enable/disable_data_checksums(). And
"DataChecksumsWorkerOperation" is not accessed by workers at all. Or I
guess the "operation" global variable is used in
DataChecksumsWorkerMain(), but it's always set to ENABLE_DATACHECKSUMS
in the worker. Do you need the "operation" global variable at all?

> +void
> +SetDataChecksumsOn(void)
> +{
> + uint64 barrier;
> +
> + Assert(ControlFile != NULL);
> +
> + SpinLockAcquire(&XLogCtl->info_lck);
> +
> + /*
> + * The only allowed state transition to "on" is from "inprogress-on" since
> + * that state ensures that all pages will have data checksums written.
> + */
> + if (XLogCtl->data_checksum_version != PG_DATA_CHECKSUM_INPROGRESS_ON)
> + {
> + SpinLockRelease(&XLogCtl->info_lck);
> + elog(PANIC, "checksums not in \"inprogress-on\" mode");
> + }
> +
> + SpinLockRelease(&XLogCtl->info_lck);

The PANIC seems a little harsh, you haven't done anything destructive
here. It's unexpected for this to be called in any other state, so this
is a "can't happen" scenario, but I don't think we usually PANIC on those.

> + <para>
> + If <parameter>cost_delay</parameter> and <parameter>cost_limit</parameter> are
> + specified, the process is throttled using the same principles as
> + <link linkend="runtime-config-resource-vacuum-cost">Cost-based Vacuum Delay</link>.
> + </para>

Ugh, yet another place where we expose the "cost delay/limit" throttling
mechanism. I agree it's good to be consistent here and use the same
method we use for vacuum, I just wish we had something more user-friendly..

Grammar / spelling:

> + * state will also be set of "off".

> + * When moving to inprogress-on the current state cannot enabled, but when

> + * If a worker process currently running? This is set by the worker

> + * These are set by pg_{enable|disable|verify}_data_checksums, to tell the

there is no "pg_verify_data_checksums" function.

"calcuated" in commit message

- Heikki