From 6236991143aa9e6a10121431451972c2991533c7 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Sun, 6 Nov 2005 22:39:21 +0000 Subject: [PATCH] Add simple sanity checks on newly-read pages to GiST, too. --- src/backend/access/gist/gist.c | 30 +++++--- src/backend/access/gist/gistget.c | 10 ++- src/backend/access/gist/gistutil.c | 109 ++++++++++++++++++++------- src/backend/access/gist/gistvacuum.c | 13 +++- src/include/access/gist_private.h | 3 +- 5 files changed, 119 insertions(+), 46 deletions(-) diff --git a/src/backend/access/gist/gist.c b/src/backend/access/gist/gist.c index 00b1620ab0..71d07d87c1 100644 --- a/src/backend/access/gist/gist.c +++ b/src/backend/access/gist/gist.c @@ -8,7 +8,7 @@ * Portions Copyright (c) 1994, Regents of the University of California * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/access/gist/gist.c,v 1.127 2005/10/18 01:06:22 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/access/gist/gist.c,v 1.128 2005/11/06 22:39:20 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -529,12 +529,12 @@ gistfindleaf(GISTInsertState *state, GISTSTATE *giststate) * ready to recheck path in a bad case... We remember, that page->lsn * should never be invalid. */ - while (true) + for (;;) { - if (XLogRecPtrIsInvalid(state->stack->lsn)) state->stack->buffer = ReadBuffer(state->r, state->stack->blkno); LockBuffer(state->stack->buffer, GIST_SHARE); + gistcheckpage(state->r, state->stack->buffer); state->stack->page = (Page) BufferGetPage(state->stack->buffer); opaque = GistPageGetOpaque(state->stack->page); @@ -555,7 +555,6 @@ gistfindleaf(GISTInsertState *state, GISTSTATE *giststate) continue; } - if (!GistPageIsLeaf(state->stack->page)) { /* @@ -649,14 +648,18 @@ gistReadAndLockBuffer(Relation r, BlockNumber blkno) } /* - * Traverse the tree to find path from root page, - * to prevent deadlocks, it should lock only one page simultaneously. - * Function uses in recovery and usial mode, so should work with different - * read functions (gistReadAndLockBuffer and XLogReadBuffer) + * Traverse the tree to find path from root page. + * * returns from the begining of closest parent; + * + * Function is used in both regular and recovery mode, so must work with + * different read functions (gistReadAndLockBuffer and XLogReadBuffer) + * + * To prevent deadlocks, this should lock only one page simultaneously. */ GISTInsertStack * -gistFindPath(Relation r, BlockNumber child, Buffer (*myReadBuffer) (Relation, BlockNumber)) +gistFindPath(Relation r, BlockNumber child, + Buffer (*myReadBuffer) (Relation, BlockNumber)) { Page page; Buffer buffer; @@ -674,7 +677,8 @@ gistFindPath(Relation r, BlockNumber child, Buffer (*myReadBuffer) (Relation, Bl while (top && top->blkno != child) { - buffer = myReadBuffer(r, top->blkno); /* buffer locked */ + buffer = myReadBuffer(r, top->blkno); /* locks buffer */ + gistcheckpage(r, buffer); page = (Page) BufferGetPage(buffer); if (GistPageIsLeaf(page)) @@ -771,9 +775,9 @@ gistFindCorrectParent(Relation r, GISTInsertStack *child) GISTInsertStack *parent = child->parent; LockBuffer(parent->buffer, GIST_EXCLUSIVE); + gistcheckpage(r, parent->buffer); parent->page = (Page) BufferGetPage(parent->buffer); - /* here we don't need to distinguish between split and page update */ if (parent->childoffnum == InvalidOffsetNumber || !XLByteEQ(parent->lsn, PageGetLSN(parent->page))) { @@ -811,6 +815,7 @@ gistFindCorrectParent(Relation r, GISTInsertStack *child) break; parent->buffer = ReadBuffer(r, parent->blkno); LockBuffer(parent->buffer, GIST_EXCLUSIVE); + gistcheckpage(r, parent->buffer); parent->page = (Page) BufferGetPage(parent->buffer); } @@ -831,7 +836,8 @@ gistFindCorrectParent(Relation r, GISTInsertStack *child) ptr = parent = gistFindPath(r, child->blkno, gistReadAndLockBuffer); Assert(ptr != NULL); - /* read all buffers as supposed in caller */ + /* read all buffers as expected by caller */ + /* note we don't lock them or gistcheckpage them here! */ while (ptr) { ptr->buffer = ReadBuffer(r, ptr->blkno); diff --git a/src/backend/access/gist/gistget.c b/src/backend/access/gist/gistget.c index 5ae48bd66e..bc8f9e0c07 100644 --- a/src/backend/access/gist/gistget.c +++ b/src/backend/access/gist/gistget.c @@ -8,7 +8,7 @@ * Portions Copyright (c) 1994, Regents of the University of California * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/access/gist/gistget.c,v 1.52 2005/10/06 02:29:07 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/access/gist/gistget.c,v 1.53 2005/11/06 22:39:20 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -40,6 +40,7 @@ killtuple(Relation r, GISTScanOpaque so, ItemPointer iptr) maxoff; LockBuffer(buffer, GIST_SHARE); + gistcheckpage(r, buffer); p = (Page) BufferGetPage(buffer); if (buffer == so->curbuf && XLByteEQ(so->stack->lsn, PageGetLSN(p))) @@ -176,6 +177,7 @@ gistnext(IndexScanDesc scan, ScanDirection dir, ItemPointer tids, int maxtids, b /* First of all, we need lock buffer */ Assert(so->curbuf != InvalidBuffer); LockBuffer(so->curbuf, GIST_SHARE); + gistcheckpage(scan->indexRelation, so->curbuf); p = BufferGetPage(so->curbuf); opaque = GistPageGetOpaque(p); resetoffset = false; @@ -224,7 +226,8 @@ gistnext(IndexScanDesc scan, ScanDirection dir, ItemPointer tids, int maxtids, b continue; } - if (!GistPageIsLeaf(p) || resetoffset || ItemPointerIsValid(&scan->currentItemData) == false) + if (!GistPageIsLeaf(p) || resetoffset || + !ItemPointerIsValid(&scan->currentItemData)) { if (ScanDirectionIsBackward(dir)) n = PageGetMaxOffsetNumber(p); @@ -268,7 +271,8 @@ gistnext(IndexScanDesc scan, ScanDirection dir, ItemPointer tids, int maxtids, b return ntids; } - so->curbuf = ReleaseAndReadBuffer(so->curbuf, scan->indexRelation, + so->curbuf = ReleaseAndReadBuffer(so->curbuf, + scan->indexRelation, stk->block); /* XXX go up */ break; diff --git a/src/backend/access/gist/gistutil.c b/src/backend/access/gist/gistutil.c index 16b1ceecf2..faf261b2af 100644 --- a/src/backend/access/gist/gistutil.c +++ b/src/backend/access/gist/gistutil.c @@ -8,7 +8,7 @@ * Portions Copyright (c) 1994, Regents of the University of California * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/access/gist/gistutil.c,v 1.7 2005/09/22 20:44:36 momjian Exp $ + * $PostgreSQL: pgsql/src/backend/access/gist/gistutil.c,v 1.8 2005/11/06 22:39:20 tgl Exp $ *------------------------------------------------------------------------- */ #include "postgres.h" @@ -798,23 +798,6 @@ gistpenalty(GISTSTATE *giststate, int attno, PointerGetDatum(penalty)); } -void -GISTInitBuffer(Buffer b, uint32 f) -{ - GISTPageOpaque opaque; - Page page; - Size pageSize; - - pageSize = BufferGetPageSize(b); - page = BufferGetPage(b); - PageInit(page, pageSize, sizeof(GISTPageOpaqueData)); - - opaque = GistPageGetOpaque(page); - opaque->flags = f; - opaque->rightlink = InvalidBlockNumber; - memset(&(opaque->nsn), 0, sizeof(GistNSN)); -} - void gistUserPicksplit(Relation r, GistEntryVector *entryvec, GIST_SPLITVEC *v, IndexTuple *itup, int len, GISTSTATE *giststate) @@ -864,36 +847,108 @@ gistUserPicksplit(Relation r, GistEntryVector *entryvec, GIST_SPLITVEC *v, } } +/* + * Initialize a new index page + */ +void +GISTInitBuffer(Buffer b, uint32 f) +{ + GISTPageOpaque opaque; + Page page; + Size pageSize; + + pageSize = BufferGetPageSize(b); + page = BufferGetPage(b); + PageInit(page, pageSize, sizeof(GISTPageOpaqueData)); + + opaque = GistPageGetOpaque(page); + opaque->flags = f; + opaque->rightlink = InvalidBlockNumber; + /* page was already zeroed by PageInit, so this is not needed: */ + /* memset(&(opaque->nsn), 0, sizeof(GistNSN)); */ +} + +/* + * Verify that a freshly-read page looks sane. + */ +void +gistcheckpage(Relation rel, Buffer buf) +{ + Page page = BufferGetPage(buf); + + /* + * ReadBuffer verifies that every newly-read page passes PageHeaderIsValid, + * which means it either contains a reasonably sane page header or is + * all-zero. We have to defend against the all-zero case, however. + */ + if (PageIsNew(page)) + ereport(ERROR, + (errcode(ERRCODE_INDEX_CORRUPTED), + errmsg("index \"%s\" contains unexpected zero page at block %u", + RelationGetRelationName(rel), + BufferGetBlockNumber(buf)), + errhint("Please REINDEX it."))); + + /* + * Additionally check that the special area looks sane. + */ + if (((PageHeader) (page))->pd_special != + (BLCKSZ - MAXALIGN(sizeof(GISTPageOpaqueData)))) + ereport(ERROR, + (errcode(ERRCODE_INDEX_CORRUPTED), + errmsg("index \"%s\" contains corrupted page at block %u", + RelationGetRelationName(rel), + BufferGetBlockNumber(buf)), + errhint("Please REINDEX it."))); +} + + +/* + * Allocate a new page (either by recycling, or by extending the index file) + * + * The returned buffer is already pinned and exclusive-locked + * + * Caller is responsible for initializing the page by calling GISTInitBuffer + */ Buffer gistNewBuffer(Relation r) { - Buffer buffer = InvalidBuffer; + Buffer buffer; bool needLock; - while (true) + /* First, try to get a page from FSM */ + for (;;) { BlockNumber blkno = GetFreeIndexPage(&r->rd_node); if (blkno == InvalidBlockNumber) - break; + break; /* nothing left in FSM */ buffer = ReadBuffer(r, blkno); + /* + * We have to guard against the possibility that someone else already + * recycled this page; the buffer may be locked if so. + */ if (ConditionalLockBuffer(buffer)) { Page page = BufferGetPage(buffer); + if (PageIsNew(page)) + return buffer; /* OK to use, if never initialized */ + + gistcheckpage(r, buffer); + if (GistPageIsDeleted(page)) - { - GistPageSetNonDeleted(page); - return buffer; - } - else - LockBuffer(buffer, GIST_UNLOCK); + return buffer; /* OK to use */ + + LockBuffer(buffer, GIST_UNLOCK); } + /* Can't use it, so release buffer and try again */ ReleaseBuffer(buffer); } + /* Must extend the file */ needLock = !RELATION_IS_LOCAL(r); if (needLock) diff --git a/src/backend/access/gist/gistvacuum.c b/src/backend/access/gist/gistvacuum.c index 11497c8093..60725e5e05 100644 --- a/src/backend/access/gist/gistvacuum.c +++ b/src/backend/access/gist/gistvacuum.c @@ -8,7 +8,7 @@ * Portions Copyright (c) 1994, Regents of the University of California * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/access/gist/gistvacuum.c,v 1.9 2005/09/22 20:44:36 momjian Exp $ + * $PostgreSQL: pgsql/src/backend/access/gist/gistvacuum.c,v 1.10 2005/11/06 22:39:20 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -65,6 +65,11 @@ gistVacuumUpdate(GistVacuum *gv, BlockNumber blkno, bool needunion) lencompleted = 16; buffer = ReadBuffer(gv->index, blkno); + /* + * This is only used during VACUUM FULL, so we need not bother to lock + * individual index pages + */ + gistcheckpage(gv->index, buffer); page = (Page) BufferGetPage(buffer); maxoff = PageGetMaxOffsetNumber(page); @@ -378,9 +383,10 @@ gistvacuumcleanup(PG_FUNCTION_ARGS) needFullVacuum = false; - needLock = !RELATION_IS_LOCAL(rel); if (info->vacuum_full) needLock = false; /* relation locked with AccessExclusiveLock */ + else + needLock = !RELATION_IS_LOCAL(rel); /* try to find deleted pages */ if (needLock) @@ -403,7 +409,7 @@ gistvacuumcleanup(PG_FUNCTION_ARGS) LockBuffer(buffer, GIST_SHARE); page = (Page) BufferGetPage(buffer); - if (GistPageIsDeleted(page)) + if (PageIsNew(page) || GistPageIsDeleted(page)) { if (nFreePages < maxFreePages) { @@ -513,6 +519,7 @@ gistbulkdelete(PG_FUNCTION_ARGS) ItemId iid; LockBuffer(buffer, GIST_SHARE); + gistcheckpage(rel, buffer); page = (Page) BufferGetPage(buffer); if (GistPageIsLeaf(page)) diff --git a/src/include/access/gist_private.h b/src/include/access/gist_private.h index 1cfa5b92bc..9ce5a860c4 100644 --- a/src/include/access/gist_private.h +++ b/src/include/access/gist_private.h @@ -7,7 +7,7 @@ * Portions Copyright (c) 1996-2005, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * $PostgreSQL: pgsql/src/include/access/gist_private.h,v 1.8 2005/10/15 02:49:42 momjian Exp $ + * $PostgreSQL: pgsql/src/include/access/gist_private.h,v 1.9 2005/11/06 22:39:21 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -263,6 +263,7 @@ extern Datum gistgettuple(PG_FUNCTION_ARGS); extern Datum gistgetmulti(PG_FUNCTION_ARGS); /* gistutil.c */ +extern void gistcheckpage(Relation rel, Buffer buf); extern Buffer gistNewBuffer(Relation r); extern OffsetNumber gistfillbuffer(Relation r, Page page, IndexTuple *itup, int len, OffsetNumber off);