rdelaage/postgresql
1
0
Fork 0
mirror of https://git.postgresql.org/git/postgresql.git synced 2024-09-06 12:09:20 +02:00
Code Issues Packages Projects Releases Wiki Activity

Fix deadlock between ginDeletePage() and ginStepRight()

Browse Source 
When ginDeletePage() is about to delete page it locks its left sibling to revise
the rightlink.  So, it locks pages in right to left manner.  Int he same time
ginStepRight() locks pages in left to right manner, and that could cause a
deadlock.

This commit makes ginScanToDelete() keep exclusive lock on left siblings of
currently investigated path.  That elimites need to relock left sibling in
ginDeletePage().  Thus, deadlock with ginStepRight() can't happen anymore.

Reported-by: Chen Huajun
Discussion: https://postgr.es/m/5c332bd1.87b6.16d7c17aa98.Coremail.chjischj%40163.com
Author: Alexander Korotkov
Reviewed-by: Peter Geoghegan
Backpatch-through: 10
This commit is contained in:
Alexander Korotkov 2019-11-19 23:07:36 +03:00
parent 823a551fe0
commit 051c50c01c
2 changed files with 80 additions and 51 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -277,50 +277,63 @@ followed by the packed items.
Concurrency Concurrency
----------- -----------
The entry tree and each posting tree is a B-tree, with right-links connecting The entry tree and each posting tree are B-trees, with right-links connecting
sibling pages at the same level. This is the same structure that is used in sibling pages at the same level. This is the same structure that is used in
the regular B-tree indexam (invented by Lehman & Yao), but we don't support the regular B-tree indexam (invented by Lehman & Yao), but we don't support
scanning a GIN trees backwards, so we don't need left-links. scanning a GIN trees backwards, so we don't need left-links.
To avoid deadlocks, B-tree pages must always be locked in the same order: To avoid deadlocks, B-tree pages must always be locked in the same order:
left to right, and bottom to top. When searching, the tree is traversed from left to right, and bottom to top. The exception is page deletion during vacuum,
top to bottom, so the lock on the parent page must be released before which would be considered separately. When searching, the tree is traversed
from top to bottom, so the lock on the parent page must be released before
descending to the next level. Concurrent page splits move the keyspace to descending to the next level. Concurrent page splits move the keyspace to
right, so after following a downlink, the page actually containing the key right, so after following a downlink, the page actually containing the key
we're looking for might be somewhere to the right of the page we landed on. we're looking for might be somewhere to the right of the page we landed on.
In that case, we follow the right-links until we find the page we're looking In that case, we follow the right-links until we find the page we're looking
for. for. In spite of searches, insertions keeps pins on all pages in path from
root to the current page. These pages potentially requies downlink insertion,
while pins prevent them from being deleted.
To delete a page, the page's left sibling, the target page, and its parent, Vacuum never deletes tuples or pages from the entry tree. It traverses entry
are locked in that order, and the page is marked as deleted. However, a tree leafs in logical order by rightlinks and removes deletable TIDs from
concurrent search might already have read a pointer to the page, and might be posting lists. Posting trees are processed by links from entry tree leafs. They
just about to follow it. A page can be reached via the right-link of its left are vacuumed in two stages. At first stage, deletable TIDs are removed from
sibling, or via its downlink in the parent. leafs. If first stage detects at least one empty page, then at the second stage
ginScanToDelete() deletes empty pages.
ginScanToDelete() scans posting tree to delete empty pages, while vacuum holds
cleanup lock on the posting tree root. This lock prevent concurrent inserts,
because inserters hold a pin on the root page. In spite of inserters searches
don't hold pin on root page. So, while new searches cannot begin while root page
is locked, any already-in-progress scans can continue concurrently with vacuum.
ginScanToDelete() does depth-first tree scanning while keeping each page in path
from root to current page exclusively locked. It also keeps left sibling of
each page in the path locked. Thus, if current page is to be removed, all
required pages to remove both downlink and rightlink are already locked.
Therefore, page deletion locks pages top to bottom and left to right breaking
our general rule. But assuming there is no concurrent insertions, this can't
cause a deadlock.
During replay od page deletion at standby, the page's left sibling, the target
page, and its parent, are locked in that order. So it follows bottom to top and
left to right rule.
A search concurrent to page deletion might already have read a pointer to the
page to be deleted, and might be just about to follow it. A page can be reached
via the right-link of its left sibling, or via its downlink in the parent.
To prevent a backend from reaching a deleted page via a right-link, when To prevent a backend from reaching a deleted page via a right-link, when
following a right-link the lock on the previous page is not released until following a right-link the lock on the previous page is not released until the
the lock on next page has been acquired. lock on next page has been acquired.
The downlink is more tricky. A search descending the tree must release the The downlink is more tricky. A search descending the tree must release the lock
lock on the parent page before locking the child, or it could deadlock with on the parent page before locking the child, or it could deadlock with a
a concurrent split of the child page; a page split locks the parent, while concurrent split of the child page; a page split locks the parent, while already
already holding a lock on the child page. So, deleted page cannot be reclaimed holding a lock on the child page. So, deleted page cannot be reclaimed
immediately. Instead, we have to wait for every transaction, which might wait immediately. Instead, we have to wait for every transaction, which might wait to
to reference this page, to finish. Corresponding processes must observe that reference this page, to finish. Corresponding processes must observe that the
the page is marked deleted and recover accordingly. page is marked deleted and recover accordingly.
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 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
deleted pages around with the right-link intact until all concurrent scans
have finished.)
Predicate Locking Predicate Locking
----------------- -----------------

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

