On Fri, 10 Jan 2020 at 20:54, Mahendra Singh Thalor <mahi6...@gmail.com> wrote: > > On Fri, 10 Jan 2020 at 15:51, Sergei Kornilov <s...@zsrv.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