Re: [HACKERS] Block level parallel vacuum

On Fri, 10 Jan 2020 at 20:54, Mahendra Singh Thalor <mahi6run(at)gmail(dot)com>
wrote:
>
> On Fri, 10 Jan 2020 at 15:51, Sergei Kornilov <sk(at)zsrv(dot)org> wrote:
> >
> > Hi
> > Thank you for update! I looked again
> >
> > (vacuum_indexes_leader)
> > + /* Skip the indexes that can be processed by parallel
workers */
> > + if (!skip_index)
> > + continue;
> >
> > Does the variable name skip_index not confuse here? Maybe rename to
something like can_parallel?
>
> I also agree with your point.

I don't think the change is a good idea.

- bool skip_index = (get_indstats(lps->lvshared,
i) == NULL ||
-
skip_parallel_vacuum_index(Irel[i], lps->lvshared));
+ bool can_parallel = (get_indstats(lps->lvshared,
i) == NULL ||
+
skip_parallel_vacuum_index(Irel[i],
+
lps->lvshared));

The above condition is true when the index can *not* do parallel index
vacuum. How about changing it to skipped_index and change the comment to
something like “We are interested in only index skipped parallel vacuum”?

>
> >
> > Another question about behavior on temporary tables. Use case: the user
commands just "vacuum;" to vacuum entire database (and has enough
maintenance workers). Vacuum starts fine in parallel, but on first
temporary table we hit:
> >
> > + if (RelationUsesLocalBuffers(onerel) && params->nworkers >= 0)
> > + {
> > + ereport(WARNING,
> > + (errmsg("disabling parallel option of
vacuum on \"%s\" --- cannot vacuum temporary tables in parallel",
> > +
RelationGetRelationName(onerel))));
> > + params->nworkers = -1;
> > + }
> >
> > And therefore we turn off the parallel vacuum for the remaining
tables... Can we improve this case?
>
> Good point.
> Yes, we should improve this. I tried to fix this.

+1

Regards,

--
Masahiko Sawada http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Masahiko Sawada http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services