From 19af794b660e3711d8b698aeedfc33e13dc235d8 Mon Sep 17 00:00:00 2001 From: Melanie Plageman Date: Mon, 26 Jan 2026 17:12:05 -0500 Subject: [PATCH] Refactor lazy_scan_prune() VM clear logic into helper Encapsulating the cases that clear the visibility map after vacuum phase I, when corruption is detected, into in a helper makes the code cleaner and enables further refactoring in future commits. Author: Melanie Plageman Reviewed-by: Andres Freund Reviewed-by: Kirill Reshke Reviewed-by: Chao Li Reviewed-by: Andrey Borodin Discussion: https://postgr.es/m/7ib3sa55sapwjlaz4sijbiq7iezna27kjvvvar4dpgkmadml6t%40gfpkkwmdnepx --- src/backend/access/heap/vacuumlazy.c | 131 +++++++++++++++++---------- 1 file changed, 83 insertions(+), 48 deletions(-) diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c index d5021450eb4..4be267ff657 100644 --- a/src/backend/access/heap/vacuumlazy.c +++ b/src/backend/access/heap/vacuumlazy.c @@ -422,6 +422,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 void 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, @@ -1955,6 +1960,81 @@ 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. + * + * If it clears corruption, it will zero out vmbits. + */ +static void +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); + *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 (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); + *vmbits = 0; + } +} + /* * lazy_scan_prune() -- lazy_scan_heap() pruning and freezing. * @@ -2097,54 +2177,9 @@ 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 */ - 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; - } + identify_and_fix_vm_corruption(vacrel->rel, buf, blkno, page, + presult.lpdead_items, vmbuffer, + &old_vmbits); if (!presult.all_visible) return presult.ndeleted; -- 2.39.5