From 22005e0e3e674261bb30c7b694cc471616461926 Mon Sep 17 00:00:00 2001 From: Vignesh C Date: Fri, 30 Oct 2026 10:58:12 +0530 Subject: [PATCH v3 2/2] 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. --- 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 | 17 +++++++++++------ src/backend/access/rmgrdesc/hashdesc.c | 9 +++------ src/include/access/hash_xlog.h | 19 +++++++++++-------- 6 files changed, 43 insertions(+), 31 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..a5a40ba6a91 100644 --- a/src/backend/access/hash/hashovfl.c +++ b/src/backend/access/hash/hashovfl.c @@ -656,8 +656,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 +670,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); @@ -971,7 +974,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 +983,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..c4a55f071bc 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.55.0