Re: WAL record CRC calculated incorrectly because of underlying buffer modification

On Tue, 27 Oct 2026 at 19:25, Greg Burd <greg(at)burd(dot)me> wrote:
>
> On Tue, 27 Oct 2026 08:06:48 +0900, Michael Paquier <michael(at)paquier(dot)xyz> wrote:
> > it still looks like a problem that needs to be tackled in three steps:
> > - Avoid the CRC problem down to v17, including HEAD.
> > - Address hash to avoid the rule breakage.
> > - Lift the check in replay for wal_consistency_checking (re-add the
> > assert) and push REGBUF_NO_CHANGE into orbit.
> > [...] if somebody is willing to work that, be it with an LLM, I would
> > read the code.
>
> That plan works for me, and I'll take the hash rework (step 2).
>
> Step 1 (down to v17, including HEAD): the flag-level fix we discussed,
> with your comment wording. I'll post it as the next version shortly.
> The one line I'd keep is the pointer from verifyBackupPageConsistency()
> back to XLogRecordAssemble(), since the two have to stay in agreement
> and they're far apart in the file.

Thanks for working on this. These comments are on the step 1 patch
(v1-0001, "Don't take a full-page image for REGBUF_NO_CHANGE
buffers").
I could reproduce the problem consistently with a VACUUM on a table
with a hash index and a concurrent INSERT, with
wal_consistency_checking = 'hash'. The test is attached; it needs a
test-only injection point after the CRC loop in XLogRecordAssemble().
Your step 1 patch fixes it.

Few comments:
1. The README, the macro comment, the code comment and the commit
message say such a buffer "never has a full-page image taken". The
code is include_image = needs_backup, so an image is still taken when
needs_backup is true. For example, _hash_freeovflpage() registers the
write buffer with REGBUF_STANDARD | REGBUF_NO_CHANGE, without
REGBUF_NO_IMAGE, when the primary bucket page is the write page and no
tuples are moved onto it. After a checkpoint, that record carries an
ordinary FPI of the page.
+ The REGBUF_NO_CHANGE flag indicates that the buffer is registered without
+ being modified by this record, and possibly without an exclusive content
+ lock (for example, to force a cleanup lock on the block during replay).
+ Such a buffer never has a full-page image taken, not even when WAL
+ consistency checking is enabled: its contents may change concurrently

Is this ok, or should we mention like "no consistency-check image"

2. The patch does not apply to REL_17 (the README hunk fails), so a
separate patch is needed for that branch.

Regards,
Vignesh