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

> On 25 Apr 2026, at 13:59, Ayush Tiwari <ayushtiwari(dot)slg01(at)gmail(dot)com> wrote:

> I don't think 0006 fully addresses the duplicate-launcher exit race [1]. I was
> able to reproduce the issue with a deterministic test using the existing
> test_checksums delay hooks: delay
> the initial StartDataChecksumsWorkerLauncher()
> path so two quick pg_enable_data_checksums() calls can queue two launchers,
> delay the final transition to on, then fire follow-up pg_enable_data_checksums()
> calls while the real launcher is still active.

Thanks to the really great reproducer script I got offlist (which is attached
here) I was able to reproduce and figure out what was wrong. This sequence
exposed a thinko in the cleanup handler where a process exiting due to an
already running launcher would enter cleanup, and thus wipe state from the
running launcher. It needed this quite narrow window to hit, but it was good
to catch that. Fixed in the attached by giving processes a way to register
themselves as needing no cleanup when exiting.

> A few other smaller comments/nits:
>
> 1. In src/backend/access/transam/xlog.c, the comment in SetDataChecksumsOff()
> says the branch implies the state is inprogress-on or inprogress-off, but
> after the patch inprogress-on is handled by the preceding if condition. It
> looks like this should probably only mention inprogress-off.

Fixed.

> 2. There is a repeated typo in comments:
>
> src/backend/utils/init/postinit.c: "interrups" -> "interrupts"
> src/backend/postmaster/auxprocess.c: "interrups" -> "interrupts"

Fixed.

> 3. A few commit messages could use a quick polish pass:
>
> 0002: "onine" -> "online"
> 0004: "Additionelly" -> "Additionally"
> 0006: "being enabled of disabled" -> "being enabled or disabled"
> 0006: "change it's operation" -> "change its operation"
> 0006: "ss now avoided" -> "is now avoided"

Fixed.

> 4. One minor question on 0006: the runtime cost-parameter update path is only
> meaningful while checksums are being enabled. That looks fine from the current
> control flow, but would it be worth adding a short comment or assertion near
> that update path to make the assumption explicit?

We have this assertion which makes sure that there are no costs passed for
disabling, not sure if we need much more since there is now way for the user to
inject any cost parameters.

#ifdef USE_ASSERT_CHECKING
/* The cost delay settings have no effect when disabling */
if (op == DISABLE_DATACHECKSUMS)
Assert(cost_delay == 0 && cost_limit == 0);
#endif

--
Daniel Gustafsson