Re: [HACKERS] Block level parallel vacuum

On Thu, Jan 9, 2020 at 5:31 PM Sergei Kornilov <sk(at)zsrv(dot)org> wrote:
>
> Hello
>
> I noticed that parallel vacuum uses min_parallel_index_scan_size GUC to skip small indexes but this is not mentioned in documentation for both vacuum command and GUC itself.
>

Changed documentation at both places.

> + /* Determine the number of parallel workers to launch */
> + if (lps->lvshared->for_cleanup)
> + {
> + if (lps->lvshared->first_time)
> + nworkers = lps->nindexes_parallel_cleanup +
> + lps->nindexes_parallel_condcleanup - 1;
> + else
> + nworkers = lps->nindexes_parallel_cleanup - 1;
> +
> + }
> + else
> + nworkers = lps->nindexes_parallel_bulkdel - 1;
>
> (lazy_parallel_vacuum_indexes)
> Perhaps we need to add a comment for future readers, why we reduce the number of workers by 1. Maybe this would be cleaner?
>

Adapted your suggestion.

>
> I have no more comments after reading the patches.
>

Thank you for reviewing the patch.

> 1.
> + <term><replaceable class="parameter">integer</replaceable></term>
> + <listitem>
> + <para>
> + Specifies a positive integer value passed to the selected option.
> + The <replaceable class="parameter">integer</replaceable> value can
> + also be omitted, in which case the value is decided by the command
> + based on the option used.
> + </para>
> + </listitem
>
> I think, now we are supporting zero also as a degree, so it should be
> changed from "positive integer" to "positive integer(including zero)"
>

I have replaced it with "non-negative integer .."

> 2.
> + * with parallel worker processes. Individual indexes are processed by one
> + * vacuum process. At the beginning of a lazy vacuum (at lazy_scan_heap) we
>
> I think, above sentence should be like "Each individual index is
> processed by one vacuum process." or one worker
>

Hmm, in the above sentence vacuum process refers to either a leader or
worker process, so not sure if what you are suggesting is an
improvement over current.

> 3.
> + * Lazy vacuum supports parallel execution with parallel worker processes. In
> + * a parallel lazy vacuum, we perform both index vacuuming and index cleanup
>
> Here, "index vacuuming" should be changed to "index vacuum" or "index
> cleanup" to "index cleaning"
>

Okay, changed at the place you mentioned and other places where
similar change is required.

--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com