> On 28 Aug 2026, at 07:33, Bertrand Drouvot <[email protected]> 
> wrote:

> Only looking at 0001 here, I've a few comments:

Thanks, I've yet to dig into it completely but below are a few quick questions
to help me along the way.

> === 1
> 
> @@ -9288,17 +9572,33 @@ xlog2_redo(XLogReaderState *record)
> 
>        SpinLockAcquire(&XLogCtl->info_lck);
>        XLogCtl->data_checksum_version = state.new_checksum_state;
> +       SetLocalDataChecksumState(state.new_checksum_state);
>        SpinLockRelease(&XLogCtl->info_lck);
> 
> This applies every XLOG2_CHECKSUMS record encountered during recovery, even 
> when
> the same record was applied before.
> 
> For example, a standby can replay the final "on" record and then stop cleanly
> without advancing its restartpoint beyond that record. If checksums are 
> subsequently
> disabled offline, the next startup begins from the older restartpoint and 
> replays
> the same on record again, overriding the offline disable.

Do you mean that checksums are disabled offline across the cluster on all
nodes, or just on the standby?

> === 2
> 
> +    * checkPoint.dataChecksumState was sampled while holding the WAL insert
> +    * locks, so it is the state in effect at the redo point. 
> .
> .
> .
> +   SpinLockAcquire(&XLogCtl->info_lck);
> +   if (checkPoint.dataChecksumState == XLogCtl->data_checksum_version)
> +       ControlFile->data_checksum_version = checkPoint.dataChecksumState;
> +   SpinLockRelease(&XLogCtl->info_lck);
> 
> I’m not sure the "state in effect at the redo point" is always correct. There 
> is
> a window between inserting the checksum transition record and updating
> XLogCtl->data_checksum_version.
> 
> XLOG2_CHECKSUMS(on) records the target state. The transition can therefore
> proceed as follows:
> 
> 1. The current shared state is inprogress-on.
> 2. XLogChecksums() inserts XLOG2_CHECKSUMS(on) and releases its WAL insertion
>   lock.
> 3. Before the shared state is updated to on, the checkpoint still reads
>   inprogress-on and inserts XLOG_CHECKPOINT_REDO.
> 4. The transition then updates the shared state to on.
> 
> The WAL order is then:
> 
> XLOG2_CHECKSUMS(on)
> XLOG_CHECKPOINT_REDO(inprogress-on)
> 
> If the server crashes after the concurrent checkpoint from step 3 completes,
> but before the enabling operation’s later checkpoint completes, recovery 
> starts
> from that redo point and does not replay the preceding on record. It can
> therefore resolve inprogress-on back to off. The equality check above does
> not repair this ordering.

If this can happen then online checksums wouldn't work at all right?  This
window is happening inside a critical section while DELAY_CHKPT_START is set to
prevent a checkpoint from storing the state and completing to protect against
this.  Have you been able to construct a repro (with injection points) where a
REDO record after a CHECKSUM record carries the wrong state?

--
Daniel Gustafsson



Reply via email to