Fix handling of all-zero pages in SP-GiST vacuum.
authorHeikki Linnakangas <heikki.linnakangas@iki.fi>
Mon, 27 Jul 2015 09:28:21 +0000 (12:28 +0300)
committerHeikki Linnakangas <heikki.linnakangas@iki.fi>
Mon, 27 Jul 2015 09:32:08 +0000 (12:32 +0300)
SP-GiST initialized an all-zeros page at vacuum, but that was not
WAL-logged, which is not safe. You might get a torn page write, when it gets
flushed to disk, and end-up with a half-initialized index page. To fix,
leave it in the all-zeros state, and add it to the FSM. It will be
initialized when reused. Also don't set the page-deleted flag when recycling
an empty page. That was also not WAL-logged, and a torn write of that would
cause the page to have an invalid checksum.

Backpatch to 9.2, where SP-GiST indexes were added.

src/backend/access/spgist/spgvacuum.c
src/include/access/spgist_private.h

index 7c5a0016c2ab47ea5c92d95088e2ab94102b9b24..9033a9df35f9e11ce1eb3eb7f18e58041fe8af26 100644 (file)
@@ -618,14 +618,10 @@ spgvacuumpage(spgBulkDeleteState *bds, BlockNumber blkno)
    {
        /*
         * We found an all-zero page, which could happen if the database
-        * crashed just after extending the file.  Initialize and recycle it.
+        * crashed just after extending the file.  Recycle it.
         */
-       SpGistInitBuffer(buffer, 0);
-       SpGistPageSetDeleted(page);
-       /* We don't bother to WAL-log this action; easy to redo */
-       MarkBufferDirty(buffer);
    }
-   else if (SpGistPageIsDeleted(page))
+   else if (PageIsEmpty(page))
    {
        /* nothing to do */
    }
@@ -651,30 +647,23 @@ spgvacuumpage(spgBulkDeleteState *bds, BlockNumber blkno)
    /*
     * The root pages must never be deleted, nor marked as available in FSM,
     * because we don't want them ever returned by a search for a place to put
-    * a new tuple.  Otherwise, check for empty/deletable page, and make sure
-    * FSM knows about it.
+    * a new tuple.  Otherwise, check for empty page, and make sure the FSM
+    * knows about it.
     */
    if (!SpGistBlockIsRoot(blkno))
    {
-       /* If page is now empty, mark it deleted */
-       if (PageIsEmpty(page) && !SpGistPageIsDeleted(page))
-       {
-           SpGistPageSetDeleted(page);
-           /* We don't bother to WAL-log this action; easy to redo */
-           MarkBufferDirty(buffer);
-       }
-
-       if (SpGistPageIsDeleted(page))
+       if (PageIsEmpty(page))
        {
            RecordFreeIndexPage(index, blkno);
            bds->stats->pages_deleted++;
        }
        else
+       {
+           SpGistSetLastUsedPage(index, buffer);
            bds->lastFilledBlock = blkno;
+       }
    }
 
-   SpGistSetLastUsedPage(index, buffer);
-
    UnlockReleaseBuffer(buffer);
 }
 
index d092029d8a793c735efa10b7a1253c3d969905df..9293788b0b9216b581ff6eb28c41b801b45a061d 100644 (file)
@@ -48,14 +48,14 @@ typedef SpGistPageOpaqueData *SpGistPageOpaque;
 
 /* Flag bits in page special space */
 #define SPGIST_META            (1<<0)
-#define SPGIST_DELETED     (1<<1)
+#define SPGIST_DELETED     (1<<1)      /* never set, but keep for backwards
+                                        * compatibility */
 #define SPGIST_LEAF            (1<<2)
 #define SPGIST_NULLS       (1<<3)
 
 #define SpGistPageGetOpaque(page) ((SpGistPageOpaque) PageGetSpecialPointer(page))
 #define SpGistPageIsMeta(page) (SpGistPageGetOpaque(page)->flags & SPGIST_META)
 #define SpGistPageIsDeleted(page) (SpGistPageGetOpaque(page)->flags & SPGIST_DELETED)
-#define SpGistPageSetDeleted(page) (SpGistPageGetOpaque(page)->flags |= SPGIST_DELETED)
 #define SpGistPageIsLeaf(page) (SpGistPageGetOpaque(page)->flags & SPGIST_LEAF)
 #define SpGistPageStoresNulls(page) (SpGistPageGetOpaque(page)->flags & SPGIST_NULLS)