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

On Thu, Dec 13, 2018 at 03:03:54PM +0300, Alexander Korotkov wrote:
> On Wed, Dec 12, 2018 at 4:08 PM Andrey Borodin <x4mmm@yandex-team.ru> wrote:
> > > 12 дек. 2018 г., в 3:26, Alexander Korotkov <aekorotkov@gmail.com> написал(а):
> > >
> > > BTW, I still can't see you're setting deleteXid field of
> > > ginxlogDeletePage struct anywhere.
> > Oops. Fixed.
> > >
> > > Also, I note that protection against re-usage of recently deleted
> > > pages is implemented differently than it is in B-tree.
> > > 1) You check TransactionIdPrecedes(GinPageGetDeleteXid(page),
> > > RecentGlobalDataXmin)), while B-tree checks
> > > TransactionIdPrecedes(opaque->btpo.xact, RecentGlobalXmin).  Could you
> > > clarify why do we use RecentGlobalDataXmin instead of RecentGlobalXmin
> > > here?
> > As far as I understand RecentGlobalDataXmin may be slightly farther than RecentGlobalXmin in case when
replication_slot_catalog_xminis holding RecentGlobalXmin. And GIN is never used by catalog tables. But let's use
RecentGlobalXminlike in B-tree.
 
> >
> > > 2) B-tree checks this condition both before putting page into FSM and
> > > after getting page from FSM.  You're checking only after getting page
> > > from FSM.  Approach of B-tree looks better for me.  It's seems more
> > > consistent when FSM pages are really free for usage.
> > Fixed.
> 
> Thank you.  I've revised your patch and pushed it.  As long as two
> other patches in this thread.

I am seeing this warning in the 9.4 branch:

    ginxlog.c:756:5: warning: ‘lbuffer’ may be used uninitialized
    in this function [-Wmaybe-uninitialized]

from this commit:

    commit 19cf52e6cc33b9e126775f28269ccccb6ddf7e30
    Author: Alexander Korotkov <akorotkov@postgresql.org>
    Date:   Thu Dec 13 06:12:25 2018 +0300

    Prevent deadlock in ginRedoDeletePage()

    On standby ginRedoDeletePage() can work concurrently with read-only queries.
    Those queries can traverse posting tree in two ways.
    1) Using rightlinks by ginStepRight(), which locks the next page before
       unlocking its left sibling.
    2) Using downlinks by ginFindLeafPage(), which locks at most one page at time.

    Original lock order was: page, parent, left sibling.  That lock order can
    deadlock with ginStepRight().  In order to prevent deadlock this commit changes
    lock order to: left sibling, page, parent.  Note, that position of parent in
    locking order seems insignificant, because we only lock one page at time while
    traversing downlinks.

    Reported-by: Chen Huajun
    Diagnosed-by: Chen Huajun, Peter Geoghegan, Andrey Borodin
    Discussion: https://postgr.es/m/31a702a.14dd.166c1366ac1.Coremail.chjischj%40163.com
    Author: Alexander Korotkov
    Backpatch-through: 9.4

-- 
  Bruce Momjian  <bruce@momjian.us>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com

+ As you are, so once was I.  As I am, so you will be. +
+                      Ancient Roman grave inscription +