From 121ba45c82d9a3798243d65f9838b1875484cb49 Mon Sep 17 00:00:00 2001 From: Melanie Plageman Date: Wed, 18 Jun 2025 12:30:42 -0400 Subject: [PATCH v3 11/13] Remove xl_heap_visible entirely There are now no users of this, so eliminate it entirely. --- src/backend/access/common/bufmask.c | 3 +- src/backend/access/heap/heapam.c | 54 +------- src/backend/access/heap/heapam_xlog.c | 150 +---------------------- src/backend/access/heap/pruneheap.c | 4 +- src/backend/access/heap/vacuumlazy.c | 10 +- src/backend/access/heap/visibilitymap.c | 103 +--------------- src/backend/access/rmgrdesc/heapdesc.c | 10 -- src/backend/replication/logical/decode.c | 1 - src/include/access/heapam_xlog.h | 6 - src/include/access/visibilitymap.h | 10 +- 10 files changed, 23 insertions(+), 328 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 68db4325285..48f7b84156a 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" @@ -2512,11 +2513,11 @@ heap_multi_insert(Relation relation, TupleTableSlot **slots, int ntuples, { PageSetAllVisible(page); LockBuffer(vmbuffer, BUFFER_LOCK_EXCLUSIVE); - visibilitymap_set_vmbyte(relation, - BufferGetBlockNumber(buffer), - vmbuffer, - VISIBILITYMAP_ALL_VISIBLE | - VISIBILITYMAP_ALL_FROZEN); + visibilitymap_set(relation, + BufferGetBlockNumber(buffer), + vmbuffer, + VISIBILITYMAP_ALL_VISIBLE | + VISIBILITYMAP_ALL_FROZEN); } /* @@ -8784,49 +8785,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 14541e2e94f..51c158af89e 100644 --- a/src/backend/access/heap/heapam_xlog.c +++ b/src/backend/access/heap/heapam_xlog.c @@ -267,7 +267,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); @@ -278,143 +278,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. @@ -791,16 +654,16 @@ 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); /* * It is not possible that the VM was already set for this heap page, * so the vmbuffer must have been modified and marked dirty. */ Assert(BufferIsDirty(vmbuffer)); + 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/pruneheap.c b/src/backend/access/heap/pruneheap.c index 6208f55176f..f6509695e3a 100644 --- a/src/backend/access/heap/pruneheap.c +++ b/src/backend/access/heap/pruneheap.c @@ -959,8 +959,8 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer, if (do_set_vm) { LockBuffer(vmbuffer, BUFFER_LOCK_EXCLUSIVE); - old_vmbits = visibilitymap_set_vmbyte(relation, blockno, - vmbuffer, vmflags); + old_vmbits = visibilitymap_set(relation, blockno, + vmbuffer, vmflags); if (old_vmbits == vmflags) { diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c index 91e209901b8..6a0fa371a06 100644 --- a/src/backend/access/heap/vacuumlazy.c +++ b/src/backend/access/heap/vacuumlazy.c @@ -1887,8 +1887,8 @@ lazy_scan_new_or_empty(LVRelState *vacrel, Buffer buf, BlockNumber blkno, MarkBufferDirty(buf); LockBuffer(vmbuffer, BUFFER_LOCK_EXCLUSIVE); - visibilitymap_set_vmbyte(vacrel->rel, blkno, - vmbuffer, new_vmbits); + visibilitymap_set(vacrel->rel, blkno, + vmbuffer, new_vmbits); if (RelationNeedsWAL(vacrel->rel)) { @@ -2754,9 +2754,9 @@ lazy_vacuum_heap_page(LVRelState *vacrel, BlockNumber blkno, Buffer buffer, set_pd_all_vis = true; LockBuffer(vmbuffer, BUFFER_LOCK_EXCLUSIVE); PageSetAllVisible(page); - visibilitymap_set_vmbyte(vacrel->rel, - blkno, - vmbuffer, vmflags); + visibilitymap_set(vacrel->rel, + blkno, + vmbuffer, vmflags); conflict_xid = visibility_cutoff_xid; } diff --git a/src/backend/access/heap/visibilitymap.c b/src/backend/access/heap/visibilitymap.c index 573df6f6891..478b08fa520 100644 --- a/src/backend/access/heap/visibilitymap.c +++ b/src/backend/access/heap/visibilitymap.c @@ -219,105 +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 - * - * recptr is the LSN of the XLOG record we're replaying, if we're in recovery, - * or InvalidXLogRecPtr in normal running. The VM page LSN is advanced to the - * one provided; in normal running, we generate a new XLOG record and set the - * page LSN to that value (though the heap page's LSN may *not* be updated; - * see below). 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. - * - * 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. - * - * Returns the state of the page's VM bits before setting flags. - */ -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 heap page pinned, if present */ - if (BufferIsValid(heapBuf) && BufferGetBlockNumber(heapBuf) != heapBlk) - elog(ERROR, "wrong heap buffer passed to visibilitymap_set"); - - /* 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); - - /* - * 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); - } - } - 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. * @@ -337,8 +238,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_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 977566f6b98..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.43.0