Re: [HACKERS] Block level parallel vacuum

On Tue, 28 Jan 2020 at 12:32, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> On Tue, Jan 28, 2020 at 12:04 PM Mahendra Singh Thalor
> <mahi6run(at)gmail(dot)com> wrote:
> >
> > On Tue, 28 Jan 2020 at 08:14, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> > >
> > > On Tue, Jan 28, 2020 at 2:13 AM Mahendra Singh Thalor
> > > <mahi6run(at)gmail(dot)com> wrote:
> > > >
> > > > On Sat, 25 Jan 2020 at 12:11, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> > > > >
> > > > > On Fri, Jan 24, 2020 at 4:58 PM Mahendra Singh Thalor
> > > > > <mahi6run(at)gmail(dot)com> wrote:
> > > > > >
> > > > > > On Thu, 23 Jan 2020 at 15:32, Mahendra Singh Thalor <mahi6run(at)gmail(dot)com> wrote:
> > > > > > >
> > > > > > > On Wed, 22 Jan 2020 at 12:48, Masahiko Sawada
> > > > > > > <masahiko(dot)sawada(at)2ndquadrant(dot)com> wrote:
> > > > > > > >
> > > > > > > > Attached the updated version patch.
> > > > > > >
> > > > > > > Thanks Sawada-san for the re-based patch.
> > > > > > >
> > > > > > > I reviewed and tested this patch. Patch looks good to me.
> > > > > >
> > > > > > As offline, suggested by Amit Kapila, I verified vacuumdb "-P" option
> > > > > > functionality with older versions(<13) and also I tested vacuumdb by
> > > > > > giving "-j" option with "-P". All are working as per expectation and I
> > > > > > didn't find any issue with these options.
> > > > > >
> > > > >
> > > > > I have made few modifications in the patch.
> > > > >
> > > > > 1. I think we should try to block the usage of 'full' and 'parallel'
> > > > > option in the utility rather than allowing the server to return an
> > > > > error.
> > > > > 2. It is better to handle 'P' option in getopt_long in the order of
> > > > > its declaration in long_options array.
> > > > > 3. Added an Assert for server version while handling of parallel option.
> > > > > 4. Added a few sentences in the documentation.
> > > > >
> > > > > What do you guys think of the attached?
> > > > >
> > > >
> > > > I took one more review round. Below are some review comments:
> > > >
> > > > 1.
> > > > -P, --parallel=PARALLEL_DEGREE do parallel vacuum
> > > >
> > > > I think, "do parallel vacuum" should be modified. Without specifying -P, we are still doing parallel vacuum so we can use like "degree for parallel vacuum"
> > > >
> > >
> > > I am not sure if 'degree' makes it very clear. How about "use this
> > > many background workers for vacuum, if available"?
> >
> > If background workers are many, then automatically, we are using them(by default parallel vacuum). This option is to put limit on background workers(limit for vacuum workers) to be used by vacuum process.
> >
>
> I don't think that the option is just to specify the max limit because
> that is generally controlled by guc parameters. This option allows
> users to specify the number of workers for the cases where he has more
> knowledge about the size/type of indexes. In some cases, the user
> might be able to make a better decision and that was the reason we
> have added this option in the first place.
>
> > So I think, we can use "max parallel vacuum workers (by default, based on no. of indexes)" or "control parallel vacuum workers"
> >
>
> Hmm, I feel what I suggested is better because of the above explanation.

Agreed.

--
Thanks and Regards
Mahendra Singh Thalor
EnterpriseDB: http://www.enterprisedb.com