From a1bbff2e42b771bbd8a4b8e2b0719e4582bfcf1f Mon Sep 17 00:00:00 2001 From: Melanie Plageman Date: Wed, 18 Jun 2025 12:30:42 -0400 Subject: [PATCH v2 12/12] Remove xl_heap_visible entirely There are now no users of this, so eliminate it entirely. ci-os-only: --- src/backend/access/common/bufmask.c | 3 +- src/backend/access/heap/heapam.c | 113 +---------------- src/backend/access/heap/heapam_xlog.c | 150 +---------------------- src/backend/access/heap/vacuumlazy.c | 4 +- src/backend/access/heap/visibilitymap.c | 84 +------------ src/backend/access/rmgrdesc/heapdesc.c | 10 -- src/backend/replication/logical/decode.c | 1 - src/include/access/heapam.h | 3 - src/include/access/heapam_xlog.h | 6 - src/include/access/visibilitymap.h | 10 +- 10 files changed, 14 insertions(+), 370 deletions(-) diff --git a/src/backend/access/common/bufmask.c b/src/backend/access/common/bufmask.c index bb260cffa68..1fff01383b3 100644 --- a/src/backend/access/common/bufmask.c +++ b/src/backend/access/common/bufmask.c @@ -56,8 +56,7 @@ mask_page_hint_bits(Page page) /* * During replay, if the page LSN has advanced past our XLOG record's LSN, - * we don't mark the page all-visible. See heap_xlog_visible() for - * details. + * we don't mark the page all-visible. */ PageClearAllVisible(page); } diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c index d2cf8aa9fb8..6f134dfd535 100644 --- a/src/backend/access/heap/heapam.c +++ b/src/backend/access/heap/heapam.c @@ -40,6 +40,7 @@ #include "access/valid.h" #include "access/visibilitymap.h" #include "access/xloginsert.h" +#include "access/xlogutils.h" #include "catalog/pg_database.h" #include "catalog/pg_database_d.h" #include "commands/vacuum.h" @@ -7845,73 +7846,6 @@ heap_tuple_needs_eventual_freeze(HeapTupleHeader tuple) return false; } -/* - * Make the heap and VM page changes needed to set a page all-visible. - * Do not call in recovery. - */ -uint8 -heap_page_set_vm_and_log(Relation rel, BlockNumber heap_blk, Buffer heap_buf, - Buffer vmbuf, TransactionId cutoff_xid, - uint8 vmflags) -{ - Page heap_page = BufferGetPage(heap_buf); - bool set_heap_lsn = false; - XLogRecPtr recptr = InvalidXLogRecPtr; - uint8 old_vmbits = 0; - - Assert(BufferIsValid(heap_buf)); - - START_CRIT_SECTION(); - - /* Check that we have the right heap page pinned, if present */ - if (BufferGetBlockNumber(heap_buf) != heap_blk) - elog(ERROR, "wrong heap buffer passed to heap_page_set_vm_and_log"); - - /* - * We must never end up with the VM bit set and the page-level - * PD_ALL_VISIBLE bit clear. If that were to occur, a subsequent page - * modification would fail to clear the VM bit. Though it is possible for - * the page-level bit to be set and the VM bit to be clear if checksums - * and wal_log_hints are not enabled. - */ - if (!PageIsAllVisible(heap_page)) - { - PageSetAllVisible(heap_page); - - /* - * Buffer will usually be dirty from other changes, so it is worth the - * extra check - */ - if (!BufferIsDirty(heap_buf)) - { - if (XLogHintBitIsNeeded()) - MarkBufferDirty(heap_buf); - else - MarkBufferDirtyHint(heap_buf, true); - } - - set_heap_lsn = XLogHintBitIsNeeded(); - } - - old_vmbits = visibilitymap_set(rel, heap_blk, heap_buf, - &recptr, vmbuf, cutoff_xid, vmflags); - - /* - * If we modified the heap page and 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 in - * the VM record, so it would be incorrect to update the heap page's LSN. - */ - if (set_heap_lsn) - PageSetLSN(heap_page, recptr); - - END_CRIT_SECTION(); - - return old_vmbits; -} - /* * Ensure the provided heap page is marked PD_ALL_VISIBLE and then set the * provided vmflags in the provided vmbuf. @@ -7953,7 +7887,7 @@ heap_page_set_vm(Relation rel, BlockNumber heap_blk, Buffer heap_buf, MarkBufferDirty(heap_buf); } - return visibilitymap_set_vmbyte(rel, heap_blk, vmbuf, vmflags); + return visibilitymap_set(rel, heap_blk, vmbuf, vmflags); } /* @@ -8895,49 +8829,6 @@ bottomup_sort_and_shrink(TM_IndexDeleteOp *delstate) return nblocksfavorable; } -/* - * Perform XLogInsert for a heap-visible operation. 'block' is the block - * being marked all-visible, and vm_buffer is the buffer containing the - * corresponding visibility map block. Both should have already been modified - * and dirtied. - * - * 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, - TransactionId snapshotConflictHorizon, uint8 vmflags) -{ - xl_heap_visible xlrec; - XLogRecPtr recptr; - uint8 flags; - - Assert(BufferIsValid(heap_buffer)); - Assert(BufferIsValid(vm_buffer)); - - xlrec.snapshotConflictHorizon = snapshotConflictHorizon; - xlrec.flags = vmflags; - if (RelationIsAccessibleInLogicalDecoding(rel)) - 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; -} /* * Perform XLogInsert for a heap-update operation. Caller must already diff --git a/src/backend/access/heap/heapam_xlog.c b/src/backend/access/heap/heapam_xlog.c index 70a46a37357..975a59d717e 100644 --- a/src/backend/access/heap/heapam_xlog.c +++ b/src/backend/access/heap/heapam_xlog.c @@ -273,7 +273,7 @@ heap_xlog_prune_freeze(XLogReaderState *record) Relation reln = CreateFakeRelcacheEntry(rlocator); visibilitymap_pin(reln, blkno, &vmbuffer); - old_vmbits = visibilitymap_set_vmbyte(reln, blkno, vmbuffer, vmflags); + old_vmbits = visibilitymap_set(reln, blkno, vmbuffer, vmflags); /* Only set VM page LSN if we modified the page */ if (old_vmbits != vmflags) PageSetLSN(BufferGetPage(vmbuffer), lsn); @@ -284,143 +284,6 @@ heap_xlog_prune_freeze(XLogReaderState *record) UnlockReleaseBuffer(vmbuffer); } -/* - * Replay XLOG_HEAP2_VISIBLE records. - * - * The critical integrity requirement here is that we must never end up with - * a situation where the visibility map bit is set, and the page-level - * PD_ALL_VISIBLE bit is clear. If that were to occur, then a subsequent - * page modification would fail to clear the visibility map bit. - */ -static void -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); - - /* - * If there are any Hot Standby transactions running that have an xmin - * horizon old enough that this page isn't all-visible for them, they - * might incorrectly decide that an index-only scan can skip a heap fetch. - * - * NB: It might be better to throw some kind of "soft" conflict here that - * forces any index-only scan that is in flight to perform heap fetches, - * rather than killing the transaction outright. - */ - if (InHotStandby) - ResolveRecoveryConflictWithSnapshot(xlrec->snapshotConflictHorizon, - xlrec->flags & VISIBILITYMAP_XLOG_CATALOG_REL, - 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. - */ - if (XLogReadBufferForRedoExtended(record, 0, RBM_ZERO_ON_ERROR, false, - &vmbuffer) == BLK_NEEDS_REDO) - { - Page vmpage = BufferGetPage(vmbuffer); - Relation reln; - uint8 vmbits; - - /* initialize the page if it was read as zeros */ - if (PageIsNew(vmpage)) - PageInit(vmpage, BLCKSZ, 0); - - /* remove VISIBILITYMAP_XLOG_* */ - vmbits = xlrec->flags & VISIBILITYMAP_VALID_BITS; - - /* - * XLogReadBufferForRedoExtended locked the buffer. But - * visibilitymap_set will handle locking itself. - */ - LockBuffer(vmbuffer, BUFFER_LOCK_UNLOCK); - - reln = CreateFakeRelcacheEntry(rlocator); - visibilitymap_pin(reln, blkno, &vmbuffer); - - visibilitymap_set(reln, blkno, InvalidBuffer, &lsn, vmbuffer, - xlrec->snapshotConflictHorizon, vmbits); - - ReleaseBuffer(vmbuffer); - FreeFakeRelcacheEntry(reln); - } - else if (BufferIsValid(vmbuffer)) - UnlockReleaseBuffer(vmbuffer); -} - /* * Given an "infobits" field from an XLog record, set the correct bits in the * given infomask and infomask2 for the tuple touched by the record. @@ -797,10 +660,10 @@ heap_xlog_multi_insert(XLogReaderState *record) Relation reln = CreateFakeRelcacheEntry(rlocator); visibilitymap_pin(reln, blkno, &vmbuffer); - visibilitymap_set_vmbyte(reln, blkno, - vmbuffer, - VISIBILITYMAP_ALL_VISIBLE | - VISIBILITYMAP_ALL_FROZEN); + visibilitymap_set(reln, blkno, + vmbuffer, + VISIBILITYMAP_ALL_VISIBLE | + VISIBILITYMAP_ALL_FROZEN); PageSetLSN(BufferGetPage(vmbuffer), lsn); FreeFakeRelcacheEntry(reln); } @@ -1380,9 +1243,6 @@ heap2_redo(XLogReaderState *record) case XLOG_HEAP2_PRUNE_VACUUM_CLEANUP: heap_xlog_prune_freeze(record); break; - case XLOG_HEAP2_VISIBLE: - heap_xlog_visible(record); - break; case XLOG_HEAP2_MULTI_INSERT: heap_xlog_multi_insert(record); break; diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c index 246ba07db9c..9371d6f37c1 100644 --- a/src/backend/access/heap/vacuumlazy.c +++ b/src/backend/access/heap/vacuumlazy.c @@ -1878,14 +1878,14 @@ lazy_scan_new_or_empty(LVRelState *vacrel, Buffer buf, BlockNumber blkno, */ if (!PageIsAllVisible(page)) { - uint8 new_vmbits = VISIBILITYMAP_ALL_VISIBLE | + uint8 new_vmbits = VISIBILITYMAP_ALL_VISIBLE | VISIBILITYMAP_ALL_FROZEN; START_CRIT_SECTION(); LockBuffer(vmbuffer, BUFFER_LOCK_EXCLUSIVE); heap_page_set_vm(vacrel->rel, blkno, buf, - vmbuffer, new_vmbits); + vmbuffer, new_vmbits); /* Should have set PD_ALL_VISIBLE and marked buf dirty */ Assert(BufferIsDirty(buf)); diff --git a/src/backend/access/heap/visibilitymap.c b/src/backend/access/heap/visibilitymap.c index cabd0fa0880..a24554fe191 100644 --- a/src/backend/access/heap/visibilitymap.c +++ b/src/backend/access/heap/visibilitymap.c @@ -219,86 +219,6 @@ visibilitymap_pin_ok(BlockNumber heapBlk, Buffer vmbuf) return BufferIsValid(vmbuf) && BufferGetBlockNumber(vmbuf) == mapBlock; } -/* - * visibilitymap_set - set bit(s) on a previously pinned page - * - * 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. - * - * 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 - * any I/O. - * - * cutoff_xid is the largest xmin on the page being marked all-visible; it is - * needed for Hot Standby, and can be InvalidTransactionId if the page - * contains no tuples. It can also be set to InvalidTransactionId when a page - * that is already all-visible is being marked all-frozen. - * - * If we're in recovery, recptr points to the LSN of the XLOG record we're - * replaying and the VM page LSN is advanced to this LSN. During normal - * running, we'll generate a new XLOG record for the changes to the VM and set - * the VM page LSN. We will return this LSN in recptr, and the caller may use - * this to set the heap page LSN. - * - * Returns the state of the page's VM bits before setting flags and sets. - */ -uint8 -visibilitymap_set(Relation rel, BlockNumber heapBlk, Buffer heapBuf, - XLogRecPtr *recptr, Buffer vmBuf, TransactionId cutoff_xid, - uint8 flags) -{ - BlockNumber mapBlock = HEAPBLK_TO_MAPBLOCK(heapBlk); - uint32 mapByte = HEAPBLK_TO_MAPBYTE(heapBlk); - uint8 mapOffset = HEAPBLK_TO_OFFSET(heapBlk); - Page page; - uint8 *map; - uint8 status; - -#ifdef TRACE_VISIBILITYMAP - elog(DEBUG1, "vm_set %s %d", RelationGetRelationName(rel), heapBlk); -#endif - - Assert(InRecovery || XLogRecPtrIsInvalid(*recptr)); - Assert(InRecovery || PageIsAllVisible((Page) 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 VM page pinned */ - if (!BufferIsValid(vmBuf) || BufferGetBlockNumber(vmBuf) != mapBlock) - elog(ERROR, "wrong VM buffer passed to visibilitymap_set"); - - page = BufferGetPage(vmBuf); - map = (uint8 *) PageGetContents(page); - LockBuffer(vmBuf, BUFFER_LOCK_EXCLUSIVE); - - status = (map[mapByte] >> mapOffset) & VISIBILITYMAP_VALID_BITS; - if (flags != status) - { - START_CRIT_SECTION(); - - map[mapByte] |= (flags << mapOffset); - MarkBufferDirty(vmBuf); - - if (RelationNeedsWAL(rel)) - { - if (XLogRecPtrIsInvalid(*recptr)) - { - Assert(!InRecovery); - *recptr = log_heap_visible(rel, heapBuf, vmBuf, cutoff_xid, flags); - } - PageSetLSN(page, *recptr); - } - - END_CRIT_SECTION(); - } - - LockBuffer(vmBuf, BUFFER_LOCK_UNLOCK); - return status; -} /* * Set flags in the VM block contained in the passed in vmBuf. @@ -308,8 +228,8 @@ visibilitymap_set(Relation rel, BlockNumber heapBlk, Buffer heapBuf, * making any changes needed to the associated heap page. */ uint8 -visibilitymap_set_vmbyte(Relation rel, BlockNumber heapBlk, - Buffer vmBuf, uint8 flags) +visibilitymap_set(Relation rel, BlockNumber heapBlk, + Buffer vmBuf, uint8 flags) { BlockNumber mapBlock = HEAPBLK_TO_MAPBLOCK(heapBlk); uint32 mapByte = HEAPBLK_TO_MAPBYTE(heapBlk); diff --git a/src/backend/access/rmgrdesc/heapdesc.c b/src/backend/access/rmgrdesc/heapdesc.c index d6c86ccac20..f7880a4ed81 100644 --- a/src/backend/access/rmgrdesc/heapdesc.c +++ b/src/backend/access/rmgrdesc/heapdesc.c @@ -351,13 +351,6 @@ heap2_desc(StringInfo buf, XLogReaderState *record) } } } - else if (info == XLOG_HEAP2_VISIBLE) - { - xl_heap_visible *xlrec = (xl_heap_visible *) rec; - - appendStringInfo(buf, "snapshotConflictHorizon: %u, flags: 0x%02X", - xlrec->snapshotConflictHorizon, xlrec->flags); - } else if (info == XLOG_HEAP2_MULTI_INSERT) { xl_heap_multi_insert *xlrec = (xl_heap_multi_insert *) rec; @@ -462,9 +455,6 @@ heap2_identify(uint8 info) case XLOG_HEAP2_PRUNE_VACUUM_CLEANUP: id = "PRUNE_VACUUM_CLEANUP"; break; - case XLOG_HEAP2_VISIBLE: - id = "VISIBLE"; - break; case XLOG_HEAP2_MULTI_INSERT: id = "MULTI_INSERT"; break; diff --git a/src/backend/replication/logical/decode.c b/src/backend/replication/logical/decode.c index cc03f0706e9..2fdd4af90a8 100644 --- a/src/backend/replication/logical/decode.c +++ b/src/backend/replication/logical/decode.c @@ -454,7 +454,6 @@ heap2_decode(LogicalDecodingContext *ctx, XLogRecordBuffer *buf) case XLOG_HEAP2_PRUNE_ON_ACCESS: case XLOG_HEAP2_PRUNE_VACUUM_SCAN: case XLOG_HEAP2_PRUNE_VACUUM_CLEANUP: - case XLOG_HEAP2_VISIBLE: case XLOG_HEAP2_LOCK_UPDATED: break; default: diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h index e35b4adf38d..c404b794fda 100644 --- a/src/include/access/heapam.h +++ b/src/include/access/heapam.h @@ -365,9 +365,6 @@ extern bool heap_tuple_needs_eventual_freeze(HeapTupleHeader tuple); extern uint8 heap_page_set_vm(Relation rel, BlockNumber heap_blk, Buffer heap_buf, Buffer vmbuf, uint8 vmflags); -extern uint8 heap_page_set_vm_and_log(Relation rel, BlockNumber heap_blk, Buffer heap_buf, - Buffer vmbuf, TransactionId cutoff_xid, - uint8 vmflags); extern void simple_heap_insert(Relation relation, HeapTuple tup); extern void simple_heap_delete(Relation relation, ItemPointer tid); diff --git a/src/include/access/heapam_xlog.h b/src/include/access/heapam_xlog.h index ceae9c083ff..9a61434b881 100644 --- a/src/include/access/heapam_xlog.h +++ b/src/include/access/heapam_xlog.h @@ -60,7 +60,6 @@ #define XLOG_HEAP2_PRUNE_ON_ACCESS 0x10 #define XLOG_HEAP2_PRUNE_VACUUM_SCAN 0x20 #define XLOG_HEAP2_PRUNE_VACUUM_CLEANUP 0x30 -#define XLOG_HEAP2_VISIBLE 0x40 #define XLOG_HEAP2_MULTI_INSERT 0x50 #define XLOG_HEAP2_LOCK_UPDATED 0x60 #define XLOG_HEAP2_NEW_CID 0x70 @@ -495,11 +494,6 @@ 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, - Buffer vm_buffer, - TransactionId snapshotConflictHorizon, - uint8 vmflags); - /* in heapdesc.c, so it can be shared between frontend/backend code */ extern void heap_xlog_deserialize_prune_and_freeze(char *cursor, uint8 flags, int *nplans, xlhp_freeze_plan **plans, diff --git a/src/include/access/visibilitymap.h b/src/include/access/visibilitymap.h index 91ef3705e84..20141e3e805 100644 --- a/src/include/access/visibilitymap.h +++ b/src/include/access/visibilitymap.h @@ -31,14 +31,8 @@ extern bool visibilitymap_clear(Relation rel, BlockNumber heapBlk, 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, - XLogRecPtr *recptr, - Buffer vmBuf, - TransactionId cutoff_xid, - uint8 flags); -extern uint8 visibilitymap_set_vmbyte(Relation rel, BlockNumber heapBlk, - Buffer vmBuf, uint8 flags); +extern uint8 visibilitymap_set(Relation rel, BlockNumber heapBlk, + Buffer vmBuf, uint8 flags); extern uint8 visibilitymap_get_status(Relation rel, BlockNumber heapBlk, Buffer *vmbuf); extern void visibilitymap_count(Relation rel, BlockNumber *all_visible, BlockNumber *all_frozen); extern BlockNumber visibilitymap_prepare_truncate(Relation rel, -- 2.34.1