Re: amcheck verification for GiST

On 27/03/2019 11:51, Andrey Borodin wrote:
> Hi!
> 
> Here's new version of GiST amcheck, which takes into account recently committed GiST VACUUM.
> It tests that deleted pages do not contain any data.

Thanks! I had a look, and refactored it quite a bit.

I found the way the recursion worked confusing. On each internal page, 
it looped through all the child nodes, checking the consistency of the 
downlinks. And then it looped through the children again, to recurse. 
This isn't performance-critical, but visiting every page twice still 
seems strange.

In gist_check_page_keys(), if we get into the code to deal with a 
concurrent update, we set 'parent' to point to a tuple on a parent page, 
then unlock it, and continue to look at remaining tuples, using the 
pointer that points to an unlocked buffer.

I came up with the attached, which fixes the above-mentioned things. I 
also replaced the check that each node has only internal or leaf 
children, with a different check that the tree has the same height in 
all branches. That catches more potential problems, and was easier to 
implement after the refactoring. This still needs at least a round of 
fixing typos and tidying up comments, but it's more straightforward now, 
IMHO.

What have you been using to test this?

- Heikki