From d4a4be3eed25853fc1ea84ebc2cbe0226afd823a Mon Sep 17 00:00:00 2001 From: Melanie Plageman Date: Mon, 15 Sep 2025 16:25:44 -0400 Subject: [PATCH v17 05/15] Update PruneState.all_[visible|frozen] earlier in pruning MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In the prune/freeze path, we currently delay clearing all_visible and all_frozen when dead items are present. This allows opportunistic freezing if the page would otherwise be fully frozen, since those dead items are later removed in vacuum’s third phase. However, if no freezing will be attempted, there’s no need to delay. Clearing the flags promptly avoids extra bookkeeping in heap_prune_unchanged_lp_normal(). At present this has no runtime effect because all callers that consider setting the VM also attempt freezing, but future callers (e.g. on-access pruning) may want to set the VM without preparing freeze plans. We also used to defer clearing all_visible and all_frozen until after computing the visibility cutoff XID. By determining the cutoff earlier, we can update these flags immediately after deciding whether to opportunistically freeze. This is necessary if we want to set the VM in the same WAL record that prunes and freezes tuples on the page. While we are at it, unset all_frozen whenever we unset all_visible. Previously we could only use all_frozen in combination with all_visible as all_frozen was not unset when not all-visible tuples were encountered. It is best to keep them both up-to-date to avoid mistakes when using all_frozen. --- src/backend/access/heap/pruneheap.c | 144 ++++++++++++++------------- src/backend/access/heap/vacuumlazy.c | 9 +- 2 files changed, 77 insertions(+), 76 deletions(-) diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c index 44214a57ecd..5892ed5a07e 100644 --- a/src/backend/access/heap/pruneheap.c +++ b/src/backend/access/heap/pruneheap.c @@ -138,15 +138,12 @@ typedef struct * bits. It is only valid if we froze some tuples, and all_frozen is * true. * - * NOTE: all_visible and all_frozen don't include LP_DEAD items. That's - * convenient for heap_page_prune_and_freeze(), to use them to decide - * whether to freeze the page or not. The all_visible and all_frozen - * values returned to the caller are adjusted to include LP_DEAD items at - * the end. - * - * all_frozen should only be considered valid if all_visible is also set; - * we don't bother to clear the all_frozen flag every time we clear the - * all_visible flag. + * NOTE: all_visible and all_frozen initially don't include LP_DEAD items. + * That's convenient for heap_page_prune_and_freeze(), to use them to + * decide whether to freeze the page or not. The all_visible and + * all_frozen values returned to the caller are adjusted to include + * LP_DEAD items after we determine whether or not to opportunistically + * freeze. */ bool all_visible; bool all_frozen; @@ -309,7 +306,8 @@ heap_page_prune_opt(Relation relation, Buffer buffer) * pre-freeze checks. * * do_prune, do_hint_prune, and did_tuple_hint_fpi must all have been decided - * before calling this function. + * before calling this function. *frz_conflict_horizon is set to the snapshot + * conflict horizon we for the WAL record should we decide to freeze tuples. * * prstate is an input/output parameter. * @@ -321,7 +319,8 @@ heap_page_will_freeze(Relation relation, Buffer buffer, bool did_tuple_hint_fpi, bool do_prune, bool do_hint_prune, - PruneState *prstate) + PruneState *prstate, + TransactionId *frz_conflict_horizon) { bool do_freeze = false; @@ -358,8 +357,10 @@ heap_page_will_freeze(Relation relation, Buffer buffer, * 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) + if (prstate->all_frozen && prstate->nfrozen > 0) { + Assert(prstate->all_visible); + /* * Freezing would make the page all-frozen. Have already emitted * an FPI or will do so anyway? @@ -389,6 +390,22 @@ heap_page_will_freeze(Relation relation, Buffer buffer, * critical section. */ heap_pre_freeze_checks(buffer, prstate->frozen, prstate->nfrozen); + + /* + * Calculate what the snapshot conflict horizon should be for a record + * freezing tuples. We can use the visibility_cutoff_xid as our cutoff + * for conflicts when the whole page is eligible to become all-frozen + * in the VM once we're done with it. Otherwise we generate a + * conservative cutoff by stepping back from OldestXmin. + */ + if (prstate->all_frozen) + *frz_conflict_horizon = prstate->visibility_cutoff_xid; + else + { + /* Avoids false conflicts when hot_standby_feedback in use */ + *frz_conflict_horizon = prstate->cutoffs->OldestXmin; + TransactionIdRetreat(*frz_conflict_horizon); + } } else if (prstate->nfrozen > 0) { @@ -433,10 +450,11 @@ heap_page_will_freeze(Relation relation, Buffer buffer, * considered advantageous for overall system performance to do so now. The * 'params.cutoffs', 'presult', 'new_relfrozen_xid' and 'new_relmin_mxid' * arguments are required when freezing. When HEAP_PRUNE_FREEZE option is - * passed, 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 passed, because at the moment only - * callers that also freeze need that information. + * passed, we also set presult->all_visible and presult->all_frozen after + * determining whether or not to opporunistically freeze, to indicate if the + * VM bits can be set. They are always set to false when the + * HEAP_PRUNE_FREEZE option is not passed, because at the moment only callers + * that also freeze need that information. * * presult contains output parameters needed by callers, such as the number of * tuples removed and the offsets of dead items on the page after pruning. @@ -472,6 +490,7 @@ heap_page_prune_and_freeze(PruneFreezeParams *params, bool do_hint_prune; bool did_tuple_hint_fpi; int64 fpi_before = pgWalUsage.wal_fpi; + TransactionId frz_conflict_horizon = InvalidTransactionId; /* Copy parameters to prstate */ prstate.vistest = params->vistest; @@ -541,10 +560,10 @@ heap_page_prune_and_freeze(PruneFreezeParams *params, * are tuples present that are not visible to everyone or if there are * dead tuples which are not yet removable. However, dead tuples which * will be removed by the end of vacuuming should not preclude us from - * opportunistically freezing. Because of that, we do not clear - * all_visible when we see LP_DEAD items. We fix that at the end of the - * function, when we return the value to the caller, so that the caller - * doesn't set the VM bit incorrectly. + * opportunistically freezing. Because of that, we do not immediately + * clear all_visible when we see LP_DEAD items. We fix that after + * scanning the line pointers, before we return the value to the caller, + * so that the caller doesn't set the VM bit incorrectly. */ if (prstate.attempt_freeze) { @@ -779,8 +798,26 @@ heap_page_prune_and_freeze(PruneFreezeParams *params, did_tuple_hint_fpi, do_prune, do_hint_prune, - &prstate); + &prstate, + &frz_conflict_horizon); + /* + * While scanning the line pointers, we did not clear + * all_visible/all_frozen when encountering LP_DEAD items because we + * wanted the decision whether or not to freeze the page to be unaffected + * by the short-term presence of LP_DEAD items. These LP_DEAD items are + * 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 we finished determining whether or not to freeze the page, + * update all_visible and all_frozen so that they reflect the true state + * of the page for setting PD_ALL_VISIBLE and VM bits. + */ + if (prstate.lpdead_items > 0) + prstate.all_visible = prstate.all_frozen = false; + + Assert(!prstate.all_frozen || prstate.all_visible); /* Any error while applying the changes is critical */ START_CRIT_SECTION(); @@ -839,27 +876,8 @@ heap_page_prune_and_freeze(PruneFreezeParams *params, * on the standby with xids older than the youngest tuple this * record will freeze will conflict. */ - TransactionId frz_conflict_horizon = InvalidTransactionId; TransactionId conflict_xid; - /* - * We can use the visibility_cutoff_xid as our cutoff for - * conflicts when the whole page is eligible to become all-frozen - * in the VM once we're done with it. Otherwise we generate a - * conservative cutoff by stepping back from OldestXmin. - */ - if (do_freeze) - { - if (prstate.all_visible && prstate.all_frozen) - frz_conflict_horizon = prstate.visibility_cutoff_xid; - else - { - /* Avoids false conflicts when hot_standby_feedback in use */ - frz_conflict_horizon = prstate.cutoffs->OldestXmin; - TransactionIdRetreat(frz_conflict_horizon); - } - } - if (TransactionIdFollows(frz_conflict_horizon, prstate.latest_xid_removed)) conflict_xid = frz_conflict_horizon; else @@ -885,30 +903,8 @@ heap_page_prune_and_freeze(PruneFreezeParams *params, presult->nfrozen = prstate.nfrozen; presult->live_tuples = prstate.live_tuples; presult->recently_dead_tuples = prstate.recently_dead_tuples; - - /* - * 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, as expected by our caller. - */ - if (prstate.all_visible && prstate.lpdead_items == 0) - { - presult->all_visible = prstate.all_visible; - presult->all_frozen = prstate.all_frozen; - } - else - { - presult->all_visible = false; - presult->all_frozen = false; - } - + presult->all_visible = prstate.all_visible; + presult->all_frozen = prstate.all_frozen; presult->hastup = prstate.hastup; /* @@ -1288,8 +1284,11 @@ heap_prune_record_dead(PruneState *prstate, OffsetNumber offnum, /* * Deliberately delay unsetting all_visible until later during pruning. - * Removable dead tuples shouldn't preclude freezing the page. + * Removable dead tuples shouldn't preclude freezing the page. If we won't + * attempt freezing, just unset all-visible now, though. */ + if (!prstate->attempt_freeze) + prstate->all_visible = prstate->all_frozen = false; /* Record the dead offset for vacuum */ prstate->deadoffsets[prstate->lpdead_items++] = offnum; @@ -1415,7 +1414,7 @@ heap_prune_record_unchanged_lp_normal(Page page, PruneState *prstate, OffsetNumb if (!HeapTupleHeaderXminCommitted(htup)) { - prstate->all_visible = false; + prstate->all_visible = prstate->all_frozen = false; break; } @@ -1437,7 +1436,7 @@ heap_prune_record_unchanged_lp_normal(Page page, PruneState *prstate, OffsetNumb Assert(prstate->cutoffs); if (!TransactionIdPrecedes(xmin, prstate->cutoffs->OldestXmin)) { - prstate->all_visible = false; + prstate->all_visible = prstate->all_frozen = false; break; } @@ -1450,7 +1449,7 @@ heap_prune_record_unchanged_lp_normal(Page page, PruneState *prstate, OffsetNumb case HEAPTUPLE_RECENTLY_DEAD: prstate->recently_dead_tuples++; - prstate->all_visible = false; + prstate->all_visible = prstate->all_frozen = false; /* * This tuple will soon become DEAD. Update the hint field so @@ -1469,7 +1468,7 @@ heap_prune_record_unchanged_lp_normal(Page page, PruneState *prstate, OffsetNumb * assumption is a bit shaky, but it is what acquire_sample_rows() * does, so be consistent. */ - prstate->all_visible = false; + prstate->all_visible = prstate->all_frozen = false; /* * If we wanted to optimize for aborts, we might consider marking @@ -1487,7 +1486,7 @@ heap_prune_record_unchanged_lp_normal(Page page, PruneState *prstate, OffsetNumb * will commit and update the counters after we report. */ prstate->live_tuples++; - prstate->all_visible = false; + prstate->all_visible = prstate->all_frozen = false; /* * This tuple may soon become DEAD. Update the hint field so that @@ -1555,8 +1554,11 @@ heap_prune_record_unchanged_lp_dead(Page page, PruneState *prstate, OffsetNumber * Similarly, don't unset all_visible until later, at the end of * heap_page_prune_and_freeze(). This will allow us to attempt to freeze * the page after pruning. As long as we unset it before updating the - * visibility map, this will be correct. + * visibility map, this will be correct. If we won't attempt freezing, + * though, just unset all-visible now. */ + if (!prstate->attempt_freeze) + prstate->all_visible = prstate->all_frozen = false; /* Record the dead offset for vacuum */ prstate->deadoffsets[prstate->lpdead_items++] = offnum; diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c index b25050d6773..56a0286662b 100644 --- a/src/backend/access/heap/vacuumlazy.c +++ b/src/backend/access/heap/vacuumlazy.c @@ -2012,7 +2012,6 @@ lazy_scan_prune(LVRelState *vacrel, * agreement with heap_page_is_all_visible() using an assertion. */ #ifdef USE_ASSERT_CHECKING - /* Note that all_frozen value does not matter when !all_visible */ if (presult.all_visible) { TransactionId debug_cutoff; @@ -2065,6 +2064,7 @@ lazy_scan_prune(LVRelState *vacrel, *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 bit based on information from the VM (as @@ -2170,11 +2170,10 @@ lazy_scan_prune(LVRelState *vacrel, /* * If the all-visible page is all-frozen but not marked as such yet, mark - * it as all-frozen. Note that all_frozen is only valid if all_visible is - * true, so we must check both all_visible and all_frozen. + * it as all-frozen. */ - else if (all_visible_according_to_vm && presult.all_visible && - presult.all_frozen && !VM_ALL_FROZEN(vacrel->rel, blkno, &vmbuffer)) + else if (all_visible_according_to_vm && presult.all_frozen && + !VM_ALL_FROZEN(vacrel->rel, blkno, &vmbuffer)) { uint8 old_vmbits; -- 2.43.0