On Thu, 19 Dec 2019 at 11:47, Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Wed, Dec 18, 2019 at 6:02 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> >
> > On Wed, Dec 18, 2019 at 3:36 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> > >
> > > Few other comments which I have not fixed:
> > >
> >
> > + /* interface function to support parallel vacuum */
> > + amestimateparallelvacuum_function amestimateparallelvacuum; /*
> > can be NULL */
> > } IndexAmRoutine;
> >
> > One more thing, why have you removed the estimate function for API
> > patch?
> >
>
> Again thinking about this, it seems to me what you have done here is
> probably the right direction because whatever else we will do we need
> to have some untested code or we need to write/enhance some IndexAM to
> test this. The point is that we don't have any IndexAM in the core
> (after working around Gist index) which has this requirement and we
> have not even heard from anyone of such usage, so there is a good
> chance that whatever we do might not be sufficient for the IndexAM
> that have such usage.
>
> Now, we are already providing an option that one can set
> VACUUM_OPTION_NO_PARALLEL to indicate that the IndexAM can't
> participate in a parallel vacuum. So, I feel if there is any IndexAM
> which would like to pass more data along with IndexBulkDeleteResult,
> they can use that option. It won't be very difficult to enhance or
> provide the new APIs to support a parallel vacuum if we come across
> such a usage.
Yeah that's exactly what I was thinking. I was about to send such
email. The idea is good but I thought we can exclude this feature from
the first version patch because we still don't have index AMs that
uses that callback in core after gist index patch gets committed. That
is, an index AM that does vacuum like the current gist indexes should
set VACUUM_OPTION_NO_PARALLEL and we can discuss that again when we
got real voice from index AM developers.
> I think we should just modify the comments atop
> VACUUM_OPTION_NO_PARALLEL to mention this. I think this should be
> good enough for the first version of parallel vacuum considering we
> are able to support a parallel vacuum for all in-core indexes.
I added some comments about that in v36 patch but I slightly modified it.
I'll submit an updated version patch soon.
Regards,
--
Masahiko Sawada http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services