Re: parallel vacuum comments

On Wed, Dec 8, 2021 at 12:22 PM houzj.fnst@fujitsu.com
<houzj.fnst@fujitsu.com> wrote:
>
> On Tuesday, December 7, 2021 1:42 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> > I've attached an updated patch. I've removed 0003 patch that added
> > regression tests as per discussion. Regarding the terminology like "bulkdel"
> > and "cleanup" you pointed out, I've done that in 0002 patch while moving the
> > code to vacuumparallel.c. In that file, we can consistently use the terms
> > "bulkdel" and "cleanup" instead of "vacuum"
> > and "cleanup".
> Hi,
>
> Thanks for updating the patch.
> I noticed few minor things.

Thank you for the comments!

>
> 0001
> 1)
>
>                  * Skip processing indexes that are unsafe for workers (these are
> -                * processed in do_serial_processing_for_unsafe_indexes() by leader)
> +                * processed in parallel_vacuum_process_unsafe_indexes() by leader)
>
> It might be clearer to mention that the index to be skipped are unsafe OR not
> worthwhile.

Agreed. Will add the comments.

>
> 2)
> +       /* Set index vacuum status and mark as parallel safe or not */
> +       for (int i = 0; i < pvc->nindexes; i++)
> +       {
>         ...
> +               pindstats->parallel_workers_can_process =
> +                       parallel_vacuum_index_is_parallel_safe(vacrel,
> +
vacrel->indrels[i],
> +                                                                                                  vacuum);
>
> For the comments above the loop, maybe better to mention we are marking whether
> worker can process the index(not only safe/unsafe).

Right. WIll fix.

>
> 0002
> 3)
>
> +               /*
> +                * Skip indexes that are unsuitable target for parallel index vacuum
> +                */
> +               if (parallel_vacuum_should_skip_index(indrel))
> +                       continue;
> +
>
> It seems we can use will_parallel_vacuum[] here instead of invoking the function
> again.

Oops, I missed updating it in 0002 patch. Will fix.

Regards,

-- 
Masahiko Sawada
EDB:  https://www.enterprisedb.com/