Re: GiST VACUUM

Hi!

PFA v6 of the patch.

12 июля 2018 г., в 21:07, Andrey Borodin x4mmm(at)yandex-team(dot)ru написал(а):


12 июля 2018 г., в 20:40, Heikki Linnakangas hlinnaka(at)iki(dot)fi написал(а):

Actually, now that I think about it more, I'm not happy with leaving orphaned pages like that behind. Let's WAL-log the removal of the downlink, and marking the leaf pages as deleted, in one WAL record, to avoid that.

OK, will do this. But this will complicate WAL replay seriously, and I do not know a proper way to test that (BTW there is GiST amcheck in progress, but I decided to leave it for a while).

Done. Now WAL record for deleted page also removes downlink from internal page. I had to use IndexPageTupleDelete() instead of IndexPageTupleMultiDelete(), but I do not think it will have any impact on performance.

But the situation in gistdoinsert(), where you encounter a deleted leaf page, could happen during normal operation, if vacuum runs concurrently with an insert. Insertion locks only one page at a time, as it descends the tree, so after it has released the lock on the parent, but before it has locked the child, vacuum might have deleted the page. In the latest patch, you're checking for that just before swapping the shared lock for an exclusive one, but I think that's wrong; you need to check for that after swapping the lock, because otherwise vacuum might delete the page while you're not holding the lock.

Looks like a valid concern, I'll move that code again.

Done.

I do not know how to test this

reliably. Internal pages are locked before leafs and locks are

coupled. No cuncurrent backend can see downlinks to pages being

deleted, unless crash happens.

Are you sure? At a quick glance, I don't think the locks are coupled.

I haven't yet tested WAL replay well, will do this soon.

Best regards, Andrey Borodin.