mirror/postgresql
2
0
Fork 0
mirror of https://git.postgresql.org/git/postgresql.git synced 2024-12-27 08:39:28 +08:00
Code Issues Packages Projects Releases Wiki Activity

Fix possible duplicate tuples while GiST scan. Now page is processed

Browse Source 
at once and ItemPointers are collected in memory.

Remove tuple's killing by killtuple() if tuple was moved to another
page - it could produce unaceptable overhead.

Backpatch up to 8.1 because the bug was introduced by GiST's concurrency support.
This commit is contained in:
Teodor Sigaev 2008-08-23 10:37:24 +00:00
parent 42d313f546
commit 1dcf6fdf1b
3 changed files with 127 additions and 90 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

140
src/backend/access/gist/gistget.c
View File

@ -8,7 +8,7 @@
* Portions Copyright (c) 1994, Regents of the University of California * Portions Copyright (c) 1994, Regents of the University of California
* *
* IDENTIFICATION * IDENTIFICATION
* $PostgreSQL: pgsql/src/backend/access/gist/gistget.c,v 1.74 2008/06/19 00:46:03 alvherre Exp $ * $PostgreSQL: pgsql/src/backend/access/gist/gistget.c,v 1.75 2008/08/23 10:37:24 teodor Exp $
* *
*------------------------------------------------------------------------- *-------------------------------------------------------------------------
*/ */
@ -32,30 +32,23 @@ static bool gistindex_keytest(IndexTuple tuple, IndexScanDesc scan,
static void static void
killtuple(Relation r, GISTScanOpaque so, ItemPointer iptr) killtuple(Relation r, GISTScanOpaque so, ItemPointer iptr)
{ {
Buffer buffer = so->curbuf;
for (;;)
{
Page p; Page p;
BlockNumber blkno; OffsetNumber offset;
OffsetNumber offset,
maxoff;
LockBuffer(buffer, GIST_SHARE); LockBuffer(so->curbuf, GIST_SHARE);
gistcheckpage(r, buffer); gistcheckpage(r, so->curbuf);
p = (Page) BufferGetPage(buffer); p = (Page) BufferGetPage(so->curbuf);
if (buffer == so->curbuf && XLByteEQ(so->stack->lsn, PageGetLSN(p))) if (XLByteEQ(so->stack->lsn, PageGetLSN(p)))
{ {
/* page unchanged, so all is simple */ /* page unchanged, so all is simple */
offset = ItemPointerGetOffsetNumber(iptr); offset = ItemPointerGetOffsetNumber(iptr);
ItemIdMarkDead(PageGetItemId(p, offset)); ItemIdMarkDead(PageGetItemId(p, offset));
SetBufferCommitInfoNeedsSave(buffer); SetBufferCommitInfoNeedsSave(so->curbuf);
LockBuffer(buffer, GIST_UNLOCK);
break;
} }
else
maxoff = PageGetMaxOffsetNumber(p); {
OffsetNumber maxoff = PageGetMaxOffsetNumber(p);
for (offset = FirstOffsetNumber; offset <= maxoff; offset = OffsetNumberNext(offset)) for (offset = FirstOffsetNumber; offset <= maxoff; offset = OffsetNumberNext(offset))
{ {
@ -65,30 +58,13 @@ killtuple(Relation r, GISTScanOpaque so, ItemPointer iptr)
{ {
/* found */ /* found */
ItemIdMarkDead(PageGetItemId(p, offset)); ItemIdMarkDead(PageGetItemId(p, offset));
SetBufferCommitInfoNeedsSave(buffer); SetBufferCommitInfoNeedsSave(so->curbuf);
LockBuffer(buffer, GIST_UNLOCK); break;
if (buffer != so->curbuf) }
ReleaseBuffer(buffer);
return;
} }
} }
/* follow right link */ LockBuffer(so->curbuf, GIST_UNLOCK);
/*
* ??? is it good? if tuple dropped by concurrent vacuum, we will read
* all leaf pages...
*/
blkno = GistPageGetOpaque(p)->rightlink;
LockBuffer(buffer, GIST_UNLOCK);
if (buffer != so->curbuf)
ReleaseBuffer(buffer);
if (blkno == InvalidBlockNumber)
/* can't found, dropped by somebody else */
return;
buffer = ReadBuffer(r, blkno);
}
} }
/* /*
@ -154,7 +130,6 @@ gistnext(IndexScanDesc scan, ScanDirection dir, TIDBitmap *tbm)
GISTSearchStack *stk; GISTSearchStack *stk;
IndexTuple it; IndexTuple it;
GISTPageOpaque opaque; GISTPageOpaque opaque;
bool resetoffset = false;
int64 ntids = 0; int64 ntids = 0;
so = (GISTScanOpaque) scan->opaque; so = (GISTScanOpaque) scan->opaque;
@ -179,6 +154,50 @@ gistnext(IndexScanDesc scan, ScanDirection dir, TIDBitmap *tbm)
return 0; return 0;
} }
/*
* check stored pointers from last visit
*/
if ( so->nPageData > 0 )
{
/*
* gistgetmulti never should go here
*/
Assert( tbm == NULL );
if ( so->curPageData < so->nPageData )
{
/*
* pageData is already ordered for scan's direction
*/
scan->xs_ctup.t_self = so->pageData[ so->curPageData ].iptr;
scan->xs_recheck = so->pageData[ so->curPageData ].recheck;
so->curPageData ++;
return 1;
}
else
{
/*
* Go to the next page
*/
stk = so->stack->next;
pfree(so->stack);
so->stack = stk;
/* If we're out of stack entries, we're done */
if (so->stack == NULL)
{
ReleaseBuffer(so->curbuf);
so->curbuf = InvalidBuffer;
return 0;
}
so->curbuf = ReleaseAndReadBuffer(so->curbuf,
scan->indexRelation,
stk->block);
}
}
for (;;) for (;;)
{ {
CHECK_FOR_INTERRUPTS(); CHECK_FOR_INTERRUPTS();
@ -189,15 +208,11 @@ gistnext(IndexScanDesc scan, ScanDirection dir, TIDBitmap *tbm)
gistcheckpage(scan->indexRelation, so->curbuf); gistcheckpage(scan->indexRelation, so->curbuf);
p = BufferGetPage(so->curbuf); p = BufferGetPage(so->curbuf);
opaque = GistPageGetOpaque(p); opaque = GistPageGetOpaque(p);
resetoffset = false;
if (XLogRecPtrIsInvalid(so->stack->lsn) || !XLByteEQ(so->stack->lsn, PageGetLSN(p))) /* remember lsn to identify page changed for tuple's killing */
{
/* first visit or page changed from last visit, reset offset */
so->stack->lsn = PageGetLSN(p); so->stack->lsn = PageGetLSN(p);
resetoffset = true;
/* check page split, occured from last visit or visit to parent */ /* check page split, occured since visit to parent */
if (!XLogRecPtrIsInvalid(so->stack->parentlsn) && if (!XLogRecPtrIsInvalid(so->stack->parentlsn) &&
XLByteLT(so->stack->parentlsn, opaque->nsn) && XLByteLT(so->stack->parentlsn, opaque->nsn) &&
opaque->rightlink != InvalidBlockNumber /* sanity check */ && opaque->rightlink != InvalidBlockNumber /* sanity check */ &&
@ -213,7 +228,6 @@ gistnext(IndexScanDesc scan, ScanDirection dir, TIDBitmap *tbm)
memset(&(stk->lsn), 0, sizeof(GistNSN)); memset(&(stk->lsn), 0, sizeof(GistNSN));
so->stack->next = stk; so->stack->next = stk;
} }
}
/* if page is empty, then just skip it */ /* if page is empty, then just skip it */
if (PageIsEmpty(p)) if (PageIsEmpty(p))
@ -235,25 +249,13 @@ gistnext(IndexScanDesc scan, ScanDirection dir, TIDBitmap *tbm)
continue; continue;
} }
if (!GistPageIsLeaf(p) || resetoffset ||
!ItemPointerIsValid(&so->curpos))
{
if (ScanDirectionIsBackward(dir)) if (ScanDirectionIsBackward(dir))
n = PageGetMaxOffsetNumber(p); n = PageGetMaxOffsetNumber(p);
else else
n = FirstOffsetNumber; n = FirstOffsetNumber;
}
else
{
n = ItemPointerGetOffsetNumber(&(so->curpos));
if (ScanDirectionIsBackward(dir))
n = OffsetNumberPrev(n);
else
n = OffsetNumberNext(n);
}
/* wonderful, we can look at page */ /* wonderful, we can look at page */
so->nPageData = so->curPageData = 0;
for (;;) for (;;)
{ {
@ -261,6 +263,18 @@ gistnext(IndexScanDesc scan, ScanDirection dir, TIDBitmap *tbm)
if (!OffsetNumberIsValid(n)) if (!OffsetNumberIsValid(n))
{ {
/*
* If we was called from gistgettuple and current buffer contains
* something matched then make a recursive call - it will return
* ItemPointer from so->pageData. But we save buffer pinned to
* support tuple's killing
*/
if ( !tbm && so->nPageData > 0 )
{
LockBuffer(so->curbuf, GIST_UNLOCK);
return gistnext(scan, dir, NULL);
}
/* /*
* We ran out of matching index entries on the current page, * We ran out of matching index entries on the current page,
* so pop the top stack entry and use it to continue the * so pop the top stack entry and use it to continue the
@ -306,11 +320,9 @@ gistnext(IndexScanDesc scan, ScanDirection dir, TIDBitmap *tbm)
tbm_add_tuples(tbm, &it->t_tid, 1, scan->xs_recheck); tbm_add_tuples(tbm, &it->t_tid, 1, scan->xs_recheck);
else else
{ {
scan->xs_ctup.t_self = it->t_tid; so->pageData[ so->nPageData ].iptr = it->t_tid;
/* scan->xs_recheck is already set */ so->pageData[ so->nPageData ].recheck = scan->xs_recheck;
so->nPageData ++;
LockBuffer(so->curbuf, GIST_UNLOCK);
return ntids; /* always 1 */
} }
} }
} }

