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

From: Alexander Korotkov <aekorotkov(at)gmail(dot)com>
To: Andrey Borodin <x4mmm(at)yandex-team(dot)ru>
Cc: Peter Geoghegan <pg(at)bowt(dot)ie>, Andres Freund <andres(at)anarazel(dot)de>, chenhj <chjischj(at)163(dot)com>, Teodor Sigaev <teodor(at)sigaev(dot)ru>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Connections hang indefinitely while taking a gin index's LWLock buffer_content lock
Date: 2018-12-13 19:40:59
Message-ID: CAPpHfdt9ySGAQSqWdSGomjb6O-rHo1HRw6qZ3PXCK0Wt9o8++A@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, Dec 13, 2018 at 8:06 PM Andrey Borodin <x4mmm(at)yandex-team(dot)ru> wrote:
> > if (BufferIsValid(lbuffer))
> > UnlockReleaseBuffer(lbuffer);
> > if (BufferIsValid(pbuffer))
> > UnlockReleaseBuffer(pbuffer);
> > if (BufferIsValid(dbuffer))
> > UnlockReleaseBuffer(dbuffer);
> > ==>
> > if (BufferIsValid(pbuffer))
> > UnlockReleaseBuffer(pbuffer);
> > if (BufferIsValid(dbuffer))
> > UnlockReleaseBuffer(dbuffer);
> > if (BufferIsValid(lbuffer))
> > UnlockReleaseBuffer(lbuffer);
>
> I think that unlock order does not matter. But I may be wrong. May be just for uniformity?

It doesn't mater, because we release all locks on every buffer at one
time. The unlock order can have effect on what waiter will acquire
the lock next after ginRedoDeletePage(). However, I don't see why one
unlock order is better than another. Thus, I just used the rule of
thumb to not change code when it's not necessary for bug fix.

> > 13 дек. 2018 г., в 21:48, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> написал(а):
> >
> > Bruce Momjian <bruce(at)momjian(dot)us> writes:
> >> 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]
> >
> > I'm also getting that, just in 9.4, but at a different line number:
> >
> > ginxlog.c: In function 'ginRedoDeletePage':
> > ginxlog.c:693: warning: 'lbuffer' may be used uninitialized in this function
> >
> > That's with gcc version 4.4.7 20120313 (Red Hat 4.4.7-23)
>
>
> That's the same variable, one place is definition while other is potential misuse.
> Seems like these 2 lines [0]
>
> + if (BufferIsValid(lbuffer))
> + UnlockReleaseBuffer(lbuffer);
>
> are superfluous: lbuffer is UnlockReleased earlier.

Thanks to everybody for noticing! Speaking more generally backpatch
to 9.4 was completely wrong: there were multiple errors. It's my
oversight, I forget how much better our xlog system became since 9.4
:)

I've pushed bf0e5a73be fixing that.

------
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andrew Gierth 2018-12-13 19:41:55 Ryu floating point output patch
Previous Message Andres Freund 2018-12-13 19:40:21 Re: Reorganize collation lookup time and place