From 75a2d24ed02733533027b9fe17f25160d2529b0c Mon Sep 17 00:00:00 2001 From: Melanie Plageman Date: Tue, 16 Sep 2025 15:46:40 -0400 Subject: [PATCH v15 15/23] Set VM in heap_page_prune_and_freeze The determination as to whether or not the page can be set all-visible/all-frozen has already been done by the end of heap_page_prune_and_freeze(). Vacuum waited until it returns to lazy_scan_prune() to actually set the VM, though. This commit moves setting the VM into heap_page_prune_and_freeze(). There are still two separate WAL records -- one for the changes to the heap page and one for the changes to the VM. But, this is an incremental step toward logging setting the VM in the same WAL record as pruning and freezing. Note that this is not used by on-access pruning. --- src/backend/access/heap/pruneheap.c | 221 +++++++++++++++++++++++---- src/backend/access/heap/vacuumlazy.c | 146 ++---------------- src/include/access/heapam.h | 24 +-- 3 files changed, 221 insertions(+), 170 deletions(-) diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c index 9e00fbf3cd1..e3f9967e26c 100644 --- a/src/backend/access/heap/pruneheap.c +++ b/src/backend/access/heap/pruneheap.c @@ -19,6 +19,7 @@ #include "access/htup_details.h" #include "access/multixact.h" #include "access/transam.h" +#include "access/visibilitymap.h" #include "access/xlog.h" #include "access/visibilitymapdefs.h" #include "access/xloginsert.h" @@ -257,7 +258,9 @@ heap_page_prune_opt(Relation relation, Buffer buffer) * not the relation has indexes, since we cannot safely determine * that during on-access pruning with the current implementation. */ - heap_page_prune_and_freeze(relation, buffer, PRUNE_ON_ACCESS, 0, NULL, + heap_page_prune_and_freeze(relation, buffer, + InvalidBuffer, false, + PRUNE_ON_ACCESS, 0, NULL, vistest, &presult, &dummy_off_loc, NULL, NULL); /* @@ -423,16 +426,115 @@ heap_page_will_freeze(Relation relation, Buffer buffer, return do_freeze; } +/* + * Determine whether to set the visibility map bits based on information from + * the PruneState and blk_known_av, which some callers will provide after + * previously examining this heap page's VM bits (e.g. vacuum from the last + * heap_vac_scan_next_block() call). + * + * This should be called only after do_freeze has been decided (and do_prune + * has been set), as these factor into our heuristic-based decision. + * + * Returns true if the caller should set one or both of the VM bits and false + * otherwise. + */ +static bool +heap_page_will_set_vis(Relation relation, + BlockNumber heap_blk, + Buffer heap_buf, + Buffer vmbuffer, + bool blk_known_av, + PruneState *prstate, + uint8 *vmflags, + bool *do_set_pd_vis) +{ + Page heap_page = BufferGetPage(heap_buf); + bool do_set_vm = false; + + if (prstate->all_visible && !PageIsAllVisible(heap_page)) + *do_set_pd_vis = true; + + if ((prstate->all_visible && !blk_known_av) || + (prstate->all_frozen && !VM_ALL_FROZEN(relation, heap_blk, &vmbuffer))) + { + *vmflags = VISIBILITYMAP_ALL_VISIBLE; + if (prstate->all_frozen) + *vmflags |= VISIBILITYMAP_ALL_FROZEN; + + do_set_vm = true; + } + + /* + * Now handle two potential corruption cases: + * + * These do not need to happen in a critical section and are not + * WAL-logged. + * + * As of PostgreSQL 9.2, the visibility map bit should never be set if the + * page-level bit is clear. However, it's possible that in vacuum the bit + * got cleared after heap_vac_scan_next_block() was called, so we must + * recheck with buffer lock before concluding that the VM is corrupt. + */ + else if (blk_known_av && !PageIsAllVisible(heap_page) && + visibilitymap_get_status(relation, heap_blk, &vmbuffer) != 0) + { + ereport(WARNING, + (errcode(ERRCODE_DATA_CORRUPTED), + errmsg("page is not marked all-visible but visibility map bit is set in relation \"%s\" page %u", + RelationGetRelationName(relation), heap_blk))); + + visibilitymap_clear(relation, heap_blk, vmbuffer, + VISIBILITYMAP_VALID_BITS); + } + + /* + * It's possible for the value returned by + * GetOldestNonRemovableTransactionId() to move backwards, so it's not + * wrong for us to see tuples that appear to not be visible to everyone + * yet, while PD_ALL_VISIBLE is already set. The real safe xmin value + * never moves backwards, but GetOldestNonRemovableTransactionId() is + * conservative and sometimes returns a value that's unnecessarily small, + * so if we see that contradiction it just means that the tuples that we + * think are not visible to everyone yet actually are, and the + * PD_ALL_VISIBLE flag is correct. + * + * There should never be LP_DEAD items on a page with PD_ALL_VISIBLE set, + * however. + */ + else if (prstate->lpdead_items > 0 && PageIsAllVisible(heap_page)) + { + ereport(WARNING, + (errcode(ERRCODE_DATA_CORRUPTED), + errmsg("page containing LP_DEAD items is marked as all-visible in relation \"%s\" page %u", + RelationGetRelationName(relation), heap_blk))); + + PageClearAllVisible(heap_page); + MarkBufferDirty(heap_buf); + visibilitymap_clear(relation, heap_blk, vmbuffer, + VISIBILITYMAP_VALID_BITS); + } + + /* We should only set the VM if PD_ALL_VISIBLE is set or will be */ + Assert(!do_set_vm || PageIsAllVisible(heap_page) || *do_set_pd_vis); + + return do_set_vm; +} /* * Prune and repair fragmentation and potentially freeze tuples on the - * specified page. + * specified page. If the page's visibility status has changed, update it in + * the VM. * * Caller must have pin and buffer cleanup lock on the page. Note that we * don't update the FSM information for page on caller's behalf. Caller might * also need to account for a reduction in the length of the line pointer * array following array truncation by us. * + * vmbuffer is the buffer that must already contain contain the required block + * of the visibility map if we are to update it. blk_known_av is the + * visibility status of the heap block as of the last call to + * find_next_unskippable_block(). + * * reason indicates why the pruning is performed. It is included in the WAL * record for debugging and analysis purposes, but otherwise has no effect. * @@ -443,15 +545,20 @@ heap_page_will_freeze(Relation relation, Buffer buffer, * FREEZE indicates that we will also freeze tuples, and will return * 'all_visible', 'all_frozen' flags to the caller. * - * If the HEAP_PRUNE_FREEZE option is set, we will freeze tuples if it's + * UPDATE_VIS indicates that we will set the page's status in the VM. + * + * If the HEAP_PRUNE_FREEZE option is set, we will also freeze tuples if it's * required in order to advance relfrozenxid / relminmxid, or if it's * considered advantageous for overall system performance to do so now. The * 'cutoffs', 'presult', 'new_relfrozen_xid' and 'new_relmin_mxid' arguments - * are required when freezing. When HEAP_PRUNE_FREEZE option is set, we also - * set presult->all_visible and presult->all_frozen on exit, to indicate if - * the VM bits can be set. They are always set to false when the - * HEAP_PRUNE_FREEZE option is not set, because at the moment only callers - * that also freeze need that information. + * are required when freezing. + * + * If HEAP_PAGE_PRUNE_UPDATE_VIS is set and the visibility status of the page + * has changed, we will update the VM at the same time as pruning and freezing + * the heap page. We will also update presult->old_vmbits and + * presult->new_vmbits with the state of the VM before and after updating it + * for the caller to use in bookkeeping. + * * * cutoffs contains the freeze cutoffs, established by VACUUM at the beginning * of vacuuming the relation. Required if HEAP_PRUNE_FREEZE option is set. @@ -478,6 +585,7 @@ heap_page_will_freeze(Relation relation, Buffer buffer, */ void heap_page_prune_and_freeze(Relation relation, Buffer buffer, + Buffer vmbuffer, bool blk_known_av, PruneReason reason, int options, const struct VacuumCutoffs *cutoffs, @@ -496,10 +604,13 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer, bool do_freeze; bool do_prune; bool do_hint_prune; + bool do_set_vm; bool do_set_pd_vis; bool did_tuple_hint_fpi; int64 fpi_before = pgWalUsage.wal_fpi; TransactionId frz_conflict_horizon = InvalidTransactionId; + uint8 new_vmbits = 0; + uint8 old_vmbits = 0; /* Copy parameters to prstate */ prstate.vistest = vistest; @@ -828,19 +939,27 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer, Assert(!prstate.all_frozen || prstate.all_visible); /* - * Though callers should set the VM if PD_ALL_VISIBLE is set here, it is - * allowed for the page-level bit to be set and the VM to be clear. + * Determine whether or not to set the page level PD_ALL_VISIBLE and the + * visibility map bits based on information from the VM and from + * all_visible and all_frozen variables. + * + * Though callers should set the VM if PD_ALL_VISIBLE is set, it is + * allowed for the page-level bit to be set and the VM to be clear. We log + * setting PD_ALL_VISIBLE on the heap page in a + * XLOG_HEAP2_PRUNE_VACUUM_SCAN record and setting the VM bits in a later + * emitted XLOG_HEAP2_VISIBLE record. + * * Setting PD_ALL_VISIBLE when we are making the changes to the page that * render it all-visible allows us to omit the heap page from the WAL * chain when later updating the VM -- even when checksums/wal_log_hints * are enabled. */ do_set_pd_vis = false; + do_set_vm = false; if ((options & HEAP_PAGE_PRUNE_UPDATE_VIS) != 0) - { - if (prstate.all_visible && !PageIsAllVisible(page)) - do_set_pd_vis = true; - } + do_set_vm = heap_page_will_set_vis(relation, + blockno, buffer, vmbuffer, blk_known_av, + &prstate, &new_vmbits, &do_set_pd_vis); /* Any error while applying the changes is critical */ START_CRIT_SECTION(); @@ -928,28 +1047,72 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer, END_CRIT_SECTION(); + Assert(!prstate.all_visible || (prstate.lpdead_items == 0)); + + /* + * VACUUM will call heap_page_would_be_all_visible() during the second + * pass over the heap to determine all_visible and all_frozen for the page + * -- this is a specialized version of that logic. Now that we've finished + * pruning and freezing, make sure that we're in total agreement with + * heap_page_would_be_all_visible() using an assertion. + */ +#ifdef USE_ASSERT_CHECKING + if (prstate.all_visible) + { + TransactionId debug_cutoff; + bool debug_all_frozen; + + Assert(prstate.lpdead_items == 0); + Assert(prstate.cutoffs); + + if (!heap_page_is_all_visible(relation, buffer, + prstate.cutoffs->OldestXmin, + &debug_all_frozen, + &debug_cutoff, off_loc)) + Assert(false); + + Assert(prstate.all_frozen == debug_all_frozen); + + Assert(!TransactionIdIsValid(debug_cutoff) || + debug_cutoff == prstate.visibility_cutoff_xid); + } +#endif + + /* Now set the VM */ + if (do_set_vm) + { + TransactionId vm_conflict_horizon; + + Assert((new_vmbits & VISIBILITYMAP_VALID_BITS) != 0); + + /* + * The conflict horizon for that record must be the newest xmin on the + * page. However, if the page is completely frozen, there can be no + * conflict and the vm_conflict_horizon should remain + * InvalidTransactionId. This includes the case that we just froze + * all the tuples; the prune-freeze record included the conflict XID + * already so a snapshotConflictHorizon sufficient to make everything + * safe for REDO was logged when the page's tuples were frozen. + */ + if (prstate.all_frozen) + vm_conflict_horizon = InvalidTransactionId; + else + vm_conflict_horizon = prstate.visibility_cutoff_xid; + old_vmbits = visibilitymap_set(relation, blockno, + InvalidXLogRecPtr, + vmbuffer, vm_conflict_horizon, + new_vmbits); + } + /* Copy information back for caller */ presult->ndeleted = prstate.ndeleted; presult->nnewlpdead = prstate.ndead; presult->nfrozen = prstate.nfrozen; presult->live_tuples = prstate.live_tuples; presult->recently_dead_tuples = prstate.recently_dead_tuples; - presult->all_visible = prstate.all_visible; - presult->all_frozen = prstate.all_frozen; presult->hastup = prstate.hastup; - - /* - * For callers planning to update the visibility map, the conflict horizon - * for that record must be the newest xmin on the page. However, if the - * page is completely frozen, there can be no conflict and the - * vm_conflict_horizon should remain InvalidTransactionId. This includes - * the case that we just froze all the tuples; the prune-freeze record - * included the conflict XID already so the caller doesn't need it. - */ - if (presult->all_frozen) - presult->vm_conflict_horizon = InvalidTransactionId; - else - presult->vm_conflict_horizon = prstate.visibility_cutoff_xid; + presult->new_vmbits = new_vmbits; + presult->old_vmbits = old_vmbits; presult->lpdead_items = prstate.lpdead_items; /* the presult->deadoffsets array was already filled in */ diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c index e8721761392..d71f3755dce 100644 --- a/src/backend/access/heap/vacuumlazy.c +++ b/src/backend/access/heap/vacuumlazy.c @@ -463,11 +463,6 @@ static void dead_items_add(LVRelState *vacrel, BlockNumber blkno, OffsetNumber * int num_offsets); static void dead_items_reset(LVRelState *vacrel); static void dead_items_cleanup(LVRelState *vacrel); -static bool heap_page_is_all_visible(Relation rel, Buffer buf, - TransactionId OldestXmin, - bool *all_frozen, - TransactionId *visibility_cutoff_xid, - OffsetNumber *logging_offnum); static bool heap_page_would_be_all_visible(Relation rel, Buffer buf, TransactionId OldestXmin, OffsetNumber *deadoffsets, @@ -2015,7 +2010,9 @@ lazy_scan_prune(LVRelState *vacrel, if (vacrel->nindexes == 0) prune_options |= HEAP_PAGE_PRUNE_MARK_UNUSED_NOW; - heap_page_prune_and_freeze(rel, buf, PRUNE_VACUUM_SCAN, prune_options, + heap_page_prune_and_freeze(rel, buf, + vmbuffer, all_visible_according_to_vm, + PRUNE_VACUUM_SCAN, prune_options, &vacrel->cutoffs, vacrel->vistest, &presult, @@ -2036,33 +2033,6 @@ lazy_scan_prune(LVRelState *vacrel, vacrel->new_frozen_tuple_pages++; } - /* - * VACUUM will call heap_page_is_all_visible() during the second pass over - * the heap to determine all_visible and all_frozen for the page -- this - * is a specialized version of the logic from this function. Now that - * we've finished pruning and freezing, make sure that we're in total - * agreement with heap_page_is_all_visible() using an assertion. - */ -#ifdef USE_ASSERT_CHECKING - if (presult.all_visible) - { - TransactionId debug_cutoff; - bool debug_all_frozen; - - Assert(presult.lpdead_items == 0); - - if (!heap_page_is_all_visible(vacrel->rel, buf, - vacrel->cutoffs.OldestXmin, &debug_all_frozen, - &debug_cutoff, &vacrel->offnum)) - Assert(false); - - Assert(presult.all_frozen == debug_all_frozen); - - Assert(!TransactionIdIsValid(debug_cutoff) || - debug_cutoff == presult.vm_conflict_horizon); - } -#endif - /* * Now save details of the LP_DEAD items from the page in vacrel */ @@ -2096,112 +2066,28 @@ lazy_scan_prune(LVRelState *vacrel, /* Did we find LP_DEAD items? */ *has_lpdead_items = (presult.lpdead_items > 0); - Assert(!presult.all_visible || !(*has_lpdead_items)); - Assert(!presult.all_frozen || presult.all_visible); - /* - * Handle setting visibility map bits based on information from the VM (as - * of last heap_vac_scan_next_block() call), and from all_visible and - * all_frozen variables. + * For the purposes of logging, count whether or not the page was newly + * set all-visible and, potentially, all-frozen. */ - if ((presult.all_visible && !all_visible_according_to_vm) || - (presult.all_frozen && !VM_ALL_FROZEN(vacrel->rel, blkno, &vmbuffer))) + if ((presult.old_vmbits & VISIBILITYMAP_ALL_VISIBLE) == 0 && + (presult.new_vmbits & VISIBILITYMAP_ALL_VISIBLE) != 0) { - uint8 old_vmbits; - uint8 flags = VISIBILITYMAP_ALL_VISIBLE; - - /* - * If the page is all-frozen, we can pass InvalidTransactionId as our - * cutoff_xid, since a snapshotConflictHorizon sufficient to make - * everything safe for REDO was logged when the page's tuples were - * frozen. - */ - if (presult.all_frozen) - { - Assert(!TransactionIdIsValid(presult.vm_conflict_horizon)); - flags |= VISIBILITYMAP_ALL_FROZEN; - } - - old_vmbits = visibilitymap_set(vacrel->rel, blkno, - InvalidXLogRecPtr, - vmbuffer, presult.vm_conflict_horizon, - flags); - - /* - * Even if we are only setting the all-frozen bit, there is a small - * chance that the VM was modified sometime between setting - * all_visible_according_to_vm and checking the visibility during - * pruning. Check the return value of old_vmbits to ensure the - * visibility map counters used for logging are accurate. - * - * If the page wasn't already set all-visible and/or all-frozen in the - * VM, count it as newly set for logging. - */ - if ((old_vmbits & VISIBILITYMAP_ALL_VISIBLE) == 0) - { - vacrel->vm_new_visible_pages++; - if (presult.all_frozen) - { - vacrel->vm_new_visible_frozen_pages++; - *vm_page_frozen = true; - } - } - else if ((old_vmbits & VISIBILITYMAP_ALL_FROZEN) == 0 && - presult.all_frozen) + vacrel->vm_new_visible_pages++; + if ((presult.new_vmbits & VISIBILITYMAP_ALL_FROZEN) != 0) { - vacrel->vm_new_frozen_pages++; + vacrel->vm_new_visible_frozen_pages++; *vm_page_frozen = true; } } - - /* - * Now handle two potential corruption cases: - * - * As of PostgreSQL 9.2, the visibility map bit should never be set if the - * page-level bit is clear. However, it's possible that the bit got - * cleared after heap_vac_scan_next_block() was called, so we must recheck - * with buffer lock before concluding that the VM is corrupt. - */ - else if (all_visible_according_to_vm && !PageIsAllVisible(page) && - visibilitymap_get_status(vacrel->rel, blkno, &vmbuffer) != 0) - { - ereport(WARNING, - (errcode(ERRCODE_DATA_CORRUPTED), - errmsg("page is not marked all-visible but visibility map bit is set in relation \"%s\" page %u", - vacrel->relname, blkno))); - - visibilitymap_clear(vacrel->rel, blkno, vmbuffer, - VISIBILITYMAP_VALID_BITS); - } - - /* - * It's possible for the value returned by - * GetOldestNonRemovableTransactionId() to move backwards, so it's not - * wrong for us to see tuples that appear to not be visible to everyone - * yet, while PD_ALL_VISIBLE is already set. The real safe xmin value - * never moves backwards, but GetOldestNonRemovableTransactionId() is - * conservative and sometimes returns a value that's unnecessarily small, - * so if we see that contradiction it just means that the tuples that we - * think are not visible to everyone yet actually are, and the - * PD_ALL_VISIBLE flag is correct. - * - * There should never be LP_DEAD items on a page with PD_ALL_VISIBLE set, - * however. - */ - else if (presult.lpdead_items > 0 && PageIsAllVisible(page)) + else if ((presult.old_vmbits & VISIBILITYMAP_ALL_FROZEN) == 0 && + (presult.new_vmbits & VISIBILITYMAP_ALL_FROZEN) != 0) { - ereport(WARNING, - (errcode(ERRCODE_DATA_CORRUPTED), - errmsg("page containing LP_DEAD items is marked as all-visible in relation \"%s\" page %u", - vacrel->relname, blkno))); - - PageClearAllVisible(page); - MarkBufferDirty(buf); - visibilitymap_clear(vacrel->rel, blkno, vmbuffer, - VISIBILITYMAP_VALID_BITS); + Assert((presult.new_vmbits & VISIBILITYMAP_ALL_VISIBLE) != 0); + vacrel->vm_new_frozen_pages++; + *vm_page_frozen = true; } - return presult.ndeleted; } @@ -3591,7 +3477,7 @@ dead_items_cleanup(LVRelState *vacrel) * Wrapper for heap_page_would_be_all_visible() which can be used for * callers that expect no LP_DEAD on the page. */ -static bool +bool heap_page_is_all_visible(Relation rel, Buffer buf, TransactionId OldestXmin, bool *all_frozen, diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h index e97b53f1ee8..493ddeacbc0 100644 --- a/src/include/access/heapam.h +++ b/src/include/access/heapam.h @@ -235,19 +235,14 @@ typedef struct PruneFreezeResult int recently_dead_tuples; /* - * all_visible and all_frozen indicate if the all-visible and all-frozen - * bits in the visibility map can be set for this page, after pruning. + * old_vmbits are the state of the all-visible and all-frozen bits in the + * visibility map before updating it during phase I of vacuuming. + * new_vmbits are the state of those bits after phase I of vacuuming. * - * vm_conflict_horizon is the newest xmin of live tuples on the page. The - * caller can use it as the conflict horizon when setting the VM bits. It - * is only valid if we froze some tuples (nfrozen > 0), and all_frozen is - * true. - * - * These are only set if the HEAP_PRUNE_FREEZE option is set. + * These are only set if the HEAP_PAGE_PRUNE_UPDATE_VIS option is set. */ - bool all_visible; - bool all_frozen; - TransactionId vm_conflict_horizon; + uint8 new_vmbits; + uint8 old_vmbits; /* * Whether or not the page makes rel truncation unsafe. This is set to @@ -369,6 +364,7 @@ extern TransactionId heap_index_delete_tuples(Relation rel, struct GlobalVisState; extern void heap_page_prune_opt(Relation relation, Buffer buffer); extern void heap_page_prune_and_freeze(Relation relation, Buffer buffer, + Buffer vmbuffer, bool blk_known_av, PruneReason reason, int options, const struct VacuumCutoffs *cutoffs, @@ -397,6 +393,12 @@ extern void log_heap_prune_and_freeze(Relation relation, Buffer buffer, extern void heap_vacuum_rel(Relation rel, const VacuumParams params, BufferAccessStrategy bstrategy); +extern bool heap_page_is_all_visible(Relation rel, Buffer buf, + TransactionId OldestXmin, + bool *all_frozen, + TransactionId *visibility_cutoff_xid, + OffsetNumber *logging_offnum); + /* in heap/heapam_visibility.c */ extern bool HeapTupleSatisfiesVisibility(HeapTuple htup, Snapshot snapshot, Buffer buffer); -- 2.43.0