From 863af3a3797a3a8ca19c9bdd57c5f5b538e04e22 Mon Sep 17 00:00:00 2001 From: Heikki Linnakangas Date: Mon, 27 Jul 2015 12:28:21 +0300 Subject: [PATCH] Fix handling of all-zero pages in SP-GiST vacuum. 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 | 27 ++++++++------------------- src/include/access/spgist_private.h | 4 ++-- 2 files changed, 10 insertions(+), 21 deletions(-) diff --git a/src/backend/access/spgist/spgvacuum.c b/src/backend/access/spgist/spgvacuum.c index c7f1b8ec473..36ba24de583 100644 --- a/src/backend/access/spgist/spgvacuum.c +++ b/src/backend/access/spgist/spgvacuum.c @@ -619,14 +619,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 */ } @@ -652,30 +648,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); } diff --git a/src/include/access/spgist_private.h b/src/include/access/spgist_private.h index b45f795a890..60d509f68c2 100644 --- a/src/include/access/spgist_private.h +++ b/src/include/access/spgist_private.h @@ -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)