Horton

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

Alexander Korotkov <aekorotkov@gmail.com>

View thread View on postgr.es
From: Alexander Korotkov <aekorotkov@gmail.com>
To: Peter Geoghegan <pg@bowt.ie>
Cc: Andres Freund <andres@anarazel.de>, chenhj <chjischj@163.com>, Teodor Sigaev <teodor@sigaev.ru>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2018-12-08T01:54:45Z
Lists: pgsql-hackers

Attachments

On Fri, Dec 7, 2018 at 12:50 AM Peter Geoghegan <pg@bowt.ie> wrote:
> On Thu, Dec 6, 2018 at 12:51 PM Alexander Korotkov <aekorotkov@gmail.com> wrote:
> > So, algorithm introduced by 218f51584d5 is broken.  It tries to
> > guarantee that there are no inserters in the subtree by placing
> > cleanup lock to subtree root, assuming that inserter holds pins on the
> > path from root to leaf.  But due to concurrent splits of internal
> > pages the pins held can be not relevant to actual path.  I don't see
> > the way to fix this easily.  So, I think we should revert it from back
> > branches and try to reimplement that in master.
> >
> > However, I'd like to note that 218f51584d5 introduces two changes:
> > 1) Cleanup locking only if there pages to delete
> > 2) Cleanup locking only subtree root
> > The 2nd one is broken.  But the 1st one seems still good for me and
> > useful, because in vast majority of cases vacuum doesn't delete any
> > index pages.  So, I propose to revert 218f51584d5, but leave there
> > logic, which locks root for cleanup only once there are pages to
> > delete.  Any thoughts?
>
> Can you post a patch that just removes the 2nd part, leaving the
> still-correct first part?
>
> Your proposal sounds reasonable, but I'd like to see what you have in
> mind in detail before commenting.

Yep, please find attached draft patch.

BTW, it seems that I've another bug in GIN.  README says that

"However, posting trees are only
fully searched from left to right, starting from the leftmost leaf. (The
tree-structure is only needed by insertions, to quickly find the correct
insert location). So as long as we don't delete the leftmost page on each
level, a search can never follow a downlink to page that's about to be
deleted."

But that's not really true once we teach GIN to skip parts of posting
trees in PostgreSQL 9.4.  So, there might be a risk to visit page to
be deleted using downlink.  But in order to get real problem, vacuum
should past cleanup stage and someone else should reuse this page,
before we traverse downlink.  Thus, the risk of real problem is very
low.  But it still should be addressed.

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

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.