From fbf27c16f0add32135836ea843cdbc1b8fc4aa44 Mon Sep 17 00:00:00 2001 From: Melanie Plageman Date: Wed, 30 Jul 2025 18:51:43 -0400 Subject: [PATCH v11 18/20] Add helper functions to heap_page_prune_and_freeze heap_page_prune_and_freeze() has gotten rather long. It has several stages: 1) setup - where the PruneState is set up 2) tuple examination -- where tuples and line pointers are examined to determine what needs to be pruned and what could be frozen 3) evaluation -- where we determine based on caller provided options, heuristics, and state gathered during stage 2 whether or not to freeze tuples and set the page in the VM 4) execution - where the page changes are actually made and logged This commit refactors the evaluation stage into helpers which return whether or not to freeze and set the VM. XXX: For the purposes of committing, this likely shouldn't be a separate commit. But I'm not sure yet whether it makes more sense to do this refactoring earlier in the set for clarity for the reviewer. --- src/backend/access/heap/pruneheap.c | 473 +++++++++++++++++----------- 1 file changed, 296 insertions(+), 177 deletions(-) diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c index 69d8e42bdc8..67b56e45ad7 100644 --- a/src/backend/access/heap/pruneheap.c +++ b/src/backend/access/heap/pruneheap.c @@ -179,6 +179,22 @@ static void heap_prune_record_unchanged_lp_redirect(PruneState *prstate, OffsetN static void page_verify_redirects(Page page); +static bool heap_page_will_freeze(Relation relation, Buffer buffer, + bool do_prune, + bool do_hint_full_or_prunable, + bool did_tuple_hint_fpi, + PruneState *prstate, + bool *all_frozen_except_lp_dead); + +static bool heap_page_will_update_vm(Relation relation, + Buffer buffer, BlockNumber blockno, Page page, + PruneReason reason, + bool do_prune, bool do_freeze, + bool blk_known_av, + PruneState *prstate, + Buffer *vmbuffer, uint8 *vmflags, + bool *set_pd_all_visible); + static bool identify_and_fix_vm_corruption(Relation relation, BlockNumber heap_blk, Buffer heap_buffer, Page heap_page, @@ -382,6 +398,249 @@ identify_and_fix_vm_corruption(Relation relation, return false; } + +/* + * 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). + * + * We pass in blockno and page even those can be derived from buffer to avoid + * extra BufferGetBlock() and BufferGetBlockNumber() calls. + * + * 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. + * + * prstate and vmbuffer are input/output fields. vmflags and and + * set_pd_all_visible are output fields. + * + * Returns true if the caller should set one or both of the VM bits and false + * otherwise. + */ +static bool +heap_page_will_update_vm(Relation relation, + Buffer buffer, BlockNumber blockno, Page page, + PruneReason reason, + bool do_prune, bool do_freeze, + bool blk_known_av, + PruneState *prstate, + Buffer *vmbuffer, uint8 *vmflags, + bool *set_pd_all_visible) +{ + bool do_set_vm = false; + + /* + * If the caller specified not to update the VM, validate everything is in + * the right state and exit. + */ + if (!prstate->consider_update_vm) + { + Assert(!prstate->all_visible && !prstate->all_frozen); + /* We don't set only the page level visibility hint */ + Assert(!(*set_pd_all_visible)); + Assert(*vmflags == 0); + return false; + } + + /* + * If this is an on-access call and we're not actually pruning, avoid + * setting the visibility map if it would newly dirty the heap page or, if + * the page is already dirty, if doing so would require including a + * full-page image (FPI) of the heap page in the WAL. This situation + * should be rare, as on-access pruning is only attempted when + * pd_prune_xid is valid. + */ + if (reason == PRUNE_ON_ACCESS && + prstate->consider_update_vm && + prstate->all_visible && + !do_prune && !do_freeze && + (!BufferIsDirty(buffer) || XLogCheckBufferNeedsBackup(buffer))) + { + prstate->consider_update_vm = false; + prstate->all_visible = prstate->all_frozen = false; + } + + Assert(!prstate->all_frozen || prstate->all_visible); + + /* + * Clear any VM corruption. This does not need to be in a critical + * section, so we do it first. If PD_ALL_VISIBLE is incorrectly set, we + * may mark the heap page buffer dirty here and could end up doing so + * again later. This is not a correctness issue and is in the path of VM + * corruption, so we don't have to worry about the extra performance + * overhead. + */ + if (identify_and_fix_vm_corruption(relation, + blockno, buffer, page, + blk_known_av, prstate->lpdead_items, + *vmbuffer)) + { + /* If we fix corruption, don't update the VM further */ + } + + /* Determine if we actually need to set the VM and which bits to set. */ + else if (prstate->all_visible && + (!blk_known_av || + (prstate->all_frozen && !VM_ALL_FROZEN(relation, blockno, vmbuffer)))) + { + *vmflags |= VISIBILITYMAP_ALL_VISIBLE; + if (prstate->all_frozen) + *vmflags |= VISIBILITYMAP_ALL_FROZEN; + } + + do_set_vm = *vmflags & VISIBILITYMAP_VALID_BITS; + + /* + * Don't set PD_ALL_VISIBLE unless we also plan to set the VM. While it is + * correct for a heap page to have PD_ALL_VISIBLE even if the VM is not + * set, we strongly prefer to keep them in sync. + * + * Prior to Postgres 19, it was possible for the page-level bit to be set + * and the VM bit to be clear. This could happen if we crashed after + * setting PD_ALL_VISIBLE but before setting bits in the VM. + */ + *set_pd_all_visible = do_set_vm && !PageIsAllVisible(page); + return do_set_vm; +} + +/* + * Decide if we want to go ahead with freezing according to the freeze plans we + * prepared for the given buffer or not. If the caller specified we should not + * freeze tuples, it exits early. + * + * do_prune, do_hint_full_or_prunable, and did_tuple_hint_fpi must all have + * been decided before calling this function. + * + * prstate is an input/output parameter. all_frozen_except_lp_dead is set and + * used later to determine the snapshot conflict horizon for the record. + * + * Returns true if we should use our freeze plans and freeze tuples on the page + * and false otherwise. + */ +static bool +heap_page_will_freeze(Relation relation, Buffer buffer, + bool do_prune, + bool do_hint_full_or_prunable, + bool did_tuple_hint_fpi, + PruneState *prstate, + bool *all_frozen_except_lp_dead) +{ + bool do_freeze = false; + + /* + * If the caller specified we should not attempt to freeze any tuples, + * validate that everything is in the right state and exit. + */ + if (!prstate->attempt_freeze) + { + Assert(!prstate->all_frozen && prstate->nfrozen == 0); + Assert(prstate->lpdead_items == 0 || !prstate->all_visible); + Assert(!(*all_frozen_except_lp_dead)); + return false; + } + + if (prstate->pagefrz.freeze_required) + { + /* + * heap_prepare_freeze_tuple indicated that at least one XID/MXID from + * before FreezeLimit/MultiXactCutoff is present. Must freeze to + * advance relfrozenxid/relminmxid. + */ + do_freeze = true; + } + else + { + /* + * Opportunistically freeze the page if we are generating an FPI + * anyway and if doing so means that we can set the page all-frozen + * afterwards (might not happen until VACUUM's final heap pass). + * + * XXX: Previously, we knew if pruning emitted an FPI by checking + * pgWalUsage.wal_fpi before and after pruning. Once the freeze and + * prune records were combined, this heuristic couldn't be used + * anymore. The opportunistic freeze heuristic must be improved; + * however, for now, try to approximate the old logic. + */ + if (prstate->all_visible && prstate->all_frozen && prstate->nfrozen > 0) + { + /* + * Freezing would make the page all-frozen. Have already emitted + * an FPI or will do so anyway? + */ + if (RelationNeedsWAL(relation)) + { + if (did_tuple_hint_fpi) + do_freeze = true; + else if (do_prune) + { + if (XLogCheckBufferNeedsBackup(buffer)) + do_freeze = true; + } + else if (do_hint_full_or_prunable) + { + if (XLogHintBitIsNeeded() && XLogCheckBufferNeedsBackup(buffer)) + do_freeze = true; + } + } + } + } + + if (do_freeze) + { + /* + * Validate the tuples we will be freezing before entering the + * critical section. + */ + heap_pre_freeze_checks(buffer, prstate->frozen, prstate->nfrozen); + } + else if (prstate->nfrozen > 0) + { + /* + * The page contained some tuples that were not already frozen, and we + * chose not to freeze them now. The page won't be all-frozen then. + */ + Assert(!prstate->pagefrz.freeze_required); + + prstate->all_frozen = false; + prstate->nfrozen = 0; /* avoid miscounts in instrumentation */ + } + else + { + /* + * We have no freeze plans to execute. The page might already be + * all-frozen (perhaps only following pruning), though. Such pages + * can be marked all-frozen in the VM by our caller, even though none + * of its tuples were newly frozen here. + */ + } + + /* + * It was convenient to ignore LP_DEAD items in all_visible earlier on to + * make the choice of whether or not to freeze the page unaffected by the + * short-term presence of LP_DEAD items. These LP_DEAD items were + * effectively assumed to be LP_UNUSED items in the making. It doesn't + * matter which vacuum heap pass (initial pass or final pass) ends up + * setting the page all-frozen, as long as the ongoing VACUUM does it. + * + * Now that freezing has been finalized, unset all_visible if there are + * any LP_DEAD items on the page. It needs to reflect the present state of + * the page when using it to determine whether or not to update the VM. + * + * Keep track of whether or not the page was all-frozen except LP_DEAD + * items for the purposes of calculating the snapshot conflict horizon, + * though. + */ + *all_frozen_except_lp_dead = prstate->all_frozen; + if (prstate->lpdead_items > 0) + { + prstate->all_visible = false; + prstate->all_frozen = false; + } + + return do_freeze; +} + + /* * Prune and repair fragmentation and potentially freeze tuples on the * specified page. If the page's visibility status has changed, update it in @@ -772,20 +1031,30 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer, /* Clear the offset information once we have processed the given page. */ *off_loc = InvalidOffsetNumber; - do_prune = prstate.nredirected > 0 || - prstate.ndead > 0 || - prstate.nunused > 0; - /* * After processing all the live tuples on the page, if the newest xmin * amongst them is not visible to everyone, the page cannot be - * all-visible. + * all-visible. This must be done before we decide whether or not to + * opportunistically freeze below because we do not want to + * opportunistically freeze the page if there are live tuples not visible + * to everyone, which would prevent setting the page frozen in the VM. */ if (prstate.all_visible && TransactionIdIsNormal(prstate.visibility_cutoff_xid) && !GlobalVisXidVisibleToAll(prstate.vistest, prstate.visibility_cutoff_xid)) prstate.all_visible = prstate.all_frozen = false; + /* + * Now decide based on information collected while examining every tuple + * which actions to take. If there are any prunable tuples, we'll prune + * them. However, we will decide based on options specified by the caller + * and various heuristics whether or not to freeze any tuples and whether + * or not the page should be set all-visible/all-frozen in the VM. + */ + do_prune = prstate.nredirected > 0 || + prstate.ndead > 0 || + prstate.nunused > 0; + /* * Even if we don't prune anything, if we found a new value for the * pd_prune_xid field or the page was marked full, we will update those @@ -796,186 +1065,36 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer, PageIsFull(page); /* - * Decide if we want to go ahead with freezing according to the freeze - * plans we prepared, or not. - */ - do_freeze = false; - if (prstate.attempt_freeze) - { - if (prstate.pagefrz.freeze_required) - { - /* - * heap_prepare_freeze_tuple indicated that at least one XID/MXID - * from before FreezeLimit/MultiXactCutoff is present. Must - * freeze to advance relfrozenxid/relminmxid. - */ - do_freeze = true; - } - else - { - /* - * Opportunistically freeze the page if we are generating an FPI - * anyway and if doing so means that we can set the page - * all-frozen afterwards (might not happen until VACUUM's final - * heap pass). - * - * XXX: Previously, we knew if pruning emitted an FPI by checking - * pgWalUsage.wal_fpi before and after pruning. Once the freeze - * and prune records were combined, this heuristic couldn't be - * used anymore. The opportunistic freeze heuristic must be - * improved; however, for now, try to approximate the old logic. - */ - if (prstate.all_visible && prstate.all_frozen && prstate.nfrozen > 0) - { - /* - * Freezing would make the page all-frozen. Have already - * emitted an FPI or will do so anyway? - */ - if (RelationNeedsWAL(relation)) - { - if (did_tuple_hint_fpi) - do_freeze = true; - else if (do_prune) - { - if (XLogCheckBufferNeedsBackup(buffer)) - do_freeze = true; - } - else if (do_hint_full_or_prunable) - { - if (XLogHintBitIsNeeded() && XLogCheckBufferNeedsBackup(buffer)) - do_freeze = true; - } - } - } - } - } - - if (do_freeze) - { - /* - * Validate the tuples we will be freezing before entering the - * critical section. - */ - heap_pre_freeze_checks(buffer, prstate.frozen, prstate.nfrozen); - } - else if (prstate.nfrozen > 0) - { - /* - * The page contained some tuples that were not already frozen, and we - * chose not to freeze them now. The page won't be all-frozen then. - */ - Assert(!prstate.pagefrz.freeze_required); - - prstate.all_frozen = false; - prstate.nfrozen = 0; /* avoid miscounts in instrumentation */ - } - else - { - /* - * We have no freeze plans to execute. The page might already be - * all-frozen (perhaps only following pruning), though. Such pages - * can be marked all-frozen in the VM by our caller, even though none - * of its tuples were newly frozen here. - */ - } - - /* - * It was convenient to ignore LP_DEAD items in all_visible earlier on to - * make the choice of whether or not to freeze the page unaffected by the - * short-term presence of LP_DEAD items. These LP_DEAD items were - * effectively assumed to be LP_UNUSED items in the making. It doesn't - * matter which vacuum heap pass (initial pass or final pass) ends up - * setting the page all-frozen, as long as the ongoing VACUUM does it. - * - * Now that freezing has been finalized, unset all_visible if there are - * any LP_DEAD items on the page. It needs to reflect the present state of - * the page when using it to determine whether or not to update the VM. - * - * Keep track of whether or not the page was all-frozen except LP_DEAD - * items for the purposes of calculating the snapshot conflict horizon, - * though. + * We must decide whether or not to freeze before deciding if and what to + * set in the VM. */ - all_frozen_except_lp_dead = prstate.all_frozen; - if (prstate.lpdead_items > 0) - { - prstate.all_visible = false; - prstate.all_frozen = false; - } + do_freeze = heap_page_will_freeze(relation, buffer, + do_prune, + do_hint_full_or_prunable, + did_tuple_hint_fpi, + &prstate, + &all_frozen_except_lp_dead); + + do_set_vm = heap_page_will_update_vm(relation, + buffer, blockno, page, + reason, + do_prune, do_freeze, + blk_known_av, + &prstate, + &vmbuffer, + &vmflags, &set_pd_all_visible); - /* - * If this is an on-access call and we're not actually pruning, avoid - * setting the visibility map if it would newly dirty the heap page or, if - * the page is already dirty, if doing so would require including a - * full-page image (FPI) of the heap page in the WAL. This situation - * should be rare, as on-access pruning is only attempted when - * pd_prune_xid is valid. - */ - if (reason == PRUNE_ON_ACCESS && - prstate.consider_update_vm && - prstate.all_visible && - !do_prune && !do_freeze && - (!BufferIsDirty(buffer) || XLogCheckBufferNeedsBackup(buffer))) - { - prstate.consider_update_vm = false; - prstate.all_visible = prstate.all_frozen = false; - } - - Assert(!prstate.all_frozen || prstate.all_visible); - - /* - * Handle setting visibility map bit based on information from the VM (if - * provided, e.g. by vacuum from the last heap_vac_scan_next_block() - * call), and from all_visible and all_frozen variables. - */ - if (prstate.consider_update_vm) - { - /* - * Clear any VM corruption. This does not need to be in a critical - * section, so we do it first. If PD_ALL_VISIBLE is incorrectly set, - * we may mark the heap page buffer dirty here and could end up doing - * so again later. This is not a correctness issue and is in the path - * of VM corruption, so we don't have to worry about the extra - * performance overhead. - */ - if (identify_and_fix_vm_corruption(relation, - blockno, buffer, page, - blk_known_av, prstate.lpdead_items, vmbuffer)) - { - /* If we fix corruption, don't update the VM further */ - } - - /* Determine if we actually need to set the VM and which bits to set. */ - else if (prstate.all_visible && - (!blk_known_av || - (prstate.all_frozen && !VM_ALL_FROZEN(relation, blockno, &vmbuffer)))) - { - vmflags |= VISIBILITYMAP_ALL_VISIBLE; - if (prstate.all_frozen) - vmflags |= VISIBILITYMAP_ALL_FROZEN; - } - } - - do_set_vm = vmflags & VISIBILITYMAP_VALID_BITS; + /* Save these for the caller in case we later zero out vmflags */ + presult->new_vmbits = vmflags; - /* Lock vmbuffer before entering a critical section */ + /* Lock vmbuffer before entering critical section */ if (do_set_vm) LockBuffer(vmbuffer, BUFFER_LOCK_EXCLUSIVE); /* - * Don't set PD_ALL_VISIBLE unless we also plan to set the VM. While it is - * correct for a heap page to have PD_ALL_VISIBLE even if the VM is not - * set, we strongly prefer to keep them in sync. - * - * Prior to Postgres 19, it was possible for the page-level bit to be set - * and the VM bit to be clear. This could happen if we crashed after - * setting PD_ALL_VISIBLE but before setting bits in the VM. + * Time to actually make the changes to the page and log them. Any error + * while applying the changes is critical. */ - set_pd_all_visible = do_set_vm && !PageIsAllVisible(page); - - /* Save these for the caller in case we later zero out vmflags */ - presult->new_vmbits = vmflags; - - /* Any error while applying the changes is critical */ START_CRIT_SECTION(); if (do_hint_full_or_prunable) -- 2.43.0