RE: parallel vacuum comments

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.

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