On Tue, 3 Dec 2019 at 16:27, Amit Kapila <amit.kapil...@gmail.com> wrote:
> On Tue, Dec 3, 2019 at 4:25 PM Amit Kapila <amit.kapil...@gmail.com>
> wrote:
>
>> 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
EnterpriseDB: http://www.enterprisedb.com
