On Tue, Oct 15, 2019 at 3:55 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > On Tue, Oct 15, 2019 at 10:34 AM Masahiko Sawada <sawada.m...@gmail.com> > wrote: > > > > On Mon, Oct 14, 2019 at 6:37 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > > > > > > > 3. Do we really need to give the responsibility of deleting empty > > > pages (gistvacuum_delete_empty_pages) to gistvacuumcleanup. Can't we > > > do it in gistbulkdelte? I see one advantage of postponing it till the > > > cleanup phase which is if somehow we can accumulate stats over > > > multiple calls of gistbulkdelete, but I am not sure if it is feasible. > > > At least, the way current code works, it seems that there is no > > > advantage to postpone deleting empty pages till the cleanup phase. > > > > > > > Considering the current strategy of page deletion of gist index the > > advantage of postponing the page deletion till the cleanup phase is > > that we can do the bulk deletion in cleanup phase which is called at > > most once. But I wonder if we can do the page deletion in the similar > > way to btree index. > > > > I think there might be some advantage of the current strategy due to > which it has been chosen. I was going through the development thread > and noticed some old email which points something related to this. > See [1].
Thanks. > > > Or even we use the current strategy I think we can > > do that while not passing the pages information from bulkdelete to > > vacuumcleanup using by GistBulkDeleteResult. > > > > Yeah, I also think so. I have started a new thread [2] to know the > opinion of others on this matter. > Thank you. > > > If we avoid postponing deleting empty pages till the cleanup phase, > > > then we don't have the problem for gist indexes. > > > > Yes. But considering your pointing out I guess that there might be > > other index AMs use the stats returned from bulkdelete in the similar > > way to gist index (i.e. using more larger structure of which > > IndexBulkDeleteResult is just the first field). If we have the same > > concern the parallel vacuum still needs to deal with that as you > > mentioned. > > > > Right, apart from some functions for memory allocation/estimation and > stats copy, we might need something like amcanparallelvacuum, so that > index methods can have the option to not participate in parallel > vacuum due to reasons similar to gist or something else. I think we > can work towards this direction as this anyway seems to be required > and till we reach any conclusion for gist indexes, you can mark > amcanparallelvacuum for gist indexes as false. Agreed. I'll create a separate patch to add this callback and change parallel vacuum patch so that it checks the participation of indexes and then vacuums on un-participated indexes after parallel vacuum. Regards, -- Masahiko Sawada
