Few other things, I would like you to consider.
1. I think disable_parallel_leader_participation related code can be extracted into a separate patch as it is mainly a debug/test aid. You can also fix the problem reported by Mahendra in that context.
2. I think if we cam somehow disallow very small indexes to use parallel workers, then it will be better. Can we use min_parallel_index_scan_size to decide whether a particular index can participate in a parallel vacuum?
Forgot one minor point. Please run pgindent on all the patches.
While reviewing and testing v35 patch set, I noticed some problems. Below are some comments:
1.
/*
+ * Since parallel workers cannot access data in temporary tables, parallel
+ * vacuum is not allowed for temporary relation. However rather than
+ * skipping vacuum on the table, just disabling parallel option is better
+ * option in most cases.
+ */
+ if (RelationUsesLocalBuffers(onerel) && params->nworkers >= 0)
+ {
+ ereport(WARNING,
+ (errmsg("disabling parallel option of vacuum on \"%s\" --- cannot vacuum temporary tables in parallel",
+ RelationGetRelationName(onerel))));
+ params->nworkers = 0;
+ }
Here, I think, we should set params->nworkers = -1 to disable parallel vacuum for temporary tables. I noticed that even after warning, we were doing vacuum in parallel mode and were launching parallel workers that was wrong.
2.
Amit suggested me to check time taken by vacuum.sql regression test.
vacuum ... ok 20684 ms -------on the top of v35 patch set
vacuum ... ok 1020 ms -------without v35 patch set
Here, we can see that time taken by vacuum test case is increased too much due to parallel vacuum test cases so I will try to come with a small test case.
Thanks and Regards
Mahendra Thalor