rdelaage
/
postgresql
mirror of https://git.postgresql.org/git/postgresql.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Fix deadlock in GIN vacuum introduced by 218f51584d 

Before 218f51584d if posting tree page is about to be deleted, then the whole
posting tree is locked by LockBufferForCleanup() on root preventing all the
concurrent inserts.  218f51584d reduced locking to the subtree containing
page to be deleted.  However, due to concurrent parent split, inserter doesn't
always holds pins on all the pages constituting path from root to the target
leaf page.  That could cause a deadlock between GIN vacuum process and GIN
inserter.  And we didn't find non-invasive way to fix this.

This commit reverts VACUUM behavior to lock the whole posting tree before
delete any page.  However, we keep another useful change by 218f51584d5: the
tree is locked only if there are pages to be deleted.

Reported-by: Chen Huajun
Diagnosed-by: Chen Huajun, Andrey Borodin, Peter Geoghegan
Discussion: https://postgr.es/m/31a702a.14dd.166c1366ac1.Coremail.chjischj%40163.com
Author: Alexander Korotkov, based on ideas from Andrey Borodin and Peter Geoghegan
Reviewed-by: Andrey Borodin
Backpatch-through: 10
This commit is contained in:
Alexander Korotkov 2018-12-13 06:12:11 +03:00
parent 77d4d88afb
commit fd83c83d09
2 changed files with 80 additions and 97 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

15
src/backend/access/gin/README
View File

