From de93f7eaffb009436cae2f80571ba0148f99db7a Mon Sep 17 00:00:00 2001 From: Melanie Plageman Date: Mon, 15 Sep 2025 16:25:44 -0400 Subject: [PATCH v14 07/24] Update PruneState.all_[visible|frozen] sooner in pruning We don't clear PruneState.all_visible and all_frozen during pruning when we see LP_DEAD items because we want to still opportunistically freeze a page if it would become frozen after vacuum's third phase. Currently, this is fine because heap_page_prune_and_freeze() doesn't set PD_ALL_VISIBLE or set bits in the VM. If we want to do that in the future, we need all_visible and all_frozen to be accurate earlier in heap_page_prune_and_freeze(). To do this, we must also move up determination of the freeze conflict horizon. We use the visibility cutoff xid even if the whole page won't be frozen until after vacuum's third phase. --- src/backend/access/heap/pruneheap.c | 95 ++++++++++++++--------------- 1 file changed, 45 insertions(+), 50 deletions(-) diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c index 4ed74de6f27..5e536bd0d4d 100644 --- a/src/backend/access/heap/pruneheap.c +++ b/src/backend/access/heap/pruneheap.c @@ -296,7 +296,9 @@ heap_page_prune_opt(Relation relation, Buffer buffer) * pre-freeze checks. * * do_prune, do_hint_full_or_prunable, and did_tuple_hint_fpi must all have - * been decided before calling this function. + * been decided 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. * @@ -308,7 +310,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; @@ -378,6 +381,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) { @@ -478,6 +497,7 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer, 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 = vistest; @@ -546,10 +566,10 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer, * 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) { @@ -784,8 +804,24 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer, 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 */ @@ -846,27 +882,8 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer, * 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_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 @@ -890,30 +907,8 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer, 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; /* -- 2.43.0