On Sat, 31 Oct 2026 17:02:34 +0530, Amit Kapila <[email protected]> wrote:
> +1. I also think this should be a master-only change helping us to
> remove the use of REGBUF_NO_CHANGE from hash indexes. Also, the below
> change to make it work on back-branches appears too hacky to me.

Thanks.  Then the back branches stay on 27caf4ca5ae, and this series is
master only.  The #if HASH_WAL_REGISTER_BUCKET_PAGE shim you quoted was
in Vignesh's REL_18 compatibility patch, and it is not part of this
series.

v4 is attached, rebased on current master (5681f0cfa5a).  The only
change from v3 is formatting: pgperltidy rewraps a few lines in the new
TAP test.  The C code is the same as in v3.

   0001  Don't register pin-only primary bucket pages in hash WAL records
   0002  Append the hash primary bucket block number to WAL records only
         when needed (Vignesh; Amit's suggestion)
   0003  Add a recovery test for the hash WAL pin-only bucket page fix
         (co-authored with Vignesh)
   0004  Remove REGBUF_NO_CHANGE

One point I'd like a reviewer to look at in 0001:

redo now reads a block the record does not register.  The block number
is appended to the record's main data only when the primary bucket page
is not one of the pages the record changes.  Redo takes the relation
from a block that is always registered (block 1 for DELETE and
MOVE_PAGE_CONTENTS, block 2 for SQUEEZE_PAGE), and reads the page with
RBM_NORMAL, so a missing page goes through the same invalid-page
tracking as when the page was registered.

The page is not modified, so it needs no full-page image and no LSN
update; the old code registered it with REGBUF_NO_IMAGE, so redo already
worked from the on-disk copy. Two consumers of block references stop
seeing it.  Recovery prefetch is one, so redo may do one synchronous
read of the bucket page per record when the page isn't cached.  The
other is tools that collect changed blocks from WAL, such as pg_rewind
and the WAL summarizer, and losing it there is harmless because the page
is not changed.  FSM redo reads unregistered blocks the same way, and
nbtree's pin scan did until 9f83468b353.

Testing:

- meson test setup, regress, isolation, recovery, pg_waldump and the
   hash module, with PG_TEST_EXTRA=wal_consistency_checking, all four
   patches applied: 68 of 68 suites pass on both builds, including
   061_hash_vacuum_insert_crc and 027_stream_regress.  With 0001 applied
   alone, the same suites minus isolation also pass (64 of 64), so 0001
   can be committed on its own.

- Streaming-standby CRC reproducer: 8 sessions inserting and deleting on
   a single-bucket index under aggressive autovacuum,
   wal_consistency_checking = 'hash'.  As a control I ran master with
   27caf4ca5ae reverted: the standby stops on "incorrect resource manager
   data checksum".  With v4, which also has 0004's REGBUF_NO_CHANGE
   removal and restored asserts, the standby caught up with no errors.

- Cleanup lock on the standby: a cursor is parked in the bucket's
   overflow chain, then the primary vacuums so that every DELETE,
   MOVE_PAGE_CONTENTS and SQUEEZE_PAGE record has the primary bucket page
   unmodified (is_primary F).  On both builds replay waits on
   BufferCleanup until the cursor finishes, and the cursor sees all 2250
   rows, the same as unpatched master.

- I also reran the deadlock check from my first message on this thread
   against current master.  Taking the primary bucket page lock in
   _hash_freeovflpage() after the metapage is locked exclusive still
   hangs VACUUM and a scan of a bucket being populated by an incomplete
   split.  Taking it before the metapage read does not. 0001 takes
   neither lock; this is only why the series doesn't lock the page.

I think 0001-0003 are ready for a committer.  0004 can go in with them
or right after.

best.

-greg

From 735170984e0d69ab879b4042e695d1eb3d1ebd6e Mon Sep 17 00:00:00 2001
From: Greg Burd <[email protected]>
Date: Thu, 29 Oct 2026 10:30:14 -0400
Subject: [PATCH v4 1/4] Don't register pin-only primary bucket pages in hash
 WAL records

Replay of XLOG_HASH_DELETE, XLOG_HASH_MOVE_PAGE_CONTENTS and
XLOG_HASH_SQUEEZE_PAGE must take a cleanup lock on the bucket's primary
page, so that a hot standby scan cannot be in progress in the bucket, or
start in it, while tuples are removed or moved.  When the primary bucket
page is not itself changed by the record, hashbucketcleanup(),
_hash_squeezebucket() and _hash_freeovflpage() got that lock by
registering the page with REGBUF_NO_CHANGE while holding only a pin on
it.  That breaks the rule that a registered buffer is content-locked and
dirtied, and it is why wal_consistency_checking could compute the record
CRC over a page that a concurrent inserter was still changing (fixed on
its own in 27caf4ca5ae5 by not forcing an image for such a buffer).

Instead, store the primary bucket page's block number in the main data
of these three records, and have replay read that block and take the
cleanup lock on it directly, in the same order as before.  The page is
no longer registered unless the record changes it.  For the squeeze
record this also stops registering the write page when it is the
primary bucket page and is not modified.

With this, hash no longer uses REGBUF_NO_CHANGE.

The three record formats change, so bump XLOG_PAGE_MAGIC.

Discussion: https://postgr.es/m/[email protected]
---
 src/backend/access/hash/README         |  9 ++++
 src/backend/access/hash/hash.c         | 15 +++----
 src/backend/access/hash/hash_xlog.c    | 56 ++++++++++++++++-------
 src/backend/access/hash/hashovfl.c     | 61 ++++++++------------------
 src/backend/access/rmgrdesc/hashdesc.c |  9 ++--
 src/include/access/hash_xlog.h         | 18 ++++++--
 src/include/access/xlog_internal.h     |  2 +-
 7 files changed, 93 insertions(+), 77 deletions(-)

diff --git a/src/backend/access/hash/README b/src/backend/access/hash/README
index fc9031117c9..6d9b14e2d21 100644
--- a/src/backend/access/hash/README
+++ b/src/backend/access/hash/README
@@ -641,6 +641,15 @@ entire bucket.  After recovery, the operations will work 
correctly, but
 the index will remain bloated and this can impact performance of read and
 insert operations until the next vacuum squeeze the bucket completely.
 
+Replay of the delete, move-page-contents and squeeze-page records takes a
+cleanup lock on the primary bucket page before touching any other page, so
+that no scan on a hot standby is in progress in the bucket or can start in
+it while tuples are removed or moved; such a scan could otherwise miss
+tuples or return them twice.  When the primary bucket page is not one of
+the pages the record changes, the operation holds only a pin on it, so it
+is not registered in the record.  The record stores its block number
+instead, and replay locks that block directly.
+
 
 Other Notes
 -----------
diff --git a/src/backend/access/hash/hash.c b/src/backend/access/hash/hash.c
index 63846c10840..fc93d78f6bb 100644
--- a/src/backend/access/hash/hash.c
+++ b/src/backend/access/hash/hash.c
@@ -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;
                                xlrec.is_primary_bucket_page = (buf == 
bucket_buf);
 
@@ -909,17 +910,11 @@ hashbucketcleanup(Relation rel, Bucket cur_bucket, Buffer 
bucket_buf,
                                XLogRegisterData(&xlrec, SizeOfHashDelete);
 
                                /*
-                                * bucket buffer was not changed, but still 
needs to be
-                                * registered to ensure that we can acquire a 
cleanup lock on
-                                * it during replay.
+                                * If buf is an overflow page, the primary 
bucket page is not
+                                * changed and we hold only a pin on it, so it 
is not
+                                * registered.  Replay takes a cleanup lock on 
bucket_blkno
+                                * instead.
                                 */
-                               if (!xlrec.is_primary_bucket_page)
-                               {
-                                       uint8           flags = REGBUF_STANDARD 
| REGBUF_NO_IMAGE | REGBUF_NO_CHANGE;
-
-                                       XLogRegisterBuffer(0, bucket_buf, 
flags);
-                               }
-
                                XLogRegisterBuffer(1, buf, REGBUF_STANDARD);
                                XLogRegisterBufData(1, deletable,
                                                                        
ndeletable * sizeof(OffsetNumber));
diff --git a/src/backend/access/hash/hash_xlog.c 
b/src/backend/access/hash/hash_xlog.c
index e9a2b9aa9a7..04eb8e17dd4 100644
--- a/src/backend/access/hash/hash_xlog.c
+++ b/src/backend/access/hash/hash_xlog.c
@@ -469,6 +469,33 @@ hash_xlog_split_complete(XLogReaderState *record)
                UnlockReleaseBuffer(newbuf);
 }
 
+/*
+ * Take a cleanup lock on the primary bucket page of the bucket that a
+ * squeeze or delete record operates on, when that page is not one of the
+ * pages the record changes and so is not registered in it.
+ *
+ * This serializes replay against scans on a hot standby, which pin the
+ * primary bucket page for their whole duration; see the comments in the
+ * callers.  The page is not modified, so nothing is done with it besides
+ * locking it.  block_id must be a block of the same index registered in
+ * the record.
+ */
+static Buffer
+hash_xlog_lock_bucket(XLogReaderState *record, uint8 block_id,
+                                         BlockNumber bucket_blkno)
+{
+       RelFileLocator rlocator;
+       ForkNumber      forknum;
+       Buffer          buf;
+
+       XLogRecGetBlockTag(record, block_id, &rlocator, &forknum, NULL);
+       buf = XLogReadBufferExtended(rlocator, forknum, bucket_blkno,
+                                                                RBM_NORMAL, 
InvalidBuffer);
+       if (BufferIsValid(buf))
+               LockBufferForCleanup(buf);
+       return buf;
+}
+
 /*
  * replay move of page contents for squeeze operation of hash index
  */
@@ -493,11 +520,7 @@ hash_xlog_move_page_contents(XLogReaderState *record)
                action = XLogReadBufferForRedoExtended(record, 1, RBM_NORMAL, 
true, &writebuf);
        else
        {
-               /*
-                * we don't care for return value as the purpose of reading 
bucketbuf
-                * is to ensure a cleanup lock on primary bucket page.
-                */
-               (void) XLogReadBufferForRedoExtended(record, 0, RBM_NORMAL, 
true, &bucketbuf);
+               bucketbuf = hash_xlog_lock_bucket(record, 1, 
xldata->bucket_blkno);
 
                action = XLogReadBufferForRedo(record, 1, &writebuf);
        }
@@ -617,14 +640,19 @@ hash_xlog_squeeze_page(XLogReaderState *record)
         * can miss some records or show the same record multiple times.
         */
        if (xldata->is_prim_bucket_same_wrt)
-               action = XLogReadBufferForRedoExtended(record, 1, RBM_NORMAL, 
true, &writebuf);
+       {
+               /* block 1 is absent if the primary bucket page is not modified 
*/
+               if (XLogRecHasBlockRef(record, 1))
+                       action = XLogReadBufferForRedoExtended(record, 1, 
RBM_NORMAL, true, &writebuf);
+               else
+               {
+                       bucketbuf = hash_xlog_lock_bucket(record, 2, 
xldata->bucket_blkno);
+                       action = BLK_NOTFOUND;
+               }
+       }
        else
        {
-               /*
-                * we don't care for return value as the purpose of reading 
bucketbuf
-                * is to ensure a cleanup lock on primary bucket page.
-                */
-               (void) XLogReadBufferForRedoExtended(record, 0, RBM_NORMAL, 
true, &bucketbuf);
+               bucketbuf = hash_xlog_lock_bucket(record, 2, 
xldata->bucket_blkno);
 
                if (xldata->ntups > 0 || xldata->is_prev_bucket_same_wrt)
                        action = XLogReadBufferForRedo(record, 1, &writebuf);
@@ -851,11 +879,7 @@ hash_xlog_delete(XLogReaderState *record)
                action = XLogReadBufferForRedoExtended(record, 1, RBM_NORMAL, 
true, &deletebuf);
        else
        {
-               /*
-                * we don't care for return value as the purpose of reading 
bucketbuf
-                * is to ensure a cleanup lock on primary bucket page.
-                */
-               (void) XLogReadBufferForRedoExtended(record, 0, RBM_NORMAL, 
true, &bucketbuf);
+               bucketbuf = hash_xlog_lock_bucket(record, 1, 
xldata->bucket_blkno);
 
                action = XLogReadBufferForRedo(record, 1, &deletebuf);
        }
diff --git a/src/backend/access/hash/hashovfl.c 
b/src/backend/access/hash/hashovfl.c
index dbc57ef958c..2bc9015c223 100644
--- a/src/backend/access/hash/hashovfl.c
+++ b/src/backend/access/hash/hashovfl.c
@@ -657,6 +657,7 @@ _hash_freeovflpage(Relation rel, Buffer bucketbuf, Buffer 
ovflbuf,
        {
                xl_hash_squeeze_page xlrec;
 
+               xlrec.bucket_blkno = BufferGetBlockNumber(bucketbuf);
                xlrec.prevblkno = prevblkno;
                xlrec.nextblkno = nextblkno;
                xlrec.ntups = nitups;
@@ -667,42 +668,21 @@ _hash_freeovflpage(Relation rel, Buffer bucketbuf, Buffer 
ovflbuf,
                XLogRegisterData(&xlrec, SizeOfHashSqueezePage);
 
                /*
-                * bucket buffer was not changed, but still needs to be 
registered to
-                * ensure that we can acquire a cleanup lock on it during 
replay.
+                * Register only the pages this record changes.  The primary 
bucket
+                * page is registered only if it is the write page and is 
modified;
+                * otherwise we may hold only a pin on it, and replay takes a 
cleanup
+                * lock on bucket_blkno instead.
                 */
-               if (!is_prim_bucket_same_wrt)
-               {
-                       uint8           flags = REGBUF_STANDARD | 
REGBUF_NO_IMAGE | REGBUF_NO_CHANGE;
-
-                       XLogRegisterBuffer(0, bucketbuf, flags);
-               }
-
-               if (nitups > 0)
+               if (mod_wbuf)
                {
                        XLogRegisterBuffer(1, wbuf, REGBUF_STANDARD);
-                       XLogRegisterBufData(1, itup_offsets,
-                                                               nitups * 
sizeof(OffsetNumber));
-                       for (int i = 0; i < nitups; i++)
-                               XLogRegisterBufData(1, itups[i], tups_size[i]);
-               }
-               else if (is_prim_bucket_same_wrt || is_prev_bucket_same_wrt)
-               {
-                       uint8           wbuf_flags;
-
-                       /*
-                        * A write buffer needs to be registered even if no 
tuples are
-                        * added to it to ensure that we can acquire a cleanup 
lock on it
-                        * if it is the same as primary bucket buffer or update 
the
-                        * nextblkno if it is same as the previous bucket 
buffer.
-                        */
-                       Assert(nitups == 0);
-
-                       wbuf_flags = REGBUF_STANDARD;
-                       if (!is_prev_bucket_same_wrt)
-                               wbuf_flags |= REGBUF_NO_CHANGE;
-                       else
-                               Assert(mod_wbuf);
-                       XLogRegisterBuffer(1, wbuf, wbuf_flags);
+                       if (nitups > 0)
+                       {
+                               XLogRegisterBufData(1, itup_offsets,
+                                                                       nitups 
* sizeof(OffsetNumber));
+                               for (int i = 0; i < nitups; i++)
+                                       XLogRegisterBufData(1, itups[i], 
tups_size[i]);
+                       }
                }
 
                XLogRegisterBuffer(2, ovflbuf, REGBUF_STANDARD);
@@ -991,6 +971,7 @@ readpage:
                                        {
                                                xl_hash_move_page_contents 
xlrec;
 
+                                               xlrec.bucket_blkno = 
bucket_blkno;
                                                xlrec.ntups = nitups;
                                                xlrec.is_prim_bucket_same_wrt = 
(wbuf == bucket_buf);
 
@@ -998,17 +979,11 @@ readpage:
                                                XLogRegisterData(&xlrec, 
SizeOfHashMovePageContents);
 
                                                /*
-                                                * bucket buffer was not 
changed, but still needs to
-                                                * be registered to ensure that 
we can acquire a
-                                                * cleanup lock on it during 
replay.
+                                                * If wbuf is an overflow page, 
the primary bucket
+                                                * page is not changed and we 
hold only a pin on it,
+                                                * so it is not registered.  
Replay takes a cleanup
+                                                * lock on bucket_blkno instead.
                                                 */
-                                               if 
(!xlrec.is_prim_bucket_same_wrt)
-                                               {
-                                                       int                     
flags = REGBUF_STANDARD | REGBUF_NO_IMAGE | REGBUF_NO_CHANGE;
-
-                                                       XLogRegisterBuffer(0, 
bucket_buf, flags);
-                                               }
-
                                                XLogRegisterBuffer(1, wbuf, 
REGBUF_STANDARD);
                                                XLogRegisterBufData(1, 
itup_offsets,
                                                                                
        nitups * sizeof(OffsetNumber));
diff --git a/src/backend/access/rmgrdesc/hashdesc.c 
b/src/backend/access/rmgrdesc/hashdesc.c
index 593013afe9c..6988b6a82c3 100644
--- a/src/backend/access/rmgrdesc/hashdesc.c
+++ b/src/backend/access/rmgrdesc/hashdesc.c
@@ -80,7 +80,8 @@ hash_desc(StringInfo buf, XLogReaderState *record)
                        {
                                xl_hash_move_page_contents *xlrec = 
(xl_hash_move_page_contents *) rec;
 
-                               appendStringInfo(buf, "ntups %d, is_primary %c",
+                               appendStringInfo(buf, "bucket_blkno %u, ntups 
%d, is_primary %c",
+                                                                
xlrec->bucket_blkno,
                                                                 xlrec->ntups,
                                                                 
xlrec->is_prim_bucket_same_wrt ? 'T' : 'F');
                                break;
@@ -89,7 +90,8 @@ hash_desc(StringInfo buf, XLogReaderState *record)
                        {
                                xl_hash_squeeze_page *xlrec = 
(xl_hash_squeeze_page *) rec;
 
-                               appendStringInfo(buf, "prevblkno %u, nextblkno 
%u, ntups %d, is_primary %c, is_previous %c",
+                               appendStringInfo(buf, "bucket_blkno %u, 
prevblkno %u, nextblkno %u, ntups %d, is_primary %c, is_previous %c",
+                                                                
xlrec->bucket_blkno,
                                                                 
xlrec->prevblkno,
                                                                 
xlrec->nextblkno,
                                                                 xlrec->ntups,
@@ -101,7 +103,8 @@ hash_desc(StringInfo buf, XLogReaderState *record)
                        {
                                xl_hash_delete *xlrec = (xl_hash_delete *) rec;
 
-                               appendStringInfo(buf, "clear_dead_marking %c, 
is_primary %c",
+                               appendStringInfo(buf, "bucket_blkno %u, 
clear_dead_marking %c, is_primary %c",
+                                                                
xlrec->bucket_blkno,
                                                                 
xlrec->clear_dead_marking ? 'T' : 'F',
                                                                 
xlrec->is_primary_bucket_page ? 'T' : 'F');
                                break;
diff --git a/src/include/access/hash_xlog.h b/src/include/access/hash_xlog.h
index 149aed69dc5..9d762ff70d3 100644
--- a/src/include/access/hash_xlog.h
+++ b/src/include/access/hash_xlog.h
@@ -129,12 +129,15 @@ typedef struct xl_hash_split_complete
  *
  * This data record is used for XLOG_HASH_MOVE_PAGE_CONTENTS
  *
- * Backup Blk 0: primary bucket page
  * Backup Blk 1: page containing moved tuples
  * Backup Blk 2: page from which tuples will be removed
+ *
+ * The primary bucket page is not registered; replay takes a cleanup lock on
+ * bucket_blkno instead.
  */
 typedef struct xl_hash_move_page_contents
 {
+       BlockNumber bucket_blkno;       /* primary bucket page */
        uint16          ntups;
        bool            is_prim_bucket_same_wrt;        /* true if the page to 
which
                                                                                
         * tuples are moved is same as
@@ -149,16 +152,20 @@ typedef struct xl_hash_move_page_contents
  *
  * This data record is used for XLOG_HASH_SQUEEZE_PAGE
  *
- * Backup Blk 0: primary bucket page
- * Backup Blk 1: page containing tuples moved from freed overflow page
+ * Backup Blk 1: page containing tuples moved from freed overflow page, if
+ *                              it is modified
  * Backup Blk 2: freed overflow page
  * Backup Blk 3: page previous to the freed overflow page
  * Backup Blk 4: page next to the freed overflow page
  * Backup Blk 5: bitmap page containing info of freed overflow page
  * Backup Blk 6: meta page
+ *
+ * The primary bucket page is registered only as block 1, when it is the
+ * modified write page; otherwise replay takes a cleanup lock on bucket_blkno.
  */
 typedef struct xl_hash_squeeze_page
 {
+       BlockNumber bucket_blkno;       /* primary bucket page */
        BlockNumber prevblkno;
        BlockNumber nextblkno;
        uint16          ntups;
@@ -179,11 +186,14 @@ typedef struct xl_hash_squeeze_page
  *
  * This data record is used for XLOG_HASH_DELETE
  *
- * Backup Blk 0: primary bucket page
  * Backup Blk 1: page from which tuples are deleted
+ *
+ * If block 1 is not the primary bucket page, replay takes a cleanup lock on
+ * bucket_blkno, which is not registered.
  */
 typedef struct xl_hash_delete
 {
+       BlockNumber bucket_blkno;       /* primary bucket page */
        bool            clear_dead_marking; /* true if this operation clears
                                                                         * 
LH_PAGE_HAS_DEAD_TUPLES flag */
        bool            is_primary_bucket_page; /* true if the operation is for
diff --git a/src/include/access/xlog_internal.h 
b/src/include/access/xlog_internal.h
index bf609c3c703..ed8f488dda5 100644
--- a/src/include/access/xlog_internal.h
+++ b/src/include/access/xlog_internal.h
@@ -32,7 +32,7 @@
 /*
  * Each page of XLOG file has a header like this:
  */
-#define XLOG_PAGE_MAGIC 0xD122 /* can be used as WAL version indicator */
+#define XLOG_PAGE_MAGIC 0xD123 /* can be used as WAL version indicator */
 
 typedef struct XLogPageHeaderData
 {
-- 
2.50.1

From 02a691127756c5b24b53cea20aa90e8839d8f397 Mon Sep 17 00:00:00 2001
From: Greg Burd <[email protected]>
Date: Fri, 30 Oct 2026 10:17:46 -0400
Subject: [PATCH v4 4/4] Remove REGBUF_NO_CHANGE

With the previous commits, the hash index AM no longer registers any
pin-only or otherwise-unchanged buffer, so REGBUF_NO_CHANGE has no users
left.  Remove the flag and restore the invariant that every registered
buffer is content-locked and dirty: XLogRegisterBuffer() asserts it
unconditionally again.

This also reverts the defensive measures added in 27caf4ca5ae5, which
let a REGBUF_NO_CHANGE buffer skip the full-page image forced by
wal_consistency_checking: XLogRecordAssemble() no longer special-cases
the flag, and verifyBackupPageConsistency() again asserts that a
registered block carries an image.  27caf4ca5ae5 remains in the back
branches, where this hash rework is not applied.

Discussion: https://postgr.es/m/[email protected]
---
 src/backend/access/transam/README         |  5 ----
 src/backend/access/transam/xloginsert.c   | 28 ++++++-----------------
 src/backend/access/transam/xlogrecovery.c |  7 +-----
 src/include/access/xloginsert.h           |  3 ---
 4 files changed, 8 insertions(+), 35 deletions(-)

diff --git a/src/backend/access/transam/README 
b/src/backend/access/transam/README
index 595afa01fa7..231106270fd 100644
--- a/src/backend/access/transam/README
+++ b/src/backend/access/transam/README
@@ -586,11 +586,6 @@ void XLogRegisterBuffer(uint8 block_id, Buffer buf, uint8 
flags);
     XLogRegisterBufData() is included in the WAL record even if a full-page
     image is taken.
 
-    The REGBUF_NO_CHANGE flag indicates that the buffer is registered without
-    being modified by this record, and possibly without an exclusive content
-    lock.  wal_consistency_checking does not force a full-page image for such
-    a buffer.
-
 void XLogRegisterData(const void *data, int len);
 
     XLogRegisterData is used to include arbitrary data in the WAL record.  If
diff --git a/src/backend/access/transam/xloginsert.c 
b/src/backend/access/transam/xloginsert.c
index cd3359434e1..2d031e38594 100644
--- a/src/backend/access/transam/xloginsert.c
+++ b/src/backend/access/transam/xloginsert.c
@@ -257,22 +257,13 @@ XLogRegisterBuffer(uint8 block_id, Buffer buffer, uint8 
flags)
        Assert(begininsert_called);
 
        /*
-        * Ordinarily, the buffer should be exclusive-locked (or share-exclusive
-        * in case of hint bits) and marked dirty before we get here, otherwise 
we
-        * could end up violating one of the rules in access/transam/README.
-        *
-        * Some callers intentionally register a clean page and never update 
that
-        * page's LSN; in that case they can pass the flag REGBUF_NO_CHANGE to
-        * bypass these checks.
+        * The buffer should be exclusive-locked (or share-exclusive in case of
+        * hint bits) and marked dirty before we get here, otherwise we could 
end
+        * up violating one of the rules in access/transam/README.
         */
-#ifdef USE_ASSERT_CHECKING
-       if (!(flags & REGBUF_NO_CHANGE))
-       {
-               Assert(BufferIsDirty(buffer));
-               Assert(BufferIsLockedByMeInMode(buffer, BUFFER_LOCK_EXCLUSIVE) 
||
-                          BufferIsLockedByMeInMode(buffer, 
BUFFER_LOCK_SHARE_EXCLUSIVE));
-       }
-#endif
+       Assert(BufferIsDirty(buffer));
+       Assert(BufferIsLockedByMeInMode(buffer, BUFFER_LOCK_EXCLUSIVE) ||
+                  BufferIsLockedByMeInMode(buffer, 
BUFFER_LOCK_SHARE_EXCLUSIVE));
 
        if (block_id >= max_registered_block_id)
        {
@@ -722,13 +713,8 @@ XLogRecordAssemble(RmgrId rmid, uint8 info,
                /*
                 * If needs_backup is true or WAL checking is enabled for 
current
                 * resource manager, log a full-page write for the current 
block.
-                *
-                * Do not force an image for a REGBUF_NO_CHANGE buffer.
                 */
-               if (regbuf->flags & REGBUF_NO_CHANGE)
-                       include_image = needs_backup;
-               else
-                       include_image = needs_backup || (info & 
XLR_CHECK_CONSISTENCY) != 0;
+               include_image = needs_backup || (info & XLR_CHECK_CONSISTENCY) 
!= 0;
 
                if (include_image)
                {
diff --git a/src/backend/access/transam/xlogrecovery.c 
b/src/backend/access/transam/xlogrecovery.c
index 52ac6d58423..7a1de0567a8 100644
--- a/src/backend/access/transam/xlogrecovery.c
+++ b/src/backend/access/transam/xlogrecovery.c
@@ -2488,12 +2488,7 @@ verifyBackupPageConsistency(XLogReaderState *record)
                        continue;
                }
 
-               /*
-                * Blocks registered with REGBUF_NO_CHANGE may have no image, 
see
-                * XLogRecordAssemble().  Nothing to compare for these.
-                */
-               if (!XLogRecHasBlockImage(record, block_id))
-                       continue;
+               Assert(XLogRecHasBlockImage(record, block_id));
 
                if (XLogRecBlockImageApply(record, block_id))
                {
diff --git a/src/include/access/xloginsert.h b/src/include/access/xloginsert.h
index 86264564ebb..ddb1a85db59 100644
--- a/src/include/access/xloginsert.h
+++ b/src/include/access/xloginsert.h
@@ -38,9 +38,6 @@
                                                                         * will 
be skipped) */
 #define REGBUF_KEEP_DATA       0x10    /* include data even if a full-page 
image
                                                                         * is 
taken */
-#define REGBUF_NO_CHANGE       0x20    /* intentionally register clean buffer; 
no
-                                                                        * 
image is forced for WAL consistency
-                                                                        * 
checking */
 
 /* prototypes for public functions in xloginsert.c: */
 extern void XLogBeginInsert(void);
-- 
2.50.1

From 358e876f2961d926fe93e5a5c445b8a357a24306 Mon Sep 17 00:00:00 2001
From: Greg Burd <[email protected]>
Date: Fri, 30 Oct 2026 10:10:03 -0400
Subject: [PATCH v4 3/4] Add a recovery test for the hash WAL pin-only bucket
 page fix

Add recovery/t/061_hash_vacuum_insert_crc.pl, which exercises the
interleaving the previous commits make safe: a hash index VACUUM logging
an overflow-page delete while a concurrent INSERT changes the primary
bucket page it holds pin-only, with wal_consistency_checking='hash'.

The test adds a general, test-only injection point in
XLogRecordAssemble(), right after the record CRC is computed and before
the record is copied into the WAL buffers.  Its argument names the
record as "rmid/info/mainlen" so a test can wait on exactly one kind of
record; here the test waits on the overflow-page XLOG_HASH_DELETE (whose
main data carries the appended primary bucket block number) while VACUUM
holds the primary bucket page pin-only, then lets an INSERT change that
page, then crashes and checks that pg_waldump and crash recovery both
accept the WAL with no "incorrect resource manager data checksum".

Before 27caf4ca5ae5 and this rework, the record's CRC was computed over
the concurrently-modified primary bucket page and replay failed; the
injection point makes that window deterministic so the case is covered
from now on.

Co-authored-by: Vignesh C <[email protected]>
Discussion: 
https://postgr.es/m/CALDaNm14vRb2TZWyhjSHx=SyT7R=e7+ackrmxuojdwwee40...@mail.gmail.com
---
 src/backend/access/transam/xloginsert.c       |  22 +++
 src/test/recovery/meson.build                 |   1 +
 .../recovery/t/061_hash_vacuum_insert_crc.pl  | 181 ++++++++++++++++++
 3 files changed, 204 insertions(+)
 create mode 100644 src/test/recovery/t/061_hash_vacuum_insert_crc.pl

diff --git a/src/backend/access/transam/xloginsert.c 
b/src/backend/access/transam/xloginsert.c
index 341deb7b80d..cd3359434e1 100644
--- a/src/backend/access/transam/xloginsert.c
+++ b/src/backend/access/transam/xloginsert.c
@@ -39,6 +39,7 @@
 #include "replication/origin.h"
 #include "storage/bufmgr.h"
 #include "storage/proc.h"
+#include "utils/injection_point.h"
 #include "utils/memutils.h"
 #include "utils/pgstat_internal.h"
 #include "utils/rel.h"
@@ -991,6 +992,27 @@ XLogRecordAssemble(RmgrId rmid, uint8 info,
        for (rdt = hdr_rdt.next; rdt != NULL; rdt = rdt->next)
                COMP_CRC32C(rdata_crc, rdt->data, rdt->len);
 
+       /*
+        * Test hook: let a test modify a buffer after the record's CRC has been
+        * computed but before the record is copied into the WAL buffers, to
+        * reproduce the window in which a concurrently modified, pin-only
+        * registered buffer could make the on-disk CRC not match the record.  
The
+        * argument names the record as "rmid/info/mainlen" (resource manager, 
its
+        * info bits, and the length of the main data) so a test can wait on
+        * exactly one kind of record.  This runs inside a critical section, so 
it
+        * must not allocate; it fires only in a backend that loaded the point
+        * beforehand.
+        */
+#ifdef USE_INJECTION_POINTS
+       {
+               char            pointarg[24];
+
+               snprintf(pointarg, sizeof(pointarg), "%d/%02X/" UINT64_FORMAT,
+                                rmid, info, mainrdata_len);
+               INJECTION_POINT_CACHED("xlog-assemble-after-crc", pointarg);
+       }
+#endif
+
        /*
         * Ensure that the XLogRecord is not too large.
         *
diff --git a/src/test/recovery/meson.build b/src/test/recovery/meson.build
index b6ac9dd07a7..5051031df56 100644
--- a/src/test/recovery/meson.build
+++ b/src/test/recovery/meson.build
@@ -69,6 +69,7 @@ tests += {
       't/058_shutdown_crash_restart.pl',
       't/059_remote_apply_status_interval.pl',
       't/060_multixact_crashed_updater.pl',
+      't/061_hash_vacuum_insert_crc.pl',
     ],
   },
 }
diff --git a/src/test/recovery/t/061_hash_vacuum_insert_crc.pl 
b/src/test/recovery/t/061_hash_vacuum_insert_crc.pl
new file mode 100644
index 00000000000..aa030abd587
--- /dev/null
+++ b/src/test/recovery/t/061_hash_vacuum_insert_crc.pl
@@ -0,0 +1,181 @@
+# Copyright (c) 2026, PostgreSQL Global Development Group
+
+# Test that the WAL record written by hash index VACUUM for an overflow page is
+# intact when a concurrent INSERT changes the primary bucket page.
+#
+# hashbucketcleanup() keeps only a pin on the primary bucket page while it
+# walks the overflow chain.  Before the hash WAL rework it registered that page
+# (REGBUF_STANDARD | REGBUF_NO_IMAGE | REGBUF_NO_CHANGE) with the
+# XLOG_HASH_DELETE record it writes for each overflow page.  With
+# wal_consistency_checking enabled for the hash resource manager,
+# XLogRecordAssemble() included a full image of every registered block, so it
+# read the unlocked primary bucket page.  If an INSERT added a tuple to that
+# page after the record's CRC was computed but before the page was copied into
+# the WAL buffers, the record was written with a CRC that did not match its
+# contents, and replay stopped at it with "incorrect resource manager data
+# checksum".
+#
+# The window is a few microseconds wide, so the test widens it with an
+# injection point in XLogRecordAssemble(), placed right after the CRC is
+# computed.  It is attached to the VACUUM session only and matches just the
+# overflow-page delete, identified by "rmgr id/xl_info/main-data length".  The
+# point still fires after the fix, so the test exercises the same interleaving
+# on a correct server.
+#
+# The assertions state the correct behavior (all WAL is valid and replays), so
+# the test fails while the bug exists.
+
+use strict;
+use warnings FATAL => 'all';
+
+use IPC::Run;
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+use Test::More;
+
+if ($ENV{enable_injection_points} ne 'yes')
+{
+       plan skip_all => 'Injection points not supported by this build';
+}
+
+my $node = PostgreSQL::Test::Cluster->new('primary');
+$node->init;
+$node->append_conf(
+       'postgresql.conf', qq{
+wal_consistency_checking = 'hash'
+autovacuum = off
+fsync = off
+checkpoint_timeout = 1h
+});
+$node->start;
+
+if (!$node->check_extension('injection_points'))
+{
+       plan skip_all => 'Extension injection_points not installed';
+}
+$node->safe_psql('postgres', 'CREATE EXTENSION injection_points');
+
+my $hash_rmid = $node->safe_psql('postgres',
+       q{SELECT rm_id FROM pg_get_wal_resource_managers() WHERE rm_name = 
'Hash'}
+);
+
+# Build a single bucket whose primary page stays live and whose overflow page
+# is all dead, so VACUUM logs an XLOG_HASH_DELETE for the overflow page only:
+# the primary page has no dead index entries, so no delete record is written
+# for it, and the only consistency-checked hash DELETE in the window is the
+# overflow one, logged while the primary page is held pin-only.  All rows share
+# the key, so they land in one bucket; the first batch fills most of the
+# primary page and stays live, the second spills onto the overflow page and is
+# deleted.  Free slots are left on the primary page so the concurrent INSERT
+# below lands there.
+#
+# hashbuild() sizes the index from estimate_rel_size() on the heap, which for a
+# table that has never been vacuumed applies a floor of 10 pages.  Vacuuming
+# first sets reltuples to 0, so the estimate is believed and the index starts
+# with the minimum of 2 buckets.
+$node->safe_psql(
+       'postgres', q{
+CREATE TABLE hash_t (k int, tag int) WITH (autovacuum_enabled = false);
+VACUUM hash_t;
+CREATE INDEX hash_i ON hash_t USING hash (k);
+INSERT INTO hash_t SELECT 1, 0 FROM generate_series(1, 390);
+INSERT INTO hash_t SELECT 1, 1 FROM generate_series(1, 210);
+DELETE FROM hash_t WHERE tag = 1;
+});
+
+# Premise: metapage, two bucket pages, a bitmap page and one overflow page.
+is( $node->safe_psql(
+               'postgres',
+               q{SELECT pg_relation_size('hash_i') / 
current_setting('block_size')::int}
+       ),
+       '5',
+       'premise: the index has a primary bucket page and one overflow page');
+
+# Start from a checkpoint, so crash recovery replays everything below and
+# pg_waldump has a record boundary to start from.
+$node->safe_psql('postgres', 'CHECKPOINT');
+my $start_lsn = $node->safe_psql('postgres', 'SELECT pg_current_wal_lsn()');
+
+# The VACUUM session.  The point is attached to this session only and must be
+# loaded before the critical section in which it runs.
+#
+# Wait on the XLOG_HASH_DELETE record for the overflow page, not the one for
+# the primary page.  Both are hash records (rm id $hash_rmid) with info 0x92
+# (XLOG_HASH_DELETE 0x90 | XLR_CHECK_CONSISTENCY 0x02).  They differ in main
+# data length: the primary-page delete carries just xl_hash_delete
+# (SizeOfHashDelete, two bools = 2 bytes), while the overflow-page delete also
+# appends the primary bucket page's block number (a 4-byte BlockNumber), for 6
+# bytes.  Waiting on the 6-byte one parks VACUUM while it holds the primary
+# bucket page pin-only, which is the page the INSERT below changes.
+my $overflow_delete = "$hash_rmid/92/6";
+my $vacuum = $node->background_psql('postgres');
+my $vacuum_pid = $vacuum->query_safe('SELECT pg_backend_pid()');
+$vacuum->query_safe(q{SELECT injection_points_set_local()});
+$vacuum->query_safe(
+       qq{SELECT injection_points_attach('xlog-assemble-after-crc', 'wait', 
'$overflow_delete')}
+);
+$vacuum->query_safe(
+       q{SELECT injection_points_load('xlog-assemble-after-crc')});
+$vacuum->query_until(
+       qr/starting_vacuum/,
+       q(\echo starting_vacuum
+VACUUM (INDEX_CLEANUP on) hash_t;
+));
+
+# VACUUM has removed the dead entries from the primary page, released its lock
+# (keeping the pin), and is now logging the removal from the overflow page: the
+# CRC of that record is computed and the unlocked primary page is not yet
+# copied.
+$node->poll_query_until('postgres',
+       "SELECT wait_event = 'xlog-assemble-after-crc' FROM pg_stat_activity 
WHERE pid = $vacuum_pid"
+) or die "VACUUM did not reach the injection point";
+note('VACUUM is between computing the CRC and copying the record');
+
+# Now there is room in the primary page, so this INSERT changes it while VACUUM
+# holds it pin-only during the overflow delete.
+$node->safe_psql('postgres', q{INSERT INTO hash_t VALUES (1, 0)});
+
+$node->safe_psql('postgres',
+       q{SELECT injection_points_wakeup('xlog-assemble-after-crc')});
+$vacuum->quit;
+
+my $end_lsn =
+  $node->safe_psql('postgres', 'SELECT pg_current_wal_insert_lsn()');
+$node->poll_query_until('postgres',
+       "SELECT pg_current_wal_flush_lsn() >= '$end_lsn'")
+  or die "timed out waiting for WAL flush";
+
+# Crash, so that start-up has to replay (and CRC-check) the workload.
+$node->stop('immediate');
+
+# 1. Every record in the range must have a valid CRC.  pg_waldump verifies it
+# for every record it reads, whatever the --rmgr filter.
+my $stderr;
+my $ok = IPC::Run::run(
+       [
+               'pg_waldump',
+               '--path' => $node->data_dir . '/pg_wal',
+               '--start' => $start_lsn,
+               '--end' => $end_lsn,
+               '--rmgr' => 'Hash',
+       ],
+       '>' => '/dev/null',
+       '2>' => \$stderr);
+
+ok($ok, 'pg_waldump reads the whole workload without errors');
+unlike(
+       $stderr,
+       qr/incorrect resource manager data checksum/,
+       'no WAL record has a mismatching CRC');
+
+# 2. Crash recovery must replay it without complaint.
+my $log_offset = -s $node->logfile;
+$node->start;
+my $log = slurp_file($node->logfile, $log_offset);
+unlike(
+       $log,
+       qr/incorrect resource manager data checksum|inconsistent page found/,
+       'crash recovery replays the workload without CRC or consistency 
errors');
+
+$node->stop;
+done_testing();
-- 
2.50.1

From a1221c88a3f8916321c68c8230a5009f9c4527de Mon Sep 17 00:00:00 2001
From: Greg Burd <[email protected]>
Date: Fri, 30 Oct 2026 09:13:23 -0400
Subject: [PATCH v4 2/4] Append the hash primary bucket block number to WAL
 records only when needed

The previous commit stores the primary bucket page's block number in
xl_hash_delete, xl_hash_move_page_contents and xl_hash_squeeze_page.
Replay only uses it when the primary bucket page is not one of the pages
the record changes, so in the other cases the four bytes are wasted.

Remove the field from the three structs, and append the block number to
the main data of the record only when the primary bucket page is not
registered or modified by it.  Replay reads it from just after the
fixed-size struct.  The structs return to their previous sizes, and the
records are no larger than before in the common case.

Suggested-by: Amit Kapila <[email protected]>
Author: Vignesh C <[email protected]>
Discussion: 
https://postgr.es/m/caldanm2l_0ej1ykbp4om9xdyrghpyk3iu49nbc5hcre3f+m...@mail.gmail.com
---
 src/backend/access/hash/README         |  2 +-
 src/backend/access/hash/hash.c         |  8 ++++---
 src/backend/access/hash/hash_xlog.c    | 19 ++++++++++------
 src/backend/access/hash/hashovfl.c     | 31 +++++++++++++++++---------
 src/backend/access/rmgrdesc/hashdesc.c |  9 +++-----
 src/include/access/hash_xlog.h         | 19 +++++++++-------
 6 files changed, 53 insertions(+), 35 deletions(-)

diff --git a/src/backend/access/hash/README b/src/backend/access/hash/README
index 6d9b14e2d21..7774e1d2fe0 100644
--- a/src/backend/access/hash/README
+++ b/src/backend/access/hash/README
@@ -647,7 +647,7 @@ that no scan on a hot standby is in progress in the bucket 
or can start in
 it while tuples are removed or moved; such a scan could otherwise miss
 tuples or return them twice.  When the primary bucket page is not one of
 the pages the record changes, the operation holds only a pin on it, so it
-is not registered in the record.  The record stores its block number
+is not registered in the record.  The record then carries its block number
 instead, and replay locks that block directly.
 
 
diff --git a/src/backend/access/hash/hash.c b/src/backend/access/hash/hash.c
index fc93d78f6bb..9dbd008fed9 100644
--- a/src/backend/access/hash/hash.c
+++ b/src/backend/access/hash/hash.c
@@ -902,7 +902,6 @@ 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;
                                xlrec.is_primary_bucket_page = (buf == 
bucket_buf);
 
@@ -912,9 +911,12 @@ hashbucketcleanup(Relation rel, Bucket cur_bucket, Buffer 
bucket_buf,
                                /*
                                 * If buf is an overflow page, the primary 
bucket page is not
                                 * changed and we hold only a pin on it, so it 
is not
-                                * registered.  Replay takes a cleanup lock on 
bucket_blkno
-                                * instead.
+                                * registered.  Append its block number 
instead, and replay
+                                * takes a cleanup lock on it.  It is not 
needed otherwise.
                                 */
+                               if (!xlrec.is_primary_bucket_page)
+                                       XLogRegisterData(&bucket_blkno, 
sizeof(BlockNumber));
+
                                XLogRegisterBuffer(1, buf, REGBUF_STANDARD);
                                XLogRegisterBufData(1, deletable,
                                                                        
ndeletable * sizeof(OffsetNumber));
diff --git a/src/backend/access/hash/hash_xlog.c 
b/src/backend/access/hash/hash_xlog.c
index 04eb8e17dd4..1484c12d948 100644
--- a/src/backend/access/hash/hash_xlog.c
+++ b/src/backend/access/hash/hash_xlog.c
@@ -472,7 +472,9 @@ hash_xlog_split_complete(XLogReaderState *record)
 /*
  * Take a cleanup lock on the primary bucket page of the bucket that a
  * squeeze or delete record operates on, when that page is not one of the
- * pages the record changes and so is not registered in it.
+ * pages the record changes and so is not registered in it.  Its block number
+ * is stored in the main data of the record, right after the fixed-size struct
+ * of rec_size bytes.
  *
  * This serializes replay against scans on a hot standby, which pin the
  * primary bucket page for their whole duration; see the comments in the
@@ -481,13 +483,16 @@ hash_xlog_split_complete(XLogReaderState *record)
  * the record.
  */
 static Buffer
-hash_xlog_lock_bucket(XLogReaderState *record, uint8 block_id,
-                                         BlockNumber bucket_blkno)
+hash_xlog_lock_bucket(XLogReaderState *record, uint8 block_id, Size rec_size)
 {
        RelFileLocator rlocator;
        ForkNumber      forknum;
+       BlockNumber bucket_blkno;
        Buffer          buf;
 
+       Assert(XLogRecGetDataLen(record) == rec_size + sizeof(BlockNumber));
+       memcpy(&bucket_blkno, XLogRecGetData(record) + rec_size, 
sizeof(BlockNumber));
+
        XLogRecGetBlockTag(record, block_id, &rlocator, &forknum, NULL);
        buf = XLogReadBufferExtended(rlocator, forknum, bucket_blkno,
                                                                 RBM_NORMAL, 
InvalidBuffer);
@@ -520,7 +525,7 @@ hash_xlog_move_page_contents(XLogReaderState *record)
                action = XLogReadBufferForRedoExtended(record, 1, RBM_NORMAL, 
true, &writebuf);
        else
        {
-               bucketbuf = hash_xlog_lock_bucket(record, 1, 
xldata->bucket_blkno);
+               bucketbuf = hash_xlog_lock_bucket(record, 1, 
SizeOfHashMovePageContents);
 
                action = XLogReadBufferForRedo(record, 1, &writebuf);
        }
@@ -646,13 +651,13 @@ hash_xlog_squeeze_page(XLogReaderState *record)
                        action = XLogReadBufferForRedoExtended(record, 1, 
RBM_NORMAL, true, &writebuf);
                else
                {
-                       bucketbuf = hash_xlog_lock_bucket(record, 2, 
xldata->bucket_blkno);
+                       bucketbuf = hash_xlog_lock_bucket(record, 2, 
SizeOfHashSqueezePage);
                        action = BLK_NOTFOUND;
                }
        }
        else
        {
-               bucketbuf = hash_xlog_lock_bucket(record, 2, 
xldata->bucket_blkno);
+               bucketbuf = hash_xlog_lock_bucket(record, 2, 
SizeOfHashSqueezePage);
 
                if (xldata->ntups > 0 || xldata->is_prev_bucket_same_wrt)
                        action = XLogReadBufferForRedo(record, 1, &writebuf);
@@ -879,7 +884,7 @@ hash_xlog_delete(XLogReaderState *record)
                action = XLogReadBufferForRedoExtended(record, 1, RBM_NORMAL, 
true, &deletebuf);
        else
        {
-               bucketbuf = hash_xlog_lock_bucket(record, 1, 
xldata->bucket_blkno);
+               bucketbuf = hash_xlog_lock_bucket(record, 1, SizeOfHashDelete);
 
                action = XLogReadBufferForRedo(record, 1, &deletebuf);
        }
diff --git a/src/backend/access/hash/hashovfl.c 
b/src/backend/access/hash/hashovfl.c
index 2bc9015c223..91781f7c2b0 100644
--- a/src/backend/access/hash/hashovfl.c
+++ b/src/backend/access/hash/hashovfl.c
@@ -580,9 +580,13 @@ _hash_freeovflpage(Relation rel, Buffer bucketbuf, Buffer 
ovflbuf,
        /* Get write-lock on metapage to update firstfree */
        LockBuffer(metabuf, BUFFER_LOCK_EXCLUSIVE);
 
-       /* This operation needs to log multiple tuples, prepare WAL for that */
+       /*
+        * This operation needs to log multiple tuples, prepare WAL for that.  
One
+        * extra data segment accounts for the primary bucket page's block 
number,
+        * appended to the record when that page is pin-only (not registered).
+        */
        if (RelationNeedsWAL(rel))
-               XLogEnsureRecordSpace(HASH_XLOG_FREE_OVFL_BUFS, 4 + nitups);
+               XLogEnsureRecordSpace(HASH_XLOG_FREE_OVFL_BUFS, 5 + nitups);
 
        START_CRIT_SECTION();
 
@@ -656,8 +660,8 @@ _hash_freeovflpage(Relation rel, Buffer bucketbuf, Buffer 
ovflbuf,
        if (RelationNeedsWAL(rel))
        {
                xl_hash_squeeze_page xlrec;
+               BlockNumber bucket_blkno = BufferGetBlockNumber(bucketbuf);
 
-               xlrec.bucket_blkno = BufferGetBlockNumber(bucketbuf);
                xlrec.prevblkno = prevblkno;
                xlrec.nextblkno = nextblkno;
                xlrec.ntups = nitups;
@@ -670,9 +674,12 @@ _hash_freeovflpage(Relation rel, Buffer bucketbuf, Buffer 
ovflbuf,
                /*
                 * Register only the pages this record changes.  The primary 
bucket
                 * page is registered only if it is the write page and is 
modified;
-                * otherwise we may hold only a pin on it, and replay takes a 
cleanup
-                * lock on bucket_blkno instead.
+                * otherwise we may hold only a pin on it, so append its block 
number
+                * and replay takes a cleanup lock on it.
                 */
+               if (!(is_prim_bucket_same_wrt && mod_wbuf))
+                       XLogRegisterData(&bucket_blkno, sizeof(BlockNumber));
+
                if (mod_wbuf)
                {
                        XLogRegisterBuffer(1, wbuf, REGBUF_STANDARD);
@@ -946,10 +953,12 @@ readpage:
 
                                        /*
                                         * This operation needs to log multiple 
tuples, prepare
-                                        * WAL for that.
+                                        * WAL for that.  One extra data 
segment accounts for the
+                                        * primary bucket page's block number, 
appended to the
+                                        * record when that page is pin-only 
(not registered).
                                         */
                                        if (RelationNeedsWAL(rel))
-                                               XLogEnsureRecordSpace(0, 3 + 
nitups);
+                                               XLogEnsureRecordSpace(0, 4 + 
nitups);
 
                                        START_CRIT_SECTION();
 
@@ -971,7 +980,6 @@ readpage:
                                        {
                                                xl_hash_move_page_contents 
xlrec;
 
-                                               xlrec.bucket_blkno = 
bucket_blkno;
                                                xlrec.ntups = nitups;
                                                xlrec.is_prim_bucket_same_wrt = 
(wbuf == bucket_buf);
 
@@ -981,9 +989,12 @@ readpage:
                                                /*
                                                 * If wbuf is an overflow page, 
the primary bucket
                                                 * page is not changed and we 
hold only a pin on it,
-                                                * so it is not registered.  
Replay takes a cleanup
-                                                * lock on bucket_blkno instead.
+                                                * so it is not registered.  
Append its block number
+                                                * instead, and replay takes a 
cleanup lock on it.
                                                 */
+                                               if 
(!xlrec.is_prim_bucket_same_wrt)
+                                                       
XLogRegisterData(&bucket_blkno, sizeof(BlockNumber));
+
                                                XLogRegisterBuffer(1, wbuf, 
REGBUF_STANDARD);
                                                XLogRegisterBufData(1, 
itup_offsets,
                                                                                
        nitups * sizeof(OffsetNumber));
diff --git a/src/backend/access/rmgrdesc/hashdesc.c 
b/src/backend/access/rmgrdesc/hashdesc.c
index 6988b6a82c3..593013afe9c 100644
--- a/src/backend/access/rmgrdesc/hashdesc.c
+++ b/src/backend/access/rmgrdesc/hashdesc.c
@@ -80,8 +80,7 @@ hash_desc(StringInfo buf, XLogReaderState *record)
                        {
                                xl_hash_move_page_contents *xlrec = 
(xl_hash_move_page_contents *) rec;
 
-                               appendStringInfo(buf, "bucket_blkno %u, ntups 
%d, is_primary %c",
-                                                                
xlrec->bucket_blkno,
+                               appendStringInfo(buf, "ntups %d, is_primary %c",
                                                                 xlrec->ntups,
                                                                 
xlrec->is_prim_bucket_same_wrt ? 'T' : 'F');
                                break;
@@ -90,8 +89,7 @@ hash_desc(StringInfo buf, XLogReaderState *record)
                        {
                                xl_hash_squeeze_page *xlrec = 
(xl_hash_squeeze_page *) rec;
 
-                               appendStringInfo(buf, "bucket_blkno %u, 
prevblkno %u, nextblkno %u, ntups %d, is_primary %c, is_previous %c",
-                                                                
xlrec->bucket_blkno,
+                               appendStringInfo(buf, "prevblkno %u, nextblkno 
%u, ntups %d, is_primary %c, is_previous %c",
                                                                 
xlrec->prevblkno,
                                                                 
xlrec->nextblkno,
                                                                 xlrec->ntups,
@@ -103,8 +101,7 @@ hash_desc(StringInfo buf, XLogReaderState *record)
                        {
                                xl_hash_delete *xlrec = (xl_hash_delete *) rec;
 
-                               appendStringInfo(buf, "bucket_blkno %u, 
clear_dead_marking %c, is_primary %c",
-                                                                
xlrec->bucket_blkno,
+                               appendStringInfo(buf, "clear_dead_marking %c, 
is_primary %c",
                                                                 
xlrec->clear_dead_marking ? 'T' : 'F',
                                                                 
xlrec->is_primary_bucket_page ? 'T' : 'F');
                                break;
diff --git a/src/include/access/hash_xlog.h b/src/include/access/hash_xlog.h
index 9d762ff70d3..12fc5f50ea5 100644
--- a/src/include/access/hash_xlog.h
+++ b/src/include/access/hash_xlog.h
@@ -132,12 +132,13 @@ typedef struct xl_hash_split_complete
  * Backup Blk 1: page containing moved tuples
  * Backup Blk 2: page from which tuples will be removed
  *
- * The primary bucket page is not registered; replay takes a cleanup lock on
- * bucket_blkno instead.
+ * The primary bucket page is not registered.  If it is not the page moved to
+ * (is_prim_bucket_same_wrt is false), the block number of the primary bucket
+ * page follows the struct as a BlockNumber, and replay takes a cleanup lock
+ * on that block.
  */
 typedef struct xl_hash_move_page_contents
 {
-       BlockNumber bucket_blkno;       /* primary bucket page */
        uint16          ntups;
        bool            is_prim_bucket_same_wrt;        /* true if the page to 
which
                                                                                
         * tuples are moved is same as
@@ -161,11 +162,12 @@ typedef struct xl_hash_move_page_contents
  * Backup Blk 6: meta page
  *
  * The primary bucket page is registered only as block 1, when it is the
- * modified write page; otherwise replay takes a cleanup lock on bucket_blkno.
+ * modified write page.  Otherwise the block number of the primary bucket
+ * page follows the struct as a BlockNumber, and replay takes a cleanup lock
+ * on that block.
  */
 typedef struct xl_hash_squeeze_page
 {
-       BlockNumber bucket_blkno;       /* primary bucket page */
        BlockNumber prevblkno;
        BlockNumber nextblkno;
        uint16          ntups;
@@ -188,12 +190,13 @@ typedef struct xl_hash_squeeze_page
  *
  * Backup Blk 1: page from which tuples are deleted
  *
- * If block 1 is not the primary bucket page, replay takes a cleanup lock on
- * bucket_blkno, which is not registered.
+ * If block 1 is not the primary bucket page (is_primary_bucket_page is
+ * false), the block number of the primary bucket page follows the struct as a
+ * BlockNumber, and replay takes a cleanup lock on that block.  The primary
+ * bucket page is not registered.
  */
 typedef struct xl_hash_delete
 {
-       BlockNumber bucket_blkno;       /* primary bucket page */
        bool            clear_dead_marking; /* true if this operation clears
                                                                         * 
LH_PAGE_HAS_DEAD_TUPLES flag */
        bool            is_primary_bucket_page; /* true if the operation is for
-- 
2.50.1

Reply via email to