On Fri, 30 Oct 2026 17:18:34 +0530, vignesh C <[email protected]> wrote:
> On Thu, 29 Oct 2026 at 21:42, Greg Burd <[email protected]> wrote:
> >
> > 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.
Hi Vignesh,
Thanks for picking this up and for the two things you added: splitting
the four-byte saving out as its own patch, and the back-branch analysis.
On the structure: I've taken your v3-0001 (same as my v2-0001) and your
v3-0002 (append the block number only when the primary page is not
registered) as the base, and built the rest of the series on top. The
attached v3 is:
0001 Don't register pin-only primary bucket pages in hash WAL records
(unchanged from your v3-0001 / my v2-0001)
0002 Append the hash primary bucket block number to WAL records only
when needed (your v3-0002, Amit's suggestion)
0003 Add a recovery test for the hash WAL pin-only bucket page fix
0004 Remove REGBUF_NO_CHANGE
0002 keeps the records the same size as today in the common case, so I
think it belongs in the series rather than as a follow-up; thanks for
writing it.
0003 is the recovery test. It turns the CRC window into a deterministic
one with 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. The injection point's argument names the record
as "rmid/info/mainlen", so the test waits on exactly the overflow-page
XLOG_HASH_DELETE (whose main data carries the appended bucket block
number, so it is distinguishable from the primary-page delete), lets a
concurrent INSERT change the pin-only primary bucket page, then crashes
and checks that pg_waldump and crash recovery both accept the WAL. This
is your test from the CRC thread, carried forward: the record selector
moved from the REGBUF_NO_CHANGE buffer count (which the rework zeroes)
to the main-data length so it still picks the right record after the
rework. I kept you as a co-author.
0004 removes REGBUF_NO_CHANGE and restores the unconditional assert in
XLogRegisterBuffer(), and reverts the two defensive measures from
27caf4ca5ae5 on master.
> The patches change the WAL records, and XLOG_PAGE_MAGIC cannot be
> bumped in a minor release, so I looked at what happens if they are
> backpatched. I built the base REL_18_STABLE, REL_18 + 0001, + 0002,
> and + 0003 (attached, adapted for REL_18: the squeeze code there lacks
> the locals present on master, so 0001 computes mod_wbuf itself) and
> ran them against each other with streaming replication and crash
> recovery, with wal_consistency_checking = 'hash' and VACUUM of hash
> indexes in which every shape of the three records occurs.
On your back-branch question: I don't think we should back-patch this.
27caf4ca5ae5 already fixes the actual CRC failure down to v17, so the
back branches are correct as they stand. The rework here is a cleanup
that changes three WAL record formats and bumps XLOG_PAGE_MAGIC, which,
as your p1/p2-vs-base runs show, is not compatible across a minor
release in either direction. That is exactly the kind of change we keep
to master. So I'd leave the back branches on 27caf4ca5ae5 and not carry
your v3_REL18-0003 compatibility shim; it is good to have proven the
incompatibility, but the conclusion is "master only", which 0001 already
says in its XLOG_PAGE_MAGIC bump.
One fix went into 0002 after a first CI round: pulling the primary
bucket block number out of the record struct into its own data segment
needs one more slot than XLogEnsureRecordSpace() reserved in
_hash_freeovflpage() and the move-page-contents path, so a 32-bit build
hit "too much WAL data" during a hash VACUUM that packs more tuples per
WAL record than a 64-bit build does. 0002 now reserves the extra
segment.
Patches 0001-0004 attached, against current master.
best.
-greg
From 69b23919f242c6b777737479578abb629c035160 Mon Sep 17 00:00:00 2001
From: Greg Burd <[email protected]>
Date: Fri, 30 Oct 2026 09:13:23 -0400
Subject: [PATCH v3 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.54.0
From 78c4c34714e48a72e1aa1b6e32396b16e12fce1d Mon Sep 17 00:00:00 2001
From: Greg Burd <[email protected]>
Date: Fri, 30 Oct 2026 10:10:03 -0400
Subject: [PATCH v3 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 | 176 ++++++++++++++++++
3 files changed, 199 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..f658299c7a0
--- /dev/null
+++ b/src/test/recovery/t/061_hash_vacuum_insert_crc.pl
@@ -0,0 +1,176 @@
+# 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.54.0
From 925228f4734f471663ddb2f9764447fb8899291a Mon Sep 17 00:00:00 2001
From: Greg Burd <[email protected]>
Date: Fri, 30 Oct 2026 10:17:46 -0400
Subject: [PATCH v3 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.54.0
From ad33f03e52ea021331a4431ed6bff64335674b9c Mon Sep 17 00:00:00 2001
From: Greg Burd <[email protected]>
Date: Thu, 29 Oct 2026 10:30:14 -0400
Subject: [PATCH v3 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.54.0
