Horton

Re: Connections hang indefinitely while taking a gin index's LWLock buffer_content lock

x4mmm@yandex-team.ru

View thread View on postgr.es
From: Andrey Borodin <x4mmm@yandex-team.ru>
To: Peter Geoghegan <pg@bowt.ie>
Cc: "chjischj@163.com" <chjischj@163.com>, Teodor Sigaev <teodor@sigaev.ru>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2018-11-15T12:30:25Z
Lists: pgsql-hackers 
Hi!

Mail.App somehow borken my reponse with <div class="ApplePlainTextBody">, which is far beyond of my understanding of what is plain text, that's why I'll quote all my previous message here. Hope this mail client is OK.

14.11.2018, 23:13, "Andrey Borodin" <x4mmm@yandex-team.ru>:
> Hi everyone!
>
> I didn't noticed this thread for too long somehow, sorry.
>
>> 8 нояб. 2018 г., в 6:46, Peter Geoghegan <pg@bowt.ie> написал(а):
>>
>> I don't think
>> the general "there can't be any inserters at this subtree" thing works
>> given that we have to couple buffer locks when moving right for other
>> reasons. We call ginStepRight() within ginFinishSplit(), for reasons
>> that date back to bug fix commit ac4ab97e from 2013 -- that detail is
>> probably important, because it seems to be what breaks the subtree
>> design (we don't delete in two phases or anything in GIN).
>
> ginVacuumPostingTreeLeaves() holds LockBufferForCleanup() on subtree root b.
> Thus there may be no GinBtreeStack's holding pin on b at the moment.
> When you ginStepRight(b) to the parent in ginFinishSplit(), you always get to the buffer from your stack.
> Hence you can never have ginFinishSplit() deadlock with cleanup of subtree whose root is LockBufferForCleanup()'d.
>
> Is this correct or did I miss something?
>
> But we have a deadlock at hand, I'll think more about it. Something with locking protocol is clearly wrong.
>
>> 11 нояб. 2018 г., в 22:33, chenhj <chjischj@163.com> написал(а):
>>
>> The order of get lwlock in ginRedoDeletePage() may should be change from "dbuffer->pbuffer->lbuffer" to "lbuffer->dbuffer->pbuffer" . Is this right?
>
> This looks correct to me.
>
> Best regards, Andrey Borodin.

I've reread Chen's reports and understood his findings.

> When you ginStepRight(b) to the parent in ginFinishSplit(), you always get to the buffer from your stack.

This statement does not hold.
When you have a GinBtreeStack S, one of it's internal pages may split into new page P. If in this moment you start a multi-level delete from P and cascade split from S, P is not in S and we may deadlock.

Correct solution seems to replace lock-coupling descent by usual B-tree searches for parent pages as in B-tree.
I think I can compose patch for consideration.

Best regards, Andrey Borodin.

Commits

  1. Fix WAL format incompatibility introduced by backpatching of 52ac6cd2d0

  2. Fix wrong backpatching of ginRedoDeletePage() deadlock fix

  3. Fix deadlock in GIN vacuum introduced by 218f51584d5

  4. Prevent GIN deleted pages from being reclaimed too early

  5. Prevent deadlock in ginRedoDeletePage()

  6. Reduce page locking in GIN vacuum

  7. Fix race condition in GIN posting tree page deletion.