On 07/20/2015 07:11 PM, Alvaro Herrera wrote:
Heikki Linnakangas wrote:
Looking closer, heap vacuum does a similar thing, but there are two
mitigating factors that make it safe in practice, I think:
1. An empty heap page is all-zeroes except for the small page header in the
beginning of the page. For a torn page to matter, the page would need to be
torn in the header, but we rely elsewhere (control file) anyway that a
512-byte sector update is atomic, so that shouldn't happen. Note that this
hinges on the fact that there is no special area on heap pages, so you
cannot rely on this for index pages.
2. The redo of the first insert/update on a heap page will always
re-initialize the page, even when full-page-writes are disabled. This is the
XLOG_HEAP_INIT_PAGE optimization. So it's not just an optimization, it's
required for correctness.
Hmm, I guess this is a case of an optimization hiding a bug :-( I mean,
if we didn't have that, we would have noticed this a lot sooner, I
imagine.
Yeah, probably.
I came up with the attached, for GIN and SP-GiST. Instead of WAL-logging
the page initialization in SP-GiST vacuum, I changed it so that it
simply leaves the page as all-zeroes, and adds it to the FSM. The code
that grabs a target page from the FSM handles that, and initializes the
page anyway, so that was simpler. This made the SP-GiST is-deleted flag
obsolete, it's no longer set, although the code still checks for it for
backwards-compatibility. (even that may actually be unnecessary, as a
page that's marked as deleted must also be empty, and wherever we check
for the deleted-flag, we also check for PageIsEmpty()))
- Heikki
diff --git a/src/backend/access/gin/ginvacuum.c b/src/backend/access/gin/ginvacuum.c
index eba572b..1315762 100644
--- a/src/backend/access/gin/ginvacuum.c
+++ b/src/backend/access/gin/ginvacuum.c
@@ -710,7 +710,7 @@ ginvacuumcleanup(PG_FUNCTION_ARGS)
LockBuffer(buffer, GIN_SHARE);
page = (Page) BufferGetPage(buffer);
- if (GinPageIsDeleted(page))
+ if (PageIsNew(page) || GinPageIsDeleted(page))
{
Assert(blkno != GIN_ROOT_BLKNO);
RecordFreeIndexPage(index, blkno);
diff --git a/src/backend/access/spgist/spgvacuum.c b/src/backend/access/spgist/spgvacuum.c
index dc69d1e..36e93ad 100644
--- a/src/backend/access/spgist/spgvacuum.c
+++ b/src/backend/access/spgist/spgvacuum.c
@@ -621,14 +621,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 */
}
@@ -659,25 +655,18 @@ spgvacuumpage(spgBulkDeleteState *bds, BlockNumber blkno)
*/
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 413f71e..48dadd5 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)
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
