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

From: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
To: Greg Burd <greg(at)burd(dot)me>
Cc: pgsql-hackers(at)lists(dot)postgresql(dot)org, michael(at)paquier(dot)xyz, andres(at)anarazel(dot)de, pgsql(at)j-davis(dot)com, thomas(dot)munro(at)gmail(dot)com, exclusion(at)gmail(dot)com, hlinnaka(at)iki(dot)fi
Subject: Re: WAL record CRC calculated incorrectly because of underlying buffer modification
Date: 2026-10-29 09:31:07
Message-ID: CAA4eK1+JpYJnUWwrSO7=gs=P3qHkYTrV+gMMPKY-ERv3pHdt-Q@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, Oct 28, 2026 at 6:59 PM Greg Burd <greg(at)burd(dot)me> wrote:
>
> Here is step 2, the hash rework. 0002 is the step 3 end state, marked
> NOT FOR MERGE, so you can see where this lands.
>
> 0001: Don't register pin-only primary bucket pages in hash WAL records
>
> XLOG_HASH_DELETE, XLOG_HASH_MOVE_PAGE_CONTENTS and
> XLOG_HASH_SQUEEZE_PAGE register the primary bucket page only so that
> redo takes a cleanup lock on it. Redo never reads data for that block.
>
> 0001 puts the block number in the record's main data instead (a new
> bucket_blkno field), and redo reads the block and calls
> LockBufferForCleanup() on it directly, at the same point and in the same
> order as today. The page is registered only when the record changes it.
>
> For SQUEEZE_PAGE that also covers the REGBUF_NO_CHANGE registration of
> the write page (the case where the primary page is the write page but
> nothing moved onto it): block 1 is now registered only when it is
> modified. After this, hash registers only buffers it holds exclusively
> locked and dirty.
>
> Reading an unregistered block in redo has precedent: nbtree's "pin scan"
> did exactly that (XLogReadBufferExtended + LockBufferForCleanup) until
> 9f83468b353, which removed it because it was no longer needed, not
> because it was unsafe ACAICT. The cost is that recovery prefetch no
> longer sees the bucket page for these records. The record format
> changes, so XLOG_PAGE_MAGIC is bumped; master only.
>

This approach looks reasonable to me.
*
@@ -902,6 +902,7 @@ hashbucketcleanup(Relation rel, Bucket cur_bucket,
Buffer bucket_buf,
{
xl_hash_delete xlrec;

+ xlrec.bucket_blkno = bucket_blkno;
xlrec.clear_dead_marking = clear_dead_marking;

Though harmless, we don't need this for primary bucket pages, so one
idea could be to use XLogRegisterData to register this block number
for overflow pages only. This would save four bytes for the cases
where we don't perform the cleanup operations on overflow pages, the
same would be true for other cases.

--
With Regards,
Amit Kapila.

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message David Geier 2026-10-29 09:46:09 Re: Hash a ScalarArrayOpExpr whose array is fixed for one execution
Previous Message Bertrand Drouvot 2026-10-29 09:11:27 Re: Avoid resetting subscription two_phase state to pending when already enabled