From cbabf50dccc29aed456a56ae8909d4d593c3ab38 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Fri, 14 Jun 2013 14:26:50 -0400 Subject: Avoid deadlocks during insertion into SP-GiST indexes. SP-GiST's original scheme for avoiding deadlocks during concurrent index insertions doesn't work, as per report from Hailong Li, and there isn't any evident way to make it work completely. We could possibly lock individual inner tuples instead of their whole pages, but preliminary experimentation suggests that the performance penalty would be huge. Instead, if we fail to get a buffer lock while descending the tree, just restart the tree descent altogether. We keep the old tuple positioning rules, though, in hopes of reducing the number of cases where this can happen. Teodor Sigaev, somewhat edited by Tom Lane --- src/backend/access/spgist/spgdoinsert.c | 34 +++++++++++++++++++++++++++++---- 1 file changed, 30 insertions(+), 4 deletions(-) (limited to 'src/backend/access/spgist/spgdoinsert.c') diff --git a/src/backend/access/spgist/spgdoinsert.c b/src/backend/access/spgist/spgdoinsert.c index b3f8f6a2313..1df9fbc5b32 100644 --- a/src/backend/access/spgist/spgdoinsert.c +++ b/src/backend/access/spgist/spgdoinsert.c @@ -1850,9 +1850,13 @@ spgSplitNodeAction(Relation index, SpGistState *state, } /* - * Insert one item into the index + * Insert one item into the index. + * + * Returns true on success, false if we failed to complete the insertion + * because of conflict with a concurrent insert. In the latter case, + * caller should re-call spgdoinsert() with the same args. */ -void +bool spgdoinsert(Relation index, SpGistState *state, ItemPointer heapPtr, Datum datum, bool isnull) { @@ -1933,12 +1937,32 @@ spgdoinsert(Relation index, SpGistState *state, &isNew); current.blkno = BufferGetBlockNumber(current.buffer); } - else if (parent.buffer == InvalidBuffer || - current.blkno != parent.blkno) + else if (parent.buffer == InvalidBuffer) { + /* we hold no parent-page lock, so no deadlock is possible */ current.buffer = ReadBuffer(index, current.blkno); LockBuffer(current.buffer, BUFFER_LOCK_EXCLUSIVE); } + else if (current.blkno != parent.blkno) + { + /* descend to a new child page */ + current.buffer = ReadBuffer(index, current.blkno); + + /* + * Attempt to acquire lock on child page. We must beware of + * deadlock against another insertion process descending from that + * page to our parent page (see README). If we fail to get lock, + * abandon the insertion and tell our caller to start over. XXX + * this could be improved; perhaps it'd be worth sleeping a bit + * before giving up? + */ + if (!ConditionalLockBuffer(current.buffer)) + { + ReleaseBuffer(current.buffer); + UnlockReleaseBuffer(parent.buffer); + return false; + } + } else { /* inner tuple can be stored on the same page as parent one */ @@ -2139,4 +2163,6 @@ spgdoinsert(Relation index, SpGistState *state, SpGistSetLastUsedPage(index, parent.buffer); UnlockReleaseBuffer(parent.buffer); } + + return true; } -- cgit v1.2.3