From 78d8fd0ab8ef94c9de7f3a4c8f308ce0c2cba54b Mon Sep 17 00:00:00 2001 From: Melanie Plageman Date: Mon, 15 Sep 2025 17:48:38 -0400 Subject: [PATCH v15 08/23] Combine vacuum phase I VM update cases We update the VM after phase I of vacuum -- either setting both the VM bits when all bits are currently unset or setting just the frozen bit when the all-visible bit is already set. Those two cases shared much of the same code -- leading to unnecessary duplication. This commit combines them, which is simpler and easier to understand. --- src/backend/access/heap/vacuumlazy.c | 68 ++++++++-------------------- 1 file changed, 18 insertions(+), 50 deletions(-) diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c index 308abff16ca..5a6bbbd97f2 100644 --- a/src/backend/access/heap/vacuumlazy.c +++ b/src/backend/access/heap/vacuumlazy.c @@ -2058,15 +2058,22 @@ lazy_scan_prune(LVRelState *vacrel, Assert(!presult.all_frozen || presult.all_visible); /* - * Handle setting visibility map bit based on information from the VM (as + * 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 + * all_frozen variables. */ - if (!all_visible_according_to_vm && presult.all_visible) + 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)); @@ -2079,6 +2086,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. */ @@ -2100,6 +2113,8 @@ lazy_scan_prune(LVRelState *vacrel, } /* + * 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 @@ -2144,53 +2159,6 @@ lazy_scan_prune(LVRelState *vacrel, VISIBILITYMAP_VALID_BITS); } - /* - * If the all-visible page is all-frozen but not marked as such yet, mark - * it as all-frozen. - */ - else if (all_visible_according_to_vm && 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 = visibilitymap_set(vacrel->rel, blkno, buf, - InvalidXLogRecPtr, - 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; - } - } return presult.ndeleted; } -- 2.43.0