Re: Hash index: stop registering pin-only bucket pages in WAL (remove REGBUF_NO_CHANGE)

From: Greg Burd <greg(at)burd(dot)me>
To: Hayato Kuroda <kuroda(dot)hayato(at)fujitsu(dot)com>
Cc: vignesh C <vignesh21(at)gmail(dot)com>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, shihao zhong <zhong950419(at)gmail(dot)com>, Michael Paquier <michael(at)paquier(dot)xyz>, Jeff Davis <pgsql(at)j-davis(dot)com>, Alexander Lakhin <exclusion(at)gmail(dot)com>, Joao Rito <joaovictor(dot)rito(at)gmail(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: Hash index: stop registering pin-only bucket pages in WAL (remove REGBUF_NO_CHANGE)
Date: 2026-11-13 19:11:00
Message-ID: avdgGCKX5C-PIqGm@werk
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hayato,

Thanks, these three are good and I have taken all of them verbatim in v10.

01. hashbucketcleanup: reworded to lead with why replay needs the
cleanup lock, then the overflow-page case.

/*
* Replay must take a cleanup lock on the primary bucket page to
* serialize with scans on a hot standby. If buf is an overflow
* page, the unchanged primary page is held only by a pin, so append
* its block number instead of registering it. Otherwise block 0
* identifies the primary page.
*/

02. hash_xlog_lock_primary_bucket doc: shortened to your version. The
detail it dropped (the block number sits right after the fixed-size
struct) is still carried by the Assert and the memcpy in the body,
so nothing is lost for a reader of the function.

/*
* Take a cleanup lock on an unregistered primary bucket page, whose
* block number is stored in the record's main data. This serializes
* replay against scans on a hot standby. block_id must identify a
* registered block of the same index.
*/

03. _hash_freeovflpage: same treatment as 01.

/*
* Replay needs the primary bucket block number to acquire a cleanup
* lock. Register it as block 0 when it is modified; otherwise append
* its block number to the record.
*/

On 0003: it is meant to be pushed. It is the recovery test that pins
down the CRC window 0002 closes, via the injection point in
XLogRecordAssemble that fires after the record CRC is computed and
before the record is copied. Without 0002 the test fails with
"incorrect resource manager data checksum"; with it the same
interleaving replays cleanly. I have kept it after 0002 so each commit
is self-contained, but it is fine to squash it into 0002 on commit if
you would rather the fix and its test land together.

This v10 is the same four-commit master series we converged on with
vignesh, rebased on 77bba5454f9, with the three comment changes above
and nothing else. hash.c, hash_xlog.c, hashovfl.c, the README and the
two headers are identical to vignesh's v8 apart from those comments.

One loose end for the group: vignesh's v8 merged the two torn-page
tests into one 061_hash_torn_primary_page.pl and moved the CRC test to
062. This v10 still carries my earlier split (068a/068b plus the CRC
test), because that is what I had validated. I am happy either way.

0001 (the torn-page redo fix) stands alone and is back-patchable;
0002-0004 are master only.

best.

-greg

Attachment Content-Type Size
v10-0001-Fix-hash-index-redo-reading-a-torn-primary-bucke.patch text/x-patch 19.5 KB
v10-0004-Remove-REGBUF_NO_CHANGE.patch text/x-patch 5.2 KB
v10-0003-Add-a-recovery-test-for-the-hash-WAL-pin-only-bu.patch text/x-patch 12.3 KB
v10-0002-Don-t-register-pin-only-primary-bucket-pages-in-.patch text/x-patch 23.2 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Sami Imseih 2026-11-13 19:22:27 Re: pgstat: Flush some statistics within running transactions, take 2
Previous Message Antonin Houska 2026-11-13 18:35:15 Re: Allow progress tracking of sub-commands