From 0fc1b4cbb4e67b193eca8347dca1bf8053d2020e Mon Sep 17 00:00:00 2001 From: Melanie Plageman Date: Tue, 2 Dec 2025 13:36:39 -0500 Subject: [PATCH v32 03/16] Refactor lazy_scan_prune() VM clear logic into helper Encapsulating them in a helper makes the whole function clearer. There is no functional change other than moving it into a helper. Reviewed-by: Kirill Reshke Reviewed-by: Chao Li --- src/backend/access/heap/vacuumlazy.c | 132 +++++++++++++++++---------- 1 file changed, 85 insertions(+), 47 deletions(-) diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c index 3733a1cbc47..5857fd1bfb6 100644 --- a/src/backend/access/heap/vacuumlazy.c +++ b/src/backend/access/heap/vacuumlazy.c @@ -424,6 +424,11 @@ static void find_next_unskippable_block(LVRelState *vacrel, bool *skipsallvis); static bool lazy_scan_new_or_empty(LVRelState *vacrel, Buffer buf, BlockNumber blkno, Page page, bool sharelock, Buffer vmbuffer); +static bool identify_and_fix_vm_corruption(Relation rel, Buffer heap_buffer, + BlockNumber heap_blk, Page heap_page, + int nlpdead_items, + Buffer vmbuffer, + uint8 vmbits); static int lazy_scan_prune(LVRelState *vacrel, Buffer buf, BlockNumber blkno, Page page, Buffer vmbuffer, @@ -1957,6 +1962,83 @@ cmpOffsetNumbers(const void *a, const void *b) return pg_cmp_u16(*(const OffsetNumber *) a, *(const OffsetNumber *) b); } +/* + * Helper to correct any corruption detected on a heap page and its + * corresponding visibility map page after pruning but before setting the + * visibility map. It examines the heap page, the associated VM page, and the + * number of dead items previously identified. + * + * This function must be called while holding an exclusive lock on the heap + * buffer, and the dead items must have been discovered under that same lock. + + * The provided vmbits must reflect the current state of the VM block + * referenced by vmbuffer. Although we do not hold a lock on the VM buffer, it + * is pinned, and the heap buffer is exclusively locked, ensuring that no + * other backend can update the VM bits corresponding to this heap page. + * + * Returns true if it cleared corruption and false otherwise. + */ +static bool +identify_and_fix_vm_corruption(Relation rel, Buffer heap_buffer, + BlockNumber heap_blk, Page heap_page, + int nlpdead_items, + Buffer vmbuffer, + uint8 vmbits) +{ + Assert(visibilitymap_get_status(rel, heap_blk, &vmbuffer) == vmbits); + + Assert(BufferIsLockedByMeInMode(heap_buffer, BUFFER_LOCK_EXCLUSIVE)); + + /* + * 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. + */ + if (!PageIsAllVisible(heap_page) && + ((vmbits & VISIBILITYMAP_VALID_BITS) != 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(rel), heap_blk))); + + visibilitymap_clear(rel, heap_blk, vmbuffer, + VISIBILITYMAP_VALID_BITS); + return 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 (PageIsAllVisible(heap_page) && nlpdead_items > 0) + { + ereport(WARNING, + (errcode(ERRCODE_DATA_CORRUPTED), + errmsg("page containing LP_DEAD items is marked as all-visible in relation \"%s\" page %u", + RelationGetRelationName(rel), heap_blk))); + + PageClearAllVisible(heap_page); + MarkBufferDirty(heap_buffer); + visibilitymap_clear(rel, heap_blk, vmbuffer, + VISIBILITYMAP_VALID_BITS); + return true; + } + + return false; +} + /* * lazy_scan_prune() -- lazy_scan_heap() pruning and freezing. * @@ -2099,54 +2181,10 @@ lazy_scan_prune(LVRelState *vacrel, old_vmbits = visibilitymap_get_status(vacrel->rel, blkno, &vmbuffer); - /* - * 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. - */ - if (!PageIsAllVisible(page) && - (old_vmbits & VISIBILITYMAP_VALID_BITS) != 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); - /* VM bits are now clear */ + if (identify_and_fix_vm_corruption(vacrel->rel, buf, blkno, page, + presult.lpdead_items, vmbuffer, + old_vmbits)) old_vmbits = 0; - } - - /* - * 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)) - { - 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); - /* VM bits are now clear */ - old_vmbits = 0; - } if (!presult.all_visible) return presult.ndeleted; -- 2.43.0