Re: Questions/Observations related to Gist vacuum

On Mon, Oct 21, 2019 at 2:58 PM Andrey Borodin <x4mmm@yandex-team.ru> wrote:
>
>
>
> > 21 окт. 2019 г., в 11:12, Dilip Kumar <dilipbalaut@gmail.com> написал(а):
> >
> > On Mon, Oct 21, 2019 at 2:30 PM Andrey Borodin <x4mmm@yandex-team.ru> wrote:
> >>
> >> I've took a look into the patch, and cannot understand one simple thing...
> >> We should not call gistvacuum_delete_empty_pages() for same gist_stats twice.
> >> Another way once the function is called we should somehow update or zero empty_leaf_set.
> >> Does this invariant hold in your patch?
> >>
> > Thanks for looking into the patch.   With this patch now
> > GistBulkDeleteResult is local to single gistbulkdelete call or
> > gistvacuumcleanup.  So now we are not sharing GistBulkDeleteResult,
> > across the calls so I am not sure how it will be called twice for the
> > same gist_stats?  I might be missing something here?
>
> Yes, you are right, sorry for the noise.
> Currently we are doing both gistvacuumscan() and gistvacuum_delete_empty_pages() in both gistbulkdelete() and gistvacuumcleanup(). Is it supposed to be so?

There was an issue discussed in parallel vacuum thread[1], and for
solving that it has been discussed in this thread[2] that we can
delete empty pages in bulkdelete phase itself.  But, that does not
mean that we can remove that from the gistvacuumcleanup phase.
Because if the gistbulkdelete is not at all called in the vacuum pass
then gistvacuumcleanup, will perform both gistvacuumscan and
gistvacuum_delete_empty_pages.  In short, In whichever pass, we detect
the empty page in the same pass we delete the empty page.

Functions gistbulkdelete() and gistvacuumcleanup() look very similar
and share some comments. This is what triggered my attention.

[1] - https://www.postgresql.org/message-id/CAA4eK1JEQ2y3uNucNopDjK8pse6xSe5%3D_oknoWfRQvAF%3DVqsBA%40mail.gmail.com
[2] - https://www.postgresql.org/message-id/69EF7B88-F3E7-4E09-824D-694CF39E5683%40iki.fi

-- 
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com