From 00d6a64f3c5fa4d87e59968b636941846e6c542b Mon Sep 17 00:00:00 2001 From: Melanie Plageman Date: Wed, 28 May 2025 16:35:36 -0400 Subject: [PATCH v1 10/14] Combine vacuum phase I VM update cases After phase I of vacuum we update the VM, either setting the VM bits when all bits are currently unset or setting just the frozen bit when the all-visible bit is already set. Those cases had a lot of duplicated code. Combine them. This is simpler to understand and also allows makes the code compact enough to start using to update the VM while pruning and freezing. --- src/backend/access/heap/vacuumlazy.c | 71 +++++++++------------------- 1 file changed, 22 insertions(+), 49 deletions(-) diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c index 8328cab0955..fdac36f0835 100644 --- a/src/backend/access/heap/vacuumlazy.c +++ b/src/backend/access/heap/vacuumlazy.c @@ -2158,11 +2158,26 @@ lazy_scan_prune(LVRelState *vacrel, { /* Don't update the VM if we just cleared corruption in it */ } - else if (!all_visible_according_to_vm && presult.all_visible) + + /* + * If the page isn't yet marked all-visible in the VM or it is and needs + * to me marked all-frozen, update the VM Note that all_frozen is only + * valid if all_visible is true, so we must check both all_visible and + * all_frozen. + */ + else if (presult.all_visible && + (!all_visible_according_to_vm || + (presult.all_frozen && !VM_ALL_FROZEN(vacrel->rel, blkno, &vmbuffer)))) { 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)); @@ -2174,6 +2189,12 @@ lazy_scan_prune(LVRelState *vacrel, 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. */ @@ -2193,54 +2214,6 @@ lazy_scan_prune(LVRelState *vacrel, *vm_page_frozen = true; } } - - /* - * 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. - */ - else if (all_visible_according_to_vm && presult.all_visible && - presult.all_frozen && !VM_ALL_FROZEN(vacrel->rel, blkno, &vmbuffer)) - { - uint8 old_vmbits; - - /* - * Set the page all-frozen (and all-visible) in the VM. - * - * 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. - */ - Assert(!TransactionIdIsValid(presult.vm_conflict_horizon)); - old_vmbits = heap_page_set_vm_and_log(vacrel->rel, blkno, buf, - vmbuffer, InvalidTransactionId, - VISIBILITYMAP_ALL_VISIBLE | - VISIBILITYMAP_ALL_FROZEN); - - /* - * The page was likely already set all-visible in the VM. However, - * there is a small chance that it was modified sometime between - * setting all_visible_according_to_vm and checking the visibility - * during pruning. Check the return value of old_vmbits anyway to - * ensure the visibility map counters used for logging are accurate. - */ - if ((old_vmbits & VISIBILITYMAP_ALL_VISIBLE) == 0) - { - vacrel->vm_new_visible_pages++; - vacrel->vm_new_visible_frozen_pages++; - *vm_page_frozen = true; - } - - /* - * We already checked that the page was not set all-frozen in the VM - * above, so we don't need to test the value of old_vmbits. - */ - else - { - vacrel->vm_new_frozen_pages++; - *vm_page_frozen = true; - } - } } /* -- 2.34.1