Re: [HACKERS] Block level parallel vacuum

On Thu, Jan 9, 2020 at 5:31 PM Sergei Kornilov <sk@zsrv.org> wrote:
>
> Hello
>
> I noticed that parallel vacuum uses min_parallel_index_scan_size GUC to skip small indexes but this is not mentioned
indocumentation 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