From ac4ab97ec05ea900db0f14d428cae2e79832e02d Mon Sep 17 00:00:00 2001 From: Heikki Linnakangas Date: Fri, 8 Nov 2013 22:21:42 +0200 Subject: Fix race condition in GIN posting tree page deletion. If a page is deleted, and reused for something else, just as a search is following a rightlink to it from its left sibling, the search would continue scanning whatever the new contents of the page are. That could lead to incorrect query results, or even something more curious if the page is reused for a different kind of a page. To fix, modify the search algorithm to lock the next page before releasing the previous one, and refrain from deleting pages from the leftmost branch of the tree. Add a new Concurrency section to the README, explaining why this works. There is a lot more one could say about concurrency in GIN, but that's for another patch. Backpatch to all supported versions. --- src/backend/access/gin/ginget.c | 31 ++++++++----------------------- 1 file changed, 8 insertions(+), 23 deletions(-) (limited to 'src/backend/access/gin/ginget.c') diff --git a/src/backend/access/gin/ginget.c b/src/backend/access/gin/ginget.c index cb779aa7f74..acbdc5fe7f2 100644 --- a/src/backend/access/gin/ginget.c +++ b/src/backend/access/gin/ginget.c @@ -105,16 +105,11 @@ moveRightIfItNeeded(GinBtreeData *btree, GinBtreeStack *stack) /* * We scanned the whole page, so we should take right page */ - stack->blkno = GinPageGetOpaque(page)->rightlink; - if (GinPageRightMost(page)) return false; /* no more pages */ - LockBuffer(stack->buffer, GIN_UNLOCK); - stack->buffer = ReleaseAndReadBuffer(stack->buffer, - btree->index, - stack->blkno); - LockBuffer(stack->buffer, GIN_SHARE); + stack->buffer = ginStepRight(stack->buffer, btree->index, GIN_SHARE); + stack->blkno = BufferGetBlockNumber(stack->buffer); stack->off = FirstOffsetNumber; } @@ -132,7 +127,6 @@ scanPostingTree(Relation index, GinScanEntry scanEntry, GinPostingTreeScan *gdi; Buffer buffer; Page page; - BlockNumber blkno; /* Descend to the leftmost leaf page */ gdi = ginPrepareScanPostingTree(index, rootPostingTree, TRUE); @@ -162,10 +156,7 @@ scanPostingTree(Relation index, GinScanEntry scanEntry, if (GinPageRightMost(page)) break; /* no more pages */ - blkno = GinPageGetOpaque(page)->rightlink; - LockBuffer(buffer, GIN_UNLOCK); - buffer = ReleaseAndReadBuffer(buffer, index, blkno); - LockBuffer(buffer, GIN_SHARE); + buffer = ginStepRight(buffer, index, GIN_SHARE); } UnlockReleaseBuffer(buffer); @@ -543,7 +534,6 @@ static void entryGetNextItem(GinState *ginstate, GinScanEntry entry) { Page page; - BlockNumber blkno; for (;;) { @@ -561,23 +551,18 @@ entryGetNextItem(GinState *ginstate, GinScanEntry entry) * It's needed to go by right link. During that we should refind * first ItemPointer greater that stored */ - - blkno = GinPageGetOpaque(page)->rightlink; - - LockBuffer(entry->buffer, GIN_UNLOCK); - if (blkno == InvalidBlockNumber) + if (GinPageRightMost(page)) { - ReleaseBuffer(entry->buffer); + UnlockReleaseBuffer(entry->buffer); ItemPointerSetInvalid(&entry->curItem); entry->buffer = InvalidBuffer; entry->isFinished = TRUE; return; } - entry->buffer = ReleaseAndReadBuffer(entry->buffer, - ginstate->index, - blkno); - LockBuffer(entry->buffer, GIN_SHARE); + entry->buffer = ginStepRight(entry->buffer, + ginstate->index, + GIN_SHARE); page = BufferGetPage(entry->buffer); entry->offset = InvalidOffsetNumber; -- cgit v1.2.3