From 266450693f4df295a257b8316b285b8cfb25761a Mon Sep 17 00:00:00 2001 From: Melanie Plageman Date: Tue, 17 Jun 2025 17:22:10 -0400 Subject: [PATCH v7 03/22] Eliminate xl_heap_visible in COPY FREEZE Instead of emitting a separate WAL record for setting the VM bits in xl_heap_visible, specify the changes to make to the VM block in the xl_heap_multi_insert record instead. Author: Melanie Plageman Reviewed-by: Kirill Reshke Discussion: https://postgr.es/m/flat/CAAKRu_ZMw6Npd_qm2KM%2BFwQ3cMOMx1Dh3VMhp8-V7SOLxdK9-g%40mail.gmail.com --- src/backend/access/heap/heapam.c | 48 ++++++++++-------- src/backend/access/heap/heapam_xlog.c | 39 +++++++++++++- src/backend/access/heap/visibilitymap.c | 67 ++++++++++++++++++++++++- src/backend/access/rmgrdesc/heapdesc.c | 5 ++ src/include/access/visibilitymap.h | 2 + 5 files changed, 138 insertions(+), 23 deletions(-) diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c index 7491cc3cb93..4ce0ec61692 100644 --- a/src/backend/access/heap/heapam.c +++ b/src/backend/access/heap/heapam.c @@ -2504,9 +2504,6 @@ heap_multi_insert(Relation relation, TupleTableSlot **slots, int ntuples, /* * If the page is all visible, need to clear that, unless we're only * going to add further frozen rows to it. - * - * If we're only adding already frozen rows to a previously empty - * page, mark it as all-visible. */ if (PageIsAllVisible(page) && !(options & HEAP_INSERT_FROZEN)) { @@ -2516,8 +2513,23 @@ heap_multi_insert(Relation relation, TupleTableSlot **slots, int ntuples, BufferGetBlockNumber(buffer), vmbuffer, VISIBILITYMAP_VALID_BITS); } + + /* + * If we're only adding already frozen rows to a previously empty + * page, mark it as all-frozen and update the visibility map. We're + * already holding a pin on the vmbuffer. + */ else if (all_frozen_set) + { + Assert(visibilitymap_pin_ok(BufferGetBlockNumber(buffer), vmbuffer)); PageSetAllVisible(page); + LockBuffer(vmbuffer, BUFFER_LOCK_EXCLUSIVE); + visibilitymap_set_vmbyte(relation, + BufferGetBlockNumber(buffer), + vmbuffer, + VISIBILITYMAP_ALL_VISIBLE | + VISIBILITYMAP_ALL_FROZEN); + } /* * XXX Should we set PageSetPrunable on this page ? See heap_insert() @@ -2565,6 +2577,12 @@ heap_multi_insert(Relation relation, TupleTableSlot **slots, int ntuples, xlrec->flags = 0; if (all_visible_cleared) xlrec->flags = XLH_INSERT_ALL_VISIBLE_CLEARED; + + /* + * We don't have to worry about including a conflict xid in the + * WAL record as HEAP_INSERT_FROZEN intentionally violates + * visibility rules. + */ if (all_frozen_set) xlrec->flags = XLH_INSERT_ALL_FROZEN_SET; @@ -2627,7 +2645,10 @@ heap_multi_insert(Relation relation, TupleTableSlot **slots, int ntuples, XLogBeginInsert(); XLogRegisterData(xlrec, tupledata - scratch.data); + XLogRegisterBuffer(0, buffer, REGBUF_STANDARD | bufflags); + if (all_frozen_set) + XLogRegisterBuffer(1, vmbuffer, 0); XLogRegisterBufData(0, tupledata, totaldatalen); @@ -2637,29 +2658,14 @@ heap_multi_insert(Relation relation, TupleTableSlot **slots, int ntuples, recptr = XLogInsert(RM_HEAP2_ID, info); PageSetLSN(page, recptr); + if (all_frozen_set) + PageSetLSN(BufferGetPage(vmbuffer), recptr); } END_CRIT_SECTION(); - /* - * If we've frozen everything on the page, update the visibilitymap. - * We're already holding pin on the vmbuffer. - */ if (all_frozen_set) - { - Assert(PageIsAllVisible(page)); - Assert(visibilitymap_pin_ok(BufferGetBlockNumber(buffer), vmbuffer)); - - /* - * It's fine to use InvalidTransactionId here - this is only used - * when HEAP_INSERT_FROZEN is specified, which intentionally - * violates visibility rules. - */ - visibilitymap_set(relation, BufferGetBlockNumber(buffer), buffer, - InvalidXLogRecPtr, vmbuffer, - InvalidTransactionId, - VISIBILITYMAP_ALL_VISIBLE | VISIBILITYMAP_ALL_FROZEN); - } + LockBuffer(vmbuffer, BUFFER_LOCK_UNLOCK); UnlockReleaseBuffer(buffer); ndone += nthispage; diff --git a/src/backend/access/heap/heapam_xlog.c b/src/backend/access/heap/heapam_xlog.c index e3e021f2bdd..cf437b14eeb 100644 --- a/src/backend/access/heap/heapam_xlog.c +++ b/src/backend/access/heap/heapam_xlog.c @@ -552,6 +552,7 @@ heap_xlog_multi_insert(XLogReaderState *record) int i; bool isinit = (XLogRecGetInfo(record) & XLOG_HEAP_INIT_PAGE) != 0; XLogRedoAction action; + Buffer vmbuffer = InvalidBuffer; /* * Insertion doesn't overwrite MVCC data, so no conflict processing is @@ -572,11 +573,11 @@ heap_xlog_multi_insert(XLogReaderState *record) if (xlrec->flags & XLH_INSERT_ALL_VISIBLE_CLEARED) { Relation reln = CreateFakeRelcacheEntry(rlocator); - Buffer vmbuffer = InvalidBuffer; visibilitymap_pin(reln, blkno, &vmbuffer); visibilitymap_clear(reln, blkno, vmbuffer, VISIBILITYMAP_VALID_BITS); ReleaseBuffer(vmbuffer); + vmbuffer = InvalidBuffer; FreeFakeRelcacheEntry(reln); } @@ -663,6 +664,42 @@ heap_xlog_multi_insert(XLogReaderState *record) if (BufferIsValid(buffer)) UnlockReleaseBuffer(buffer); + buffer = InvalidBuffer; + + /* + * Now read and update the VM block. Even if we skipped updating the heap + * page due to the file being dropped or truncated later in recovery, 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. + * + * It is only okay to set the VM bits without holding the heap page lock + * because we can expect no other writers of this page. + */ + if (xlrec->flags & XLH_INSERT_ALL_FROZEN_SET && + XLogReadBufferForRedoExtended(record, 1, RBM_ZERO_ON_ERROR, false, + &vmbuffer) == BLK_NEEDS_REDO) + { + Relation reln = CreateFakeRelcacheEntry(rlocator); + + Assert(visibilitymap_pin_ok(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)); + PageSetLSN(BufferGetPage(vmbuffer), lsn); + FreeFakeRelcacheEntry(reln); + } + + if (BufferIsValid(vmbuffer)) + UnlockReleaseBuffer(vmbuffer); + /* * If the page is running low on free space, update the FSM as well. * Arbitrarily, our definition of "low" is less than 20%. We can't do much diff --git a/src/backend/access/heap/visibilitymap.c b/src/backend/access/heap/visibilitymap.c index 7440a65c404..568bc83db9c 100644 --- a/src/backend/access/heap/visibilitymap.c +++ b/src/backend/access/heap/visibilitymap.c @@ -14,7 +14,8 @@ * visibilitymap_clear - clear bits for one page in the visibility map * visibilitymap_pin - pin a map page for setting a bit * visibilitymap_pin_ok - check whether correct map page is already pinned - * visibilitymap_set - set a bit in a previously pinned page + * visibilitymap_set - set a bit in a previously pinned page and log + * visibilitymap_set_vmbyte - set a bit in a pinned page * visibilitymap_get_status - get status of bits * visibilitymap_count - count number of bits set in visibility map * visibilitymap_prepare_truncate - @@ -321,6 +322,70 @@ visibilitymap_set(Relation rel, BlockNumber heapBlk, Buffer heapBuf, return status; } +/* + * Set flags in the VM block contained in the passed in vmBuf. + * + * This function is for callers which include the VM changes in the same WAL + * record as the modifications of the heap page which rendered it all-visible. + * Callers separately logging the VM changes should invoke visibilitymap_set() + * instead. + * + * Caller must have pinned and exclusive locked the correct block of the VM in + * vmBuf. This block should contain the VM bits for the given heapBlk. + * + * During normal operation (i.e. not recovery), this should be called in a + * critical section which also makes any necessary changes to the heap page + * and, if relevant, emits WAL. + * + * Caller is responsible for WAL logging the changes to the VM buffer and for + * making any changes needed to the associated heap page. This includes + * maintaining any invariants such as ensuring the buffer containing heapBlk + * is pinned and exclusive locked. + */ +uint8 +visibilitymap_set_vmbyte(Relation rel, BlockNumber heapBlk, + Buffer vmBuf, 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 flags 0x%02X for %s %d", + flags, RelationGetRelationName(rel), heapBlk); +#endif + + /* Call in same critical section where WAL is emitted. */ + Assert(InRecovery || CritSectionCount > 0); + + /* Flags should be valid. Also never clear bits with this function */ + 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"); + + Assert(BufferIsExclusiveLocked(vmBuf)); + + page = BufferGetPage(vmBuf); + map = (uint8 *) PageGetContents(page); + + status = (map[mapByte] >> mapOffset) & VISIBILITYMAP_VALID_BITS; + if (flags != status) + { + map[mapByte] |= (flags << mapOffset); + MarkBufferDirty(vmBuf); + } + + return status; +} + /* * visibilitymap_get_status - get status of bits * diff --git a/src/backend/access/rmgrdesc/heapdesc.c b/src/backend/access/rmgrdesc/heapdesc.c index 82b62c95de5..b48d7dc1d24 100644 --- a/src/backend/access/rmgrdesc/heapdesc.c +++ b/src/backend/access/rmgrdesc/heapdesc.c @@ -16,6 +16,7 @@ #include "access/heapam_xlog.h" #include "access/rmgrdesc_utils.h" +#include "access/visibilitymapdefs.h" #include "storage/standbydefs.h" /* @@ -354,6 +355,10 @@ heap2_desc(StringInfo buf, XLogReaderState *record) appendStringInfo(buf, "ntuples: %d, flags: 0x%02X", xlrec->ntuples, xlrec->flags); + if (xlrec->flags & XLH_INSERT_ALL_FROZEN_SET) + appendStringInfo(buf, ", vm_flags: 0x%02X", + VISIBILITYMAP_ALL_VISIBLE | VISIBILITYMAP_ALL_FROZEN); + if (XLogRecHasBlockData(record, 0) && !isinit) { appendStringInfoString(buf, ", offsets:"); diff --git a/src/include/access/visibilitymap.h b/src/include/access/visibilitymap.h index be21c6dd1a3..977566f6b98 100644 --- a/src/include/access/visibilitymap.h +++ b/src/include/access/visibilitymap.h @@ -37,6 +37,8 @@ extern uint8 visibilitymap_set(Relation rel, Buffer vmBuf, TransactionId cutoff_xid, uint8 flags); +extern uint8 visibilitymap_set_vmbyte(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