14
src/backend/access/gist/gistscan.c
View File

@ -8,7 +8,7 @@
* Portions Copyright (c) 1994, Regents of the University of California * Portions Copyright (c) 1994, Regents of the University of California
* *
* IDENTIFICATION * IDENTIFICATION
* $PostgreSQL: pgsql/src/backend/access/gist/gistscan.c,v 1.70 2008/06/19 00:46:03 alvherre Exp $ * $PostgreSQL: pgsql/src/backend/access/gist/gistscan.c,v 1.71 2008/08/23 10:37:24 teodor Exp $
* *
*------------------------------------------------------------------------- *-------------------------------------------------------------------------
*/ */
@ -63,6 +63,7 @@ gistrescan(PG_FUNCTION_ARGS)
ReleaseBuffer(so->markbuf); ReleaseBuffer(so->markbuf);
so->markbuf = InvalidBuffer; so->markbuf = InvalidBuffer;
} }
} }
else else
{ {
@ -83,6 +84,7 @@ gistrescan(PG_FUNCTION_ARGS)
*/ */
ItemPointerSetInvalid(&so->curpos); ItemPointerSetInvalid(&so->curpos);
ItemPointerSetInvalid(&so->markpos); ItemPointerSetInvalid(&so->markpos);
so->nPageData = so->curPageData = 0;
/* Update scan key, if a new one is given */ /* Update scan key, if a new one is given */
if (key && scan->numberOfKeys > 0) if (key && scan->numberOfKeys > 0)
@ -150,6 +152,11 @@ gistmarkpos(PG_FUNCTION_ARGS)
so->markbuf = so->curbuf; so->markbuf = so->curbuf;
} }
so->markNPageData = so->nPageData;
so->markCurPageData = so->curPageData;
if ( so->markNPageData > 0 )
memcpy( so->markPageData, so->pageData, sizeof(ItemResult) * so->markNPageData );
PG_RETURN_VOID(); PG_RETURN_VOID();
} }
@ -199,6 +206,11 @@ gistrestrpos(PG_FUNCTION_ARGS)
so->curbuf = so->markbuf; so->curbuf = so->markbuf;
} }
so->nPageData = so->markNPageData;
so->curPageData = so->markNPageData;
if ( so->markNPageData > 0 )
memcpy( so->pageData, so->markPageData, sizeof(ItemResult) * so->markNPageData );
PG_RETURN_VOID(); PG_RETURN_VOID();
} }

