Re: GiST VACUUM

On 15/03/2019 20:25, Andrey Borodin wrote:
>> 11 марта 2019 г., в 20:03, Heikki Linnakangas <hlinnaka@iki.fi>
>> написал(а):
>> 
>> On 10/03/2019 18:40, Andrey Borodin wrote:
>>> One thing still bothers me. Let's assume that we have internal
>>> page with 2 deletable leaves. We lock these leaves in order of
>>> items on internal page. Is it possible that 2nd page have
>>> follow-right link on 1st and someone will lock 2nd page, try to
>>> lock 1st and deadlock with VACUUM?
>> 
>> Hmm. If the follow-right flag is set on a page, it means that its
>> right sibling doesn't have a downlink in the parent yet.
>> Nevertheless, I think I'd sleep better, if we acquired the locks in
>> left-to-right order, to be safe.
 >
> Actually, I did not found lock coupling in GiST code. But I decided
> to lock just two pages at once (leaf, then parent, for every pair).
> PFA v22 with this concurrency logic.

Good. I just noticed, that the README actually does say explicitly, that 
the child must be locked before the parent.

I rebased this over the new IntegerSet implementation, from the other 
thread, and did another round of refactoring, cleanups, etc. Attached is 
a new version of this patch. I'm also including the IntegerSet patch 
here, for convenience, but it's the same patch I posted at [1].

It's in pretty good shapre, but one remaining issue that needs to be fixed:

During Hot Standby, the B-tree code writes a WAL reord, when a deleted 
page is recycled, to prevent the deletion from being replayed too early 
in the hot standby. See _bt_getbuf() and btree_xlog_reuse_page(). I 
think we need to do something similar in GiST.

I'll try fixing that tomorrow, unless you beat me to it. Making the 
changes is pretty straightforward, but it's a bit cumbersome to test.

[1] 
https://www.postgresql.org/message-id/1035d8e6-cfd1-0c27-8902-40d8d45eb6e8@iki.fi

- Heikki