RE: parallel vacuum comments

On Tuesday, December 7, 2021 1:42 PM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)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.

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.

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).

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.

Best regards,
Hou zj