Ensure we MarkBufferDirty before visibilitymap_set()

logs the heap page and sets the LSN. Otherwise a
checkpoint could occur between those actions and
leave us in an inconsistent state.

Jeff Davis

diff --git a/src/backend/commands/vacuumlazy.c b/src/backend/commands/vacuumlazy.c
index 8a1ffcf0bd7e4cbdd9a81ba874b9de42e2eec1e8..9d304153b8bee4a4cd02701dc70173cdc06a60e1 100644 (file)
--- a/src/backend/commands/vacuumlazy.c
+++ b/src/backend/commands/vacuumlazy.c
@@ -894,26 +894,25 @@ lazy_scan_heap(Relation onerel, LVRelStats *vacrelstats,
        freespace = PageGetHeapFreeSpace(page);
 
        /* mark page all-visible, if appropriate */
-       if (all_visible)
+       if (all_visible && !all_visible_according_to_vm)
        {
-           if (!PageIsAllVisible(page))
-           {
-               PageSetAllVisible(page);
-               MarkBufferDirty(buf);
-               visibilitymap_set(onerel, blkno, buf, InvalidXLogRecPtr,
-                                 vmbuffer, visibility_cutoff_xid);
-           }
-           else if (!all_visible_according_to_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.  Set the visibility map bit as well so that we get
-                * back in sync.
-                */
-               visibilitymap_set(onerel, blkno, buf, InvalidXLogRecPtr,
-                                 vmbuffer, visibility_cutoff_xid);
-           }
+           /*
+            * 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 the both bits
+            * so that we get back in sync.
+            *
+            * NB: If the heap page is all-visible but the VM bit is not set,
+            * we don't need to dirty the heap page.  However, if checksums are
+            * enabled, we do need to make sure that the heap page is dirtied
+            * before passing it to visibilitymap_set(), because it may be
+            * logged.  Given that this situation should only happen in rare
+            * cases after a crash, it is not worth optimizing.
+            */
+           PageSetAllVisible(page);
+           MarkBufferDirty(buf);
+           visibilitymap_set(onerel, blkno, buf, InvalidXLogRecPtr,
+                             vmbuffer, visibility_cutoff_xid);
        }
 
        /*
@@ -1138,6 +1137,14 @@ lazy_vacuum_page(Relation onerel, BlockNumber blkno, Buffer buffer,
 
    PageRepairFragmentation(page);
 
+   /*
+    * Mark buffer dirty before we write WAL.
+    *
+    * If checksums are enabled, visibilitymap_set() may log the heap page, so
+    * we must mark heap buffer dirty before calling visibilitymap_set().
+    */
+   MarkBufferDirty(buffer);
+
    /*
     * Now that we have removed the dead tuples from the page, once again check
     * if the page has become all-visible.
@@ -1151,8 +1158,6 @@ lazy_vacuum_page(Relation onerel, BlockNumber blkno, Buffer buffer,
                visibility_cutoff_xid);
    }
 
-   MarkBufferDirty(buffer);
-
    /* XLOG stuff */
    if (RelationNeedsWAL(onerel))
    {