From f14db744ecb79b121d8d0d3489384a93bd6abf07 Mon Sep 17 00:00:00 2001 From: Melanie Plageman Date: Tue, 16 Sep 2025 11:05:30 -0400 Subject: [PATCH v15 11/23] Remove heap buffer from XLOG_HEAP2_VISIBLE WAL chain Now that all users of visibilitymap_set() include setting PD_ALL_VISIBLE in the WAL record capturing other changes to the heap page, we no longer need to include the heap buffer in the WAL chain for setting the VM. --- src/backend/access/heap/heapam.c | 16 +----- src/backend/access/heap/heapam_xlog.c | 76 +++---------------------- src/backend/access/heap/vacuumlazy.c | 6 +- src/backend/access/heap/visibilitymap.c | 31 +--------- src/include/access/heapam_xlog.h | 3 +- src/include/access/visibilitymap.h | 2 +- 6 files changed, 16 insertions(+), 118 deletions(-) diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c index 7f354caec31..d4d83a6f9fe 100644 --- a/src/backend/access/heap/heapam.c +++ b/src/backend/access/heap/heapam.c @@ -8806,21 +8806,14 @@ bottomup_sort_and_shrink(TM_IndexDeleteOp *delstate) * * snapshotConflictHorizon comes from the largest xmin on the page being * marked all-visible. REDO routine uses it to generate recovery conflicts. - * - * If checksums or wal_log_hints are enabled, we may also generate a full-page - * image of heap_buffer. Otherwise, we optimize away the FPI (by specifying - * REGBUF_NO_IMAGE for the heap buffer), in which case the caller should *not* - * update the heap page's LSN. */ XLogRecPtr -log_heap_visible(Relation rel, Buffer heap_buffer, Buffer vm_buffer, +log_heap_visible(Relation rel, Buffer vm_buffer, TransactionId snapshotConflictHorizon, uint8 vmflags) { xl_heap_visible xlrec; XLogRecPtr recptr; - uint8 flags; - Assert(BufferIsValid(heap_buffer)); Assert(BufferIsValid(vm_buffer)); xlrec.snapshotConflictHorizon = snapshotConflictHorizon; @@ -8829,14 +8822,7 @@ log_heap_visible(Relation rel, Buffer heap_buffer, Buffer vm_buffer, xlrec.flags |= VISIBILITYMAP_XLOG_CATALOG_REL; XLogBeginInsert(); XLogRegisterData(&xlrec, SizeOfHeapVisible); - XLogRegisterBuffer(0, vm_buffer, 0); - - flags = REGBUF_STANDARD; - if (!XLogHintBitIsNeeded()) - flags |= REGBUF_NO_IMAGE; - XLogRegisterBuffer(1, heap_buffer, flags); - recptr = XLogInsert(RM_HEAP2_ID, XLOG_HEAP2_VISIBLE); return recptr; diff --git a/src/backend/access/heap/heapam_xlog.c b/src/backend/access/heap/heapam_xlog.c index 4ea1a186c98..2e9fda0a9bf 100644 --- a/src/backend/access/heap/heapam_xlog.c +++ b/src/backend/access/heap/heapam_xlog.c @@ -229,15 +229,12 @@ heap_xlog_visible(XLogReaderState *record) XLogRecPtr lsn = record->EndRecPtr; xl_heap_visible *xlrec = (xl_heap_visible *) XLogRecGetData(record); Buffer vmbuffer = InvalidBuffer; - Buffer buffer; - Page page; RelFileLocator rlocator; BlockNumber blkno; - XLogRedoAction action; Assert((xlrec->flags & VISIBILITYMAP_XLOG_VALID_BITS) == xlrec->flags); - XLogRecGetBlockTag(record, 1, &rlocator, NULL, &blkno); + XLogRecGetBlockTag(record, 0, &rlocator, NULL, &blkno); /* * If there are any Hot Standby transactions running that have an xmin @@ -254,70 +251,11 @@ heap_xlog_visible(XLogReaderState *record) rlocator); /* - * Read the heap page, if it still exists. If the heap file has dropped or - * truncated later in recovery, we don't need to update the page, but we'd - * better still update the visibility map. - */ - action = XLogReadBufferForRedo(record, 1, &buffer); - if (action == BLK_NEEDS_REDO) - { - /* - * We don't bump the LSN of the heap page when setting the visibility - * map bit (unless checksums or wal_hint_bits is enabled, in which - * case we must). This exposes us to torn page hazards, but since - * we're not inspecting the existing page contents in any way, we - * don't care. - */ - page = BufferGetPage(buffer); - - PageSetAllVisible(page); - - if (XLogHintBitIsNeeded()) - PageSetLSN(page, lsn); - - MarkBufferDirty(buffer); - } - else if (action == BLK_RESTORED) - { - /* - * If heap block was backed up, we already restored it and there's - * nothing more to do. (This can only happen with checksums or - * wal_log_hints enabled.) - */ - } - - if (BufferIsValid(buffer)) - { - Size space = PageGetFreeSpace(BufferGetPage(buffer)); - - UnlockReleaseBuffer(buffer); - - /* - * Since FSM is not WAL-logged and only updated heuristically, it - * easily becomes stale in standbys. If the standby is later promoted - * and runs VACUUM, it will skip updating individual free space - * figures for pages that became all-visible (or all-frozen, depending - * on the vacuum mode,) which is troublesome when FreeSpaceMapVacuum - * propagates too optimistic free space values to upper FSM layers; - * later inserters try to use such pages only to find out that they - * are unusable. This can cause long stalls when there are many such - * pages. - * - * Forestall those problems by updating FSM's idea about a page that - * is becoming all-visible or all-frozen. - * - * Do this regardless of a full-page image being applied, since the - * FSM data is not in the page anyway. - */ - if (xlrec->flags & VISIBILITYMAP_VALID_BITS) - XLogRecordPageWithFreeSpace(rlocator, blkno, space); - } - - /* - * Even if we skipped the heap page update due to the LSN interlock, it's - * still safe to update the visibility map. Any WAL record that clears - * the visibility map bit does so before checking the page LSN, so any - * bits that need to be cleared will still be cleared. + * Even if the heap relation was dropped or truncated and the previously + * emitted record skipped the heap page update due to this LSN interlock, + * it's still safe to update the visibility map. Any WAL record that + * clears the visibility map bit does so before checking the page LSN, so + * any bits that need to be cleared will still be cleared. */ if (XLogReadBufferForRedoExtended(record, 0, RBM_ZERO_ON_ERROR, false, &vmbuffer) == BLK_NEEDS_REDO) @@ -341,7 +279,7 @@ heap_xlog_visible(XLogReaderState *record) reln = CreateFakeRelcacheEntry(rlocator); - visibilitymap_set(reln, blkno, InvalidBuffer, lsn, vmbuffer, + visibilitymap_set(reln, blkno, lsn, vmbuffer, xlrec->snapshotConflictHorizon, vmbits); ReleaseBuffer(vmbuffer); diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c index c016f8f7c25..735f1e7501e 100644 --- a/src/backend/access/heap/vacuumlazy.c +++ b/src/backend/access/heap/vacuumlazy.c @@ -1911,7 +1911,7 @@ lazy_scan_new_or_empty(LVRelState *vacrel, Buffer buf, BlockNumber blkno, NULL, 0); } - visibilitymap_set(vacrel->rel, blkno, buf, + visibilitymap_set(vacrel->rel, blkno, InvalidXLogRecPtr, vmbuffer, InvalidTransactionId, VISIBILITYMAP_ALL_VISIBLE | @@ -2100,7 +2100,7 @@ lazy_scan_prune(LVRelState *vacrel, flags |= VISIBILITYMAP_ALL_FROZEN; } - old_vmbits = visibilitymap_set(vacrel->rel, blkno, buf, + old_vmbits = visibilitymap_set(vacrel->rel, blkno, InvalidXLogRecPtr, vmbuffer, presult.vm_conflict_horizon, flags); @@ -2898,7 +2898,7 @@ lazy_vacuum_heap_page(LVRelState *vacrel, BlockNumber blkno, Buffer buffer, */ if ((vmflags & VISIBILITYMAP_ALL_VISIBLE) != 0) { - visibilitymap_set(vacrel->rel, blkno, buffer, + visibilitymap_set(vacrel->rel, blkno, InvalidXLogRecPtr, vmbuffer, visibility_cutoff_xid, vmflags); diff --git a/src/backend/access/heap/visibilitymap.c b/src/backend/access/heap/visibilitymap.c index b28460392b7..33541e36674 100644 --- a/src/backend/access/heap/visibilitymap.c +++ b/src/backend/access/heap/visibilitymap.c @@ -233,9 +233,7 @@ visibilitymap_pin_ok(BlockNumber heapBlk, Buffer vmbuf) * when a page that is already all-visible is being marked all-frozen. * * Caller is expected to set the heap page's PD_ALL_VISIBLE bit before calling - * this function. Except in recovery, caller should also pass the heap - * buffer. When checksums are enabled and we're not in recovery, we must add - * the heap buffer to the WAL chain to protect it from being torn. + * this function. * * You must pass a buffer containing the correct map page to this function. * Call visibilitymap_pin first to pin the right one. This function doesn't do @@ -244,7 +242,7 @@ visibilitymap_pin_ok(BlockNumber heapBlk, Buffer vmbuf) * Returns the state of the page's VM bits before setting flags. */ uint8 -visibilitymap_set(Relation rel, BlockNumber heapBlk, Buffer heapBuf, +visibilitymap_set(Relation rel, BlockNumber heapBlk, XLogRecPtr recptr, Buffer vmBuf, TransactionId cutoff_xid, uint8 flags) { @@ -261,18 +259,11 @@ visibilitymap_set(Relation rel, BlockNumber heapBlk, Buffer heapBuf, #endif Assert(InRecovery || XLogRecPtrIsInvalid(recptr)); - Assert(InRecovery || PageIsAllVisible(BufferGetPage(heapBuf))); Assert((flags & VISIBILITYMAP_VALID_BITS) == flags); /* Must never set all_frozen bit without also setting all_visible bit */ Assert(flags != VISIBILITYMAP_ALL_FROZEN); - /* Check that we have the right heap page pinned, if present */ - if (BufferIsValid(heapBuf) && BufferGetBlockNumber(heapBuf) != heapBlk) - elog(ERROR, "wrong heap buffer passed to visibilitymap_set"); - - Assert(!BufferIsValid(heapBuf) || BufferIsExclusiveLocked(heapBuf)); - /* Check that we have the right VM page pinned */ if (!BufferIsValid(vmBuf) || BufferGetBlockNumber(vmBuf) != mapBlock) elog(ERROR, "wrong VM buffer passed to visibilitymap_set"); @@ -294,23 +285,7 @@ visibilitymap_set(Relation rel, BlockNumber heapBlk, Buffer heapBuf, if (XLogRecPtrIsInvalid(recptr)) { Assert(!InRecovery); - recptr = log_heap_visible(rel, heapBuf, vmBuf, cutoff_xid, flags); - - /* - * If data checksums are enabled (or wal_log_hints=on), we - * need to protect the heap page from being torn. - * - * If not, then we must *not* update the heap page's LSN. In - * this case, the FPI for the heap page was omitted from the - * WAL record inserted above, so it would be incorrect to - * update the heap page's LSN. - */ - if (XLogHintBitIsNeeded()) - { - Page heapPage = BufferGetPage(heapBuf); - - PageSetLSN(heapPage, recptr); - } + recptr = log_heap_visible(rel, vmBuf, cutoff_xid, flags); } PageSetLSN(page, recptr); } diff --git a/src/include/access/heapam_xlog.h b/src/include/access/heapam_xlog.h index 7d3fb75dda7..82b8f7f2bbc 100644 --- a/src/include/access/heapam_xlog.h +++ b/src/include/access/heapam_xlog.h @@ -440,7 +440,6 @@ typedef struct xl_heap_inplace * This is what we need to know about setting a visibility map bit * * Backup blk 0: visibility map buffer - * Backup blk 1: heap buffer */ typedef struct xl_heap_visible { @@ -493,7 +492,7 @@ extern void heap2_desc(StringInfo buf, XLogReaderState *record); extern const char *heap2_identify(uint8 info); extern void heap_xlog_logical_rewrite(XLogReaderState *r); -extern XLogRecPtr log_heap_visible(Relation rel, Buffer heap_buffer, +extern XLogRecPtr log_heap_visible(Relation rel, Buffer vm_buffer, TransactionId snapshotConflictHorizon, uint8 vmflags); diff --git a/src/include/access/visibilitymap.h b/src/include/access/visibilitymap.h index 3dcf37ba03f..fbc69604d57 100644 --- a/src/include/access/visibilitymap.h +++ b/src/include/access/visibilitymap.h @@ -32,7 +32,7 @@ extern void visibilitymap_pin(Relation rel, BlockNumber heapBlk, Buffer *vmbuf); extern bool visibilitymap_pin_ok(BlockNumber heapBlk, Buffer vmbuf); extern uint8 visibilitymap_set(Relation rel, - BlockNumber heapBlk, Buffer heapBuf, + BlockNumber heapBlk, XLogRecPtr recptr, Buffer vmBuf, TransactionId cutoff_xid, -- 2.43.0