From 842dd8da0c38440315de4e01bda026970b42d7eb Mon Sep 17 00:00:00 2001 From: Melanie Plageman Date: Tue, 2 Dec 2025 13:36:39 -0500 Subject: [PATCH v24 04/16] Refactor lazy_scan_prune() VM set logic into helper While this may not be an improvement on its own, encapsulating the logic for determining what to set the VM bits to in a helper is one step toward setting the VM in heap_page_prune_and_freeze(). --- src/backend/access/heap/vacuumlazy.c | 209 ++++++++++++++++----------- 1 file changed, 126 insertions(+), 83 deletions(-) diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c index 14040552e48..577950c2f77 100644 --- a/src/backend/access/heap/vacuumlazy.c +++ b/src/backend/access/heap/vacuumlazy.c @@ -1934,6 +1934,104 @@ cmpOffsetNumbers(const void *a, const void *b) return pg_cmp_u16(*(const OffsetNumber *) a, *(const OffsetNumber *) b); } + +/* + * Decide whether to set the visibility map bits (all-visible and all-frozen) + * for heap_blk using information from PruneFreezeResult and + * all_visible_according_to_vm. This function does not actually set the VM + * bits or page-level visibility hint, PD_ALL_VISIBLE. + * + * If it finds that the page-level visibility hint or VM is corrupted, it will + * fix them by clearing the VM bits and visibility page hint. This does not + * need to be done in a critical section. + * + * Returns true if one or both VM bits should be set, along with returning + * what bits should be set in the VM in *new_vmbits. + */ +static bool +heap_page_will_set_vm(Relation relation, + BlockNumber heap_blk, + Buffer heap_buf, + Buffer vmbuffer, + bool all_visible_according_to_vm, + const PruneFreezeResult *presult, + uint8 *new_vmbits) +{ + Page heap_page = BufferGetPage(heap_buf); + + *new_vmbits = 0; + + /* + * Determine what to set the visibility map bits to based on information + * from the VM (as of last heap_vac_scan_next_block() call), and from + * all_visible and all_frozen variables. + */ + if ((presult->all_visible && !all_visible_according_to_vm) || + (presult->all_frozen && !VM_ALL_FROZEN(relation, heap_blk, &vmbuffer))) + { + *new_vmbits = VISIBILITYMAP_ALL_VISIBLE; + if (presult->all_frozen) + { + Assert(!TransactionIdIsValid(presult->vm_conflict_horizon)); + *new_vmbits |= VISIBILITYMAP_ALL_FROZEN; + } + + return true; + } + + /* + * Now handle two potential corruption cases: + * + * These do not need to happen in a critical section and are not + * WAL-logged. + * + * 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 + * with buffer lock before concluding that the VM is corrupt. + */ + else if (all_visible_according_to_vm && !PageIsAllVisible(heap_page) && + visibilitymap_get_status(relation, heap_blk, &vmbuffer) != 0) + { + ereport(WARNING, + (errcode(ERRCODE_DATA_CORRUPTED), + errmsg("page is not marked all-visible but visibility map bit is set in relation \"%s\" page %u", + RelationGetRelationName(relation), heap_blk))); + + visibilitymap_clear(relation, heap_blk, vmbuffer, + VISIBILITYMAP_VALID_BITS); + } + + /* + * It's possible for the value returned by + * GetOldestNonRemovableTransactionId() to move backwards, so it's not + * wrong for us to see tuples that appear to not be visible to everyone + * yet, while PD_ALL_VISIBLE is already set. The real safe xmin value + * never moves backwards, but GetOldestNonRemovableTransactionId() is + * conservative and sometimes returns a value that's unnecessarily small, + * so if we see that contradiction it just means that the tuples that we + * think are not visible to everyone yet actually are, and the + * PD_ALL_VISIBLE flag is correct. + * + * There should never be LP_DEAD items on a page with PD_ALL_VISIBLE set, + * however. + */ + else if (presult->lpdead_items > 0 && PageIsAllVisible(heap_page)) + { + ereport(WARNING, + (errcode(ERRCODE_DATA_CORRUPTED), + errmsg("page containing LP_DEAD items is marked as all-visible in relation \"%s\" page %u", + RelationGetRelationName(relation), heap_blk))); + + PageClearAllVisible(heap_page); + MarkBufferDirty(heap_buf); + visibilitymap_clear(relation, heap_blk, vmbuffer, + VISIBILITYMAP_VALID_BITS); + } + + return false; +} + /* * lazy_scan_prune() -- lazy_scan_heap() pruning and freezing. * @@ -1964,6 +2062,9 @@ lazy_scan_prune(LVRelState *vacrel, bool *vm_page_frozen) { Relation rel = vacrel->rel; + bool do_set_vm = false; + uint8 new_vmbits = 0; + uint8 old_vmbits = 0; PruneFreezeResult presult; PruneFreezeParams params = { .relation = rel, @@ -2075,33 +2176,20 @@ lazy_scan_prune(LVRelState *vacrel, 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 - * of last heap_vac_scan_next_block() call), and from all_visible and - * all_frozen variables - */ - if ((presult.all_visible && !all_visible_according_to_vm) || - (presult.all_frozen && !VM_ALL_FROZEN(rel, blkno, &vmbuffer))) - { - uint8 old_vmbits; - uint8 new_vmbits = VISIBILITYMAP_ALL_VISIBLE; - - if (presult.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. - */ - Assert(!TransactionIdIsValid(presult.vm_conflict_horizon)); - new_vmbits |= VISIBILITYMAP_ALL_FROZEN; - } + do_set_vm = heap_page_will_set_vm(rel, + blkno, + buf, + vmbuffer, + all_visible_according_to_vm, + &presult, + &new_vmbits); + if (do_set_vm) + { /* * It should never be the case that the visibility map page is set * while the page-level bit is clear, but the reverse is allowed (if - * checksums are not enabled). Regardless, set both bits so that we - * get back in sync. + * checksums are not enabled). * * The heap page is added to the WAL chain even if it wasn't modified, * so we still need to mark it dirty. The only scenario where it isn't @@ -2114,73 +2202,28 @@ lazy_scan_prune(LVRelState *vacrel, InvalidXLogRecPtr, vmbuffer, presult.vm_conflict_horizon, new_vmbits); - - /* - * For the purposes of logging, count whether or not the page was - * newly set all-visible and, potentially, all-frozen. - */ - if ((old_vmbits & VISIBILITYMAP_ALL_VISIBLE) == 0 && - (new_vmbits & VISIBILITYMAP_ALL_VISIBLE) != 0) - { - vacrel->vm_new_visible_pages++; - if ((new_vmbits & VISIBILITYMAP_ALL_FROZEN) != 0) - { - vacrel->vm_new_visible_frozen_pages++; - *vm_page_frozen = true; - } - } - else if ((old_vmbits & VISIBILITYMAP_ALL_FROZEN) == 0 && - (new_vmbits & VISIBILITYMAP_ALL_FROZEN) != 0) - { - Assert((new_vmbits & VISIBILITYMAP_ALL_VISIBLE) != 0); - vacrel->vm_new_frozen_pages++; - *vm_page_frozen = true; - } } /* - * 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 - * with buffer lock before concluding that the VM is corrupt. + * For the purposes of logging, count whether or not the page was newly + * set all-visible and, potentially, all-frozen. */ - else if (all_visible_according_to_vm && !PageIsAllVisible(page) && - visibilitymap_get_status(vacrel->rel, blkno, &vmbuffer) != 0) + if ((old_vmbits & VISIBILITYMAP_ALL_VISIBLE) == 0 && + (new_vmbits & VISIBILITYMAP_ALL_VISIBLE) != 0) { - ereport(WARNING, - (errcode(ERRCODE_DATA_CORRUPTED), - errmsg("page is not marked all-visible but visibility map bit is set in relation \"%s\" page %u", - vacrel->relname, blkno))); - - visibilitymap_clear(vacrel->rel, blkno, vmbuffer, - VISIBILITYMAP_VALID_BITS); + vacrel->vm_new_visible_pages++; + if ((new_vmbits & VISIBILITYMAP_ALL_FROZEN) != 0) + { + vacrel->vm_new_visible_frozen_pages++; + *vm_page_frozen = true; + } } - - /* - * It's possible for the value returned by - * GetOldestNonRemovableTransactionId() to move backwards, so it's not - * wrong for us to see tuples that appear to not be visible to everyone - * yet, while PD_ALL_VISIBLE is already set. The real safe xmin value - * never moves backwards, but GetOldestNonRemovableTransactionId() is - * conservative and sometimes returns a value that's unnecessarily small, - * so if we see that contradiction it just means that the tuples that we - * think are not visible to everyone yet actually are, and the - * PD_ALL_VISIBLE flag is correct. - * - * There should never be LP_DEAD items on a page with PD_ALL_VISIBLE set, - * however. - */ - else if (presult.lpdead_items > 0 && PageIsAllVisible(page)) + else if ((old_vmbits & VISIBILITYMAP_ALL_FROZEN) == 0 && + (new_vmbits & VISIBILITYMAP_ALL_FROZEN) != 0) { - ereport(WARNING, - (errcode(ERRCODE_DATA_CORRUPTED), - errmsg("page containing LP_DEAD items is marked as all-visible in relation \"%s\" page %u", - vacrel->relname, blkno))); - - PageClearAllVisible(page); - MarkBufferDirty(buf); - visibilitymap_clear(vacrel->rel, blkno, vmbuffer, - VISIBILITYMAP_VALID_BITS); + Assert((new_vmbits & VISIBILITYMAP_ALL_VISIBLE) != 0); + vacrel->vm_new_frozen_pages++; + *vm_page_frozen = true; } return presult.ndeleted; -- 2.43.0