15
src/include/access/gist_private.h
View File

@ -7,7 +7,7 @@
* Portions Copyright (c) 1996-2008, PostgreSQL Global Development Group * Portions Copyright (c) 1996-2008, PostgreSQL Global Development Group
* Portions Copyright (c) 1994, Regents of the University of California * Portions Copyright (c) 1994, Regents of the University of California
* *
* $PostgreSQL: pgsql/src/include/access/gist_private.h,v 1.31 2008/06/19 00:46:05 alvherre Exp $ * $PostgreSQL: pgsql/src/include/access/gist_private.h,v 1.32 2008/08/23 10:37:24 teodor Exp $
* *
*------------------------------------------------------------------------- *-------------------------------------------------------------------------
*/ */
@ -58,6 +58,12 @@ typedef struct GISTSTATE
TupleDesc tupdesc; TupleDesc tupdesc;
} GISTSTATE; } GISTSTATE;
typedef struct ItemResult
{
ItemPointerData iptr;
bool recheck;
} ItemResult;
/* /*
* When we're doing a scan, we need to keep track of the parent stack * When we're doing a scan, we need to keep track of the parent stack
* for the marked and current items. * for the marked and current items.
@ -73,6 +79,13 @@ typedef struct GISTScanOpaqueData
ItemPointerData curpos; ItemPointerData curpos;
Buffer markbuf; Buffer markbuf;
ItemPointerData markpos; ItemPointerData markpos;
ItemResult pageData[BLCKSZ/sizeof(IndexTupleData)];
OffsetNumber nPageData;
OffsetNumber curPageData;
ItemResult markPageData[BLCKSZ/sizeof(IndexTupleData)];
OffsetNumber markNPageData;
OffsetNumber markCurPageData;
} GISTScanOpaqueData; } GISTScanOpaqueData;
typedef GISTScanOpaqueData *GISTScanOpaque; typedef GISTScanOpaqueData *GISTScanOpaque;