@ -117,7 +117,8 @@ typedef struct DataPageDeleteStack
struct DataPageDeleteStack *parent; struct DataPageDeleteStack *parent;
BlockNumber blkno; /* current block number */ BlockNumber blkno; /* current block number */
BlockNumber leftBlkno; /* rightest non-deleted page on left */ Buffer leftBuffer; /* pinned and locked rightest non-deleted page
* on left */
bool isRoot; bool isRoot;
} DataPageDeleteStack; } DataPageDeleteStack;
@ -139,11 +140,8 @@ ginDeletePage(GinVacuumState *gvs, BlockNumber deleteBlkno, BlockNumber leftBlkn
/* /*
* This function MUST be called only if someone of parent pages hold * This function MUST be called only if someone of parent pages hold
* exclusive cleanup lock. This guarantees that no insertions currently * exclusive cleanup lock. This guarantees that no insertions currently
* happen in this subtree. Caller also acquire Exclusive lock on deletable * happen in this subtree. Caller also acquires Exclusive locks on
* page and is acquiring and releasing exclusive lock on left page before. * deletable, parent and left pages.
* Left page was locked and released. Then parent and this page are
* locked. We acquire left page lock here only to mark page dirty after
* changing right pointer.
*/ */
lBuffer = ReadBufferExtended(gvs->index, MAIN_FORKNUM, leftBlkno, lBuffer = ReadBufferExtended(gvs->index, MAIN_FORKNUM, leftBlkno,
RBM_NORMAL, gvs->strategy); RBM_NORMAL, gvs->strategy);
@ -152,8 +150,6 @@ ginDeletePage(GinVacuumState *gvs, BlockNumber deleteBlkno, BlockNumber leftBlkn
pBuffer = ReadBufferExtended(gvs->index, MAIN_FORKNUM, parentBlkno, pBuffer = ReadBufferExtended(gvs->index, MAIN_FORKNUM, parentBlkno,
RBM_NORMAL, gvs->strategy); RBM_NORMAL, gvs->strategy);
LockBuffer(lBuffer, GIN_EXCLUSIVE);
page = BufferGetPage(dBuffer); page = BufferGetPage(dBuffer);
rightlink = GinPageGetOpaque(page)->rightlink; rightlink = GinPageGetOpaque(page)->rightlink;
@ -227,7 +223,7 @@ ginDeletePage(GinVacuumState *gvs, BlockNumber deleteBlkno, BlockNumber leftBlkn
} }
ReleaseBuffer(pBuffer); ReleaseBuffer(pBuffer);
UnlockReleaseBuffer(lBuffer); ReleaseBuffer(lBuffer);
ReleaseBuffer(dBuffer); ReleaseBuffer(dBuffer);
END_CRIT_SECTION(); END_CRIT_SECTION();
@ -237,7 +233,11 @@ ginDeletePage(GinVacuumState *gvs, BlockNumber deleteBlkno, BlockNumber leftBlkn
/* /*
* scans posting tree and deletes empty pages * Scans posting tree and deletes empty pages. Caller must lock root page for
* cleanup. During scan path from root to current page is kept exclusively
* locked. Also keep left page exclusively locked, because ginDeletePage()
* needs it. If we try to relock left page later, it could deadlock with
* ginStepRight().
*/ */
static bool static bool
ginScanToDelete(GinVacuumState *gvs, BlockNumber blkno, bool isRoot, ginScanToDelete(GinVacuumState *gvs, BlockNumber blkno, bool isRoot,
@ -260,7 +260,7 @@ ginScanToDelete(GinVacuumState *gvs, BlockNumber blkno, bool isRoot,
me = (DataPageDeleteStack *) palloc0(sizeof(DataPageDeleteStack)); me = (DataPageDeleteStack *) palloc0(sizeof(DataPageDeleteStack));
me->parent = parent; me->parent = parent;
parent->child = me; parent->child = me;
me->leftBlkno = InvalidBlockNumber; me->leftBuffer = InvalidBuffer;
} }
else else
me = parent->child; me = parent->child;
@ -288,6 +288,12 @@ ginScanToDelete(GinVacuumState *gvs, BlockNumber blkno, bool isRoot,
if (ginScanToDelete(gvs, PostingItemGetBlockNumber(pitem), false, me, i)) if (ginScanToDelete(gvs, PostingItemGetBlockNumber(pitem), false, me, i))
i--; i--;
} }
if (GinPageRightMost(page) && BufferIsValid(me->child->leftBuffer))
{
UnlockReleaseBuffer(me->child->leftBuffer);
me->child->leftBuffer = InvalidBuffer;
}
} }
if (GinPageIsLeaf(page)) if (GinPageIsLeaf(page))
@ -298,21 +304,31 @@ ginScanToDelete(GinVacuumState *gvs, BlockNumber blkno, bool isRoot,
if (isempty) if (isempty)
{ {
/* we never delete the left- or rightmost branch */ /* we never delete the left- or rightmost branch */
if (me->leftBlkno != InvalidBlockNumber && !GinPageRightMost(page)) if (BufferIsValid(me->leftBuffer) && !GinPageRightMost(page))
{ {
Assert(!isRoot); Assert(!isRoot);
ginDeletePage(gvs, blkno, me->leftBlkno, me->parent->blkno, myoff, me->parent->isRoot); ginDeletePage(gvs, blkno, BufferGetBlockNumber(me->leftBuffer),
me->parent->blkno, myoff, me->parent->isRoot);
meDelete = true; meDelete = true;
} }
} }
if (!isRoot)
LockBuffer(buffer, GIN_UNLOCK);
ReleaseBuffer(buffer);
if (!meDelete) if (!meDelete)
me->leftBlkno = blkno; {
if (BufferIsValid(me->leftBuffer))
UnlockReleaseBuffer(me->leftBuffer);
me->leftBuffer = buffer;
}
else
{
if (!isRoot)
LockBuffer(buffer, GIN_UNLOCK);
ReleaseBuffer(buffer);
}
if (isRoot)
ReleaseBuffer(buffer);
return meDelete; return meDelete;
} }
@ -409,7 +425,7 @@ ginVacuumPostingTree(GinVacuumState *gvs, BlockNumber rootBlkno)
LockBufferForCleanup(buffer); LockBufferForCleanup(buffer);
memset(&root, 0, sizeof(DataPageDeleteStack)); memset(&root, 0, sizeof(DataPageDeleteStack));
root.leftBlkno = InvalidBlockNumber; root.leftBuffer = InvalidBuffer;
root.isRoot = true; root.isRoot = true;
ginScanToDelete(gvs, rootBlkno, true, &root, InvalidOffsetNumber); ginScanToDelete(gvs, rootBlkno, true, &root, InvalidOffsetNumber);