Hi again,

This is a follow-up to the hash-index WAL CRC thread [1].  Michael asked
that the larger hash rework not ride on that thread, since it is a
refactor that needs its own review attention rather than being buried
under a bug report [2]:

   "Could you start a new thread about all that, with all the authors of
    this original code in CC at least?  Hiding such a large refactoring on
    a thread for the CRC failure would not help in terms of attracting the
    attention this requires in terms of reviews."

So here it is, on its own thread, with the hash WAL-logging authors in
CC.

Background:

The CRC failure in [1] was that, under wal_consistency_checking='hash',
a standby could stop with "incorrect resource manager data checksum in
record".  The cause: hashbucketcleanup(), _hash_squeezebucket() and
_hash_freeovflpage() register the bucket's primary page with
REGBUF_NO_CHANGE while holding only a pin on it (no content lock),
solely so that replay takes a cleanup lock on that page.
XLR_CHECK_CONSISTENCY then forces a full-page image of a page a
concurrent inserter is still free to modify, and the record CRC is
computed over shifting data.

The immediate CRC fix is committed and back-patched to v17 as
27caf4ca5ae5 ("Do not take a full-page image for REGBUF_NO_CHANGE
buffers"): XLogRecordAssemble() no longer forces an image for a
REGBUF_NO_CHANGE buffer, and verifyBackupPageConsistency() skips a
registered block that has no image.

That stops the bleeding but leaves the real problem in place: hash
registers a buffer it does not hold locked or dirty, which breaks the
invariant every other caller of XLogRegisterBuffer() keeps.  The plan
proposed (agreed on?) in [1] was three steps:

   1. Avoid the CRC problem down to v17, including HEAD.   [done: 27caf4ca5ae5]
   2. Fix hash so it stops breaking the rule.              [0001 here]
   3. Remove REGBUF_NO_CHANGE and restore the asserts.     [0002 here]

This thread is steps 2 and 3.

0001: 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 primary bucket
page so that a hot-standby scan is not in progress in the bucket, and
cannot start in it, while tuples are removed or moved.  Today that lock
is obtained by registering the page (pin-only) with REGBUF_NO_CHANGE.
Redo never reads data from that block; the registration exists only for
the lock.

0001 instead stores the primary bucket page's block number in the main
data of these three records (a new bucket_blkno field) and has replay
read that block and call LockBufferForCleanup() on it directly, at the
same point and in the same order as today.  The page is registered only
when the record actually changes it.  For the squeeze record this also
removes the pin-only registration of the write page (the case where the
write page is the primary bucket page and nothing is moved onto it).

After 0001, hash registers only buffers it holds exclusively locked and
dirty, and there are no REGBUF_NO_CHANGE users left anywhere in the
tree.

Reading an unregistered block in redo has precedent: nbtree's "pin scan"
did exactly that (XLogReadBufferExtended + LockBufferForCleanup) until
9f83468b353 removed it as no longer needed, not as unsafe.  Two
consequences, both benign:

   - Recovery prefetch no longer sees the bucket page for these records,
     so redo may do one synchronous read of it when it is not already in
     shared buffers.
   - Block-change consumers (pg_rewind, the WAL summarizer) stop counting
     the bucket page as changed by these records.  It never was changed,
     so this is a correction, not a loss.

The three record formats change, so 0001 bumps XLOG_PAGE_MAGIC; master
only.

Why not just content-lock the primary page, as Jeff asked in [1]?

I looked for a safe placement and it deadlocks where the buffer is
registered.  In _hash_freeovflpage() that point is after the metapage is
locked exclusive.  A scan of a bucket still being populated by an
incomplete split holds that bucket's primary page share-locked and waits
for the metapage in _hash_get_oldblock_from_newbucket(); VACUUM holds
the metapage and waits for the primary page.  Both are buffer content
locks, so the deadlock detector does not see the cycle and a cancel does
not break it.  I reproduced it deterministically with an injection point
(3 of 3): pg_cancel_backend() on the VACUUM does nothing and every
insert queues behind the metapage.  Taking the bucket lock before the
metapage is read avoids the deadlock, but that variant still costs a
full-page image of the bucket page per checkpoint and holds inserts and
scans on the bucket across the bitmap/metapage work.  0001 as proposed
has neither cost and takes no new lock on the primary page.

0002: Remove REGBUF_NO_CHANGE

With 0001 there are no users of the flag left.  0002 removes it,
restores the unconditional assert in XLogRegisterBuffer() that every
registered buffer is content-locked and dirty, and reverts the two
defensive measures from 27caf4ca5ae5 on master: the XLogRecordAssemble()
special case and the verifyBackupPageConsistency() skip.  27caf4ca5ae5
stays in the back branches, where this hash rework is not applied.

Testing (16-vCPU Debian, cassert, master incl. 27caf4ca5ae5)

- meson test regress, isolation, and recovery (including
   recovery/027_stream_regress with
   PG_TEST_EXTRA=wal_consistency_checking): all pass with 0001+0002
   applied.

- CRC reproduction with 0001+0002, i.e. after 0002 has removed the
   step-1 escape and re-added the asserts: primary + streaming standby,
   wal_consistency_checking='hash', 8 sessions inserting and deleting on
   a single-bucket index with aggressive autovacuum.  The standby caught
   up with no "incorrect resource manager data checksum", no inconsistent
   page, and no assertion failure, 3 of 3.  This shows the fix comes from
   the hash rework itself, not from the step-1 safety net.

(Earlier, more detailed A/B/C/D runs on the older posting are in [3],
including a hot-standby cursor control showing the cleanup lock still
serializes replay against an in-progress scan.)

Open item from review:

Amit suggested [4] registering bucket_blkno only for the overflow-page
cases, since for a primary-bucket-page record the block is already
known, saving 4 bytes on the common records.  I have not made that
change yet; I can in the next version if we agree it is worth the extra
branch in each redo path.

Vignesh contributed a CRC injection-point test on the old thread that
I'd like to review and see how that might fit in and help.

best.

-greg

[1] https://postgr.es/m/[email protected]
[2] https://postgr.es/m/[email protected]
[3] https://postgr.es/m/auHpsdSdOXspzuqW@floki
[4] 
https://postgr.es/m/CAA4eK1+JpYJnUWwrSO7=gs=p3qhkytrv+gmmpky-erv3phd...@mail.gmail.com

From 378c674952fb89989cdf0102b4398e54d582cda1 Mon Sep 17 00:00:00 2001
From: Greg Burd <[email protected]>
Date: Thu, 29 Oct 2026 10:30:38 -0400
Subject: [PATCH v2 2/2] Remove REGBUF_NO_CHANGE

With the previous commit, 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 the 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 341deb7b80d..987a76c069b 100644
--- a/src/backend/access/transam/xloginsert.c
+++ b/src/backend/access/transam/xloginsert.c
@@ -256,22 +256,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)
        {
@@ -721,13 +712,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.54.0

From 3ebfc105e5842737c0421412bc0ee821cbe29d0c Mon Sep 17 00:00:00 2001
From: Greg Burd <[email protected]>
Date: Thu, 29 Oct 2026 10:30:14 -0400
Subject: [PATCH v2 1/2] 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.54.0

Reply via email to