Horton

Re: Connections hang indefinitely while taking a gin index's LWLock buffer_content lock(PG10.7)

Peter Geoghegan <pg@bowt.ie>

View thread View on postgr.es
From: Peter Geoghegan <pg@bowt.ie>
To: Alexander Korotkov <a.korotkov@postgrespro.ru>
Cc: chenhj <chjischj@163.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2019-09-29T19:52:43Z
Lists: pgsql-hackers 
On Sun, Sep 29, 2019 at 7:38 AM Alexander Korotkov
<a.korotkov@postgrespro.ru> wrote:
> Starting from root seems OK for me, because vacuum blocks all
> concurrent inserts before doing this.  But this needs to be properly
> documented in readme.

I never got an adequate answer to this closely related question almost
two years ago:

https://www.postgresql.org/message-id/CAH2-Wz=GTnAPzEEZqYELOv3h1Fxpo5xhMrBP6aMGEKLKv95csQ@mail.gmail.com

In general, ginInsertCleanup() seems badly designed. Why is it okay
that there is no nbtree-style distinction between page deletion and
page recycling?

> Locking from right to left is clearly wrong.  It could deadlock with
> concurrent ginStepRight(), which locks from left to right.  I expect
> this happened in your case.  I'm going to reproduce this and fix.

I am sick and tired of seeing extremely basic errors like this within
GIN's locking protocols. Bugs happen, but these are not ordinary bugs.
They're more or less all a result of giving no thought to the high
level design. I'm not blaming you for this, or any one person. But
this is not okay.

Anything around index concurrency needs to be explained in
excruciating detail, while taking a top-down approach that applies
general rules (e.g. you can only do lock coupling left to right, or
bottom to top in nbtree). Anything less than that should be assumed to
be wrong on general principle.

-- 
Peter Geoghegan

Commits

  1. Fix traversing to the deleted GIN page via downlink

  2. Revise GIN README

  3. Fix deadlock between ginDeletePage() and ginStepRight()