On Tue, Jan 14, 2020 at 10:04 AM Masahiko Sawada
<masahiko.sawada@2ndquadrant.com> wrote:
>
> On Mon, 13 Jan 2020 at 12:50, Amit Kapila <amit.kapila16@gmail.com> wrote:
> >
> > On Sat, Jan 11, 2020 at 7:48 PM Masahiko Sawada
> > <masahiko.sawada@2ndquadrant.com> wrote:
> >
> > Okay, would it better if we get rid of this variable and have code like below?
> >
> > /* Skip the indexes that can be processed by parallel workers */
> > if ( !(get_indstats(lps->lvshared, i) == NULL ||
> > skip_parallel_vacuum_index(Irel[i], lps->lvshared)))
> > continue;
>
> Make sense to me.
>
I have changed the comment and condition to make it a positive test so
that it is more clear.
> > ...
> > > Agreed. But with the updated patch the PARALLEL option without the
> > > parallel degree doesn't display warning because params->nworkers = 0
> > > in that case. So how about restoring params->nworkers at the end of
> > > vacuum_rel()?
> > >
> >
> > I had also thought on those lines, but I was not entirely sure about
> > this resetting of workers. Today, again thinking about it, it seems
> > the idea Mahendra is suggesting that is giving an error if the
> > parallel degree is not specified seems reasonable to me. This means
> > Vacuum (parallel), Vacuum (parallel) <tbl_name>, etc. will give an
> > error "parallel degree must be specified". This idea has merit as now
> > we are supporting a parallel vacuum by default, so a 'parallel' option
> > without a parallel degree doesn't have any meaning. If we do that,
> > then we don't need to do anything additional about the handling of
> > temp tables (other than what patch is already doing) as well. What do
> > you think?
> >
>
> Good point! Agreed.
>
Thanks, changed accordingly.
--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com