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 <melanieplageman@gmail.com>
Reviewed-by: Andres Freund <andres@anarazel.de>
Reviewed-by: Kirill Reshke <reshkekirill@gmail.com>
Reviewed-by: Chao Li <li.evan.chao@gmail.com>
Reviewed-by: Andrey Borodin <x4mmm@yandex-team.ru>
Discussion: https://postgr.es/m/7ib3sa55sapwjlaz4sijbiq7iezna27kjvvvar4dpgkmadml6t%40gfpkkwmdnepx

diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index d5021450eb4791ff1c550a3a677f434b320def35..4be267ff657e1d2fdd34a644cb3f9727dbdaf46d 100644 (file)
--- 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;