On Mon, Sep 30, 2019 at 11:00 AM Alexander Korotkov
<a.korotkov@postgrespro.ru> wrote:
> Thank you for pointing. I remember this thread, but don't remember
> details. I'll reread it.
I think that doing this now would be a good idea:
"""
Defensively checking the page type (pending, posting, etc) within
scanPendingInsert() would be a good start. That's already something
that we do for posting trees. If we're following the same rules as
posting trees (which sounds like a good idea), then we should have the
same defenses. Same applies to using elogs within ginInsertCleanup()
-- we should promote those Assert()s to elog()s.
"""
In other words, ginInsertCleanup() should have defensive "Page types
match?" checks that are similar to the existing checks in
ginStepRight(). In both cases we're stepping right while coupling
locks, to avoid concurrent page deletion.
> > > 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.
> Frankly speaking I'm not very happy with special version of B-tree,
> which is builtin to GIN. This version of B-tree is lacking of high
> keys. AFAIR because of lack of high keys, we can't implement the same
> concurrency model as nbtree. Instead we have to do super-locking for
> page deletion and so on. That looks ridiculous for me. I think in
> future we should somehow reimplement GIN on top of nbtree.
I think that that could work on top of the new nbtree posting list
stuff, provided that it was acceptable to not use posting list
compression in the main tree -- the way that posting list splits work
there needs to be able to think about free space in a very simple way
that is broken even by GIN's varbyte compression. Compression could
still be used in posting trees, though.
> But we have to provide some way less radical fixes for our stable
> releases. In particular, I believe patch I've posted in this thread
> makes situation better not worse. That is it fixes one bug without
> introducing mode bugs. But I'm going to analyze more on this and
> document GIN concurrency better in the README. Probably, I'll spot
> more details.
Thanks.
--
Peter Geoghegan