On Mon, Oct 21, 2019 at 2:58 PM Andrey Borodin <x4...@yandex-team.ru> wrote: > > > > > 21 окт. 2019 г., в 11:12, Dilip Kumar <dilipbal...@gmail.com> написал(а): > > > > On Mon, Oct 21, 2019 at 2:30 PM Andrey Borodin <x4...@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