@ -314,17 +314,10 @@ deleted.
The previous paragraph's reasoning only applies to searches, and only to
posting trees. To protect from inserters following a downlink to a deleted
page, vacuum simply locks out all concurrent insertions to the posting tree,
by holding a super-exclusive lock on the parent page of subtree with deletable
pages. Inserters hold a pin on the root page, but searches do not, so while
new searches cannot begin while root page is locked, any already-in-progress
scans can continue concurrently with vacuum in corresponding subtree of
posting tree. To exclude interference with readers vacuum takes exclusive
locks in a depth-first scan in left-to-right order of page tuples. Leftmost
page is never deleted. Thus before deleting any page we obtain exclusive
lock on any left page, effectively excluding deadlock with any reader, despite
taking parent lock before current and left lock after current. We take left
lock not for a concurrency reasons, but rather in need to mark page dirty.
In the entry tree, we never delete pages.
by holding a super-exclusive lock on the posting tree root. Inserters hold a
pin on the root page, but searches do not, so while new searches cannot begin
while root page is locked, any already-in-progress scans can continue
concurrently with vacuum. In the entry tree, we never delete pages.
(This is quite different from the mechanism the btree indexam uses to make
page-deletions safe; it stamps the deleted pages with an XID and keeps the

162
src/backend/access/gin/ginvacuum.c
View File

@ -315,122 +315,112 @@ ginScanToDelete(GinVacuumState *gvs, BlockNumber blkno, bool isRoot,
/*
* Scan through posting tree, delete empty tuples from leaf pages.
* Also, this function collects empty subtrees (with all empty leafs).
* For parents of these subtrees CleanUp lock is taken, then we call
* ScanToDelete. This is done for every inner page, which points to
* empty subtree.
* Scan through posting tree leafs, delete empty tuples. Returns true if there
* is at least one empty page.
*/
static bool
ginVacuumPostingTreeLeaves(GinVacuumState *gvs, BlockNumber blkno, bool isRoot)
ginVacuumPostingTreeLeaves(GinVacuumState *gvs, BlockNumber blkno)
{
Buffer buffer;
Page page;
bool hasVoidPage = false;
MemoryContext oldCxt;
buffer = ReadBufferExtended(gvs->index, MAIN_FORKNUM, blkno,
RBM_NORMAL, gvs->strategy);
page = BufferGetPage(buffer);
/* Find leftmost leaf page of posting tree and lock it in exclusive mode */
while (true)
{
PostingItem *pitem;
ginTraverseLock(buffer, false);
buffer = ReadBufferExtended(gvs->index, MAIN_FORKNUM, blkno,
RBM_NORMAL, gvs->strategy);
LockBuffer(buffer, GIN_SHARE);
page = BufferGetPage(buffer);
Assert(GinPageIsData(page));
Assert(GinPageIsData(page));
if (GinPageIsLeaf(page))
if (GinPageIsLeaf(page))
{
LockBuffer(buffer, GIN_UNLOCK);
LockBuffer(buffer, GIN_EXCLUSIVE);
break;
}
Assert(PageGetMaxOffsetNumber(page) >= FirstOffsetNumber);
pitem = GinDataPageGetPostingItem(page, FirstOffsetNumber);
blkno = PostingItemGetBlockNumber(pitem);
Assert(blkno != InvalidBlockNumber);
UnlockReleaseBuffer(buffer);
}
/* Iterate all posting tree leaves using rightlinks and vacuum them */
while (true)
{
oldCxt = MemoryContextSwitchTo(gvs->tmpCxt);
ginVacuumPostingTreeLeaf(gvs->index, buffer, gvs);
MemoryContextSwitchTo(oldCxt);
MemoryContextReset(gvs->tmpCxt);
/* if root is a leaf page, we don't desire further processing */
if (GinDataLeafPageIsEmpty(page))
hasVoidPage = true;
UnlockReleaseBuffer(buffer);
return hasVoidPage;
}
else
{
OffsetNumber i;
bool hasEmptyChild = false;
bool hasNonEmptyChild = false;
OffsetNumber maxoff = GinPageGetOpaque(page)->maxoff;
BlockNumber *children = palloc(sizeof(BlockNumber) * (maxoff + 1));
/*
* Read all children BlockNumbers. Not sure it is safe if there are
* many concurrent vacuums.
*/
for (i = FirstOffsetNumber; i <= maxoff; i++)
{
PostingItem *pitem = GinDataPageGetPostingItem(page, i);
children[i] = PostingItemGetBlockNumber(pitem);
}
blkno = GinPageGetOpaque(page)->rightlink;
UnlockReleaseBuffer(buffer);
for (i = FirstOffsetNumber; i <= maxoff; i++)
{
if (ginVacuumPostingTreeLeaves(gvs, children[i], false))
hasEmptyChild = true;
else
hasNonEmptyChild = true;
}
if (blkno == InvalidBlockNumber)
break;
pfree(children);
vacuum_delay_point();
/*
* All subtree is empty - just return true to indicate that parent
* must do a cleanup, unless we are ROOT and there is way to go upper.
*/
if (hasEmptyChild && !hasNonEmptyChild && !isRoot)
return true;
if (hasEmptyChild)
{
DataPageDeleteStack root,
*ptr,
*tmp;
buffer = ReadBufferExtended(gvs->index, MAIN_FORKNUM, blkno,
RBM_NORMAL, gvs->strategy);
LockBufferForCleanup(buffer);
memset(&root, 0, sizeof(DataPageDeleteStack));
root.leftBlkno = InvalidBlockNumber;
root.isRoot = true;
ginScanToDelete(gvs, blkno, true, &root, InvalidOffsetNumber);
ptr = root.child;
while (ptr)
{
tmp = ptr->child;
pfree(ptr);
ptr = tmp;
}
UnlockReleaseBuffer(buffer);
}
/* Here we have deleted all empty subtrees */
return false;
buffer = ReadBufferExtended(gvs->index, MAIN_FORKNUM, blkno,
RBM_NORMAL, gvs->strategy);
LockBuffer(buffer, GIN_EXCLUSIVE);
page = BufferGetPage(buffer);
}
return hasVoidPage;
}
static void
ginVacuumPostingTree(GinVacuumState *gvs, BlockNumber rootBlkno)
{
ginVacuumPostingTreeLeaves(gvs, rootBlkno, true);
if (ginVacuumPostingTreeLeaves(gvs, rootBlkno))
{
/*
* There is at least one empty page. So we have to rescan the tree
* deleting empty pages.
*/
Buffer buffer;
DataPageDeleteStack root,
*ptr,
*tmp;
buffer = ReadBufferExtended(gvs->index, MAIN_FORKNUM, rootBlkno,
RBM_NORMAL, gvs->strategy);
/*
* Lock posting tree root for cleanup to ensure there are no concurrent
* inserts.
*/
LockBufferForCleanup(buffer);
memset(&root, 0, sizeof(DataPageDeleteStack));
root.leftBlkno = InvalidBlockNumber;
root.isRoot = true;
ginScanToDelete(gvs, rootBlkno, true, &root, InvalidOffsetNumber);
ptr = root.child;
while (ptr)
{
tmp = ptr->child;
pfree(ptr);
ptr = tmp;
}
UnlockReleaseBuffer(buffer);
}
}
/*