Re: [HACKERS] Block level parallel vacuum

On Thu, Nov 28, 2019 at 4:10 PM Mahendra Singh <mahi6run(at)gmail(dot)com> wrote:
>
> On Thu, 28 Nov 2019 at 13:32, Masahiko Sawada <masahiko(dot)sawada(at)2ndquadrant(dot)com> wrote:
>>
>> On Wed, 27 Nov 2019 at 19:21, Mahendra Singh <mahi6run(at)gmail(dot)com> wrote:
>> >
>> >
>> > Thanks for the re-based patches.
>> >
>> > On the top of v35 patch, I can see one compilation warning.
>> >>
>> >> parallel.c: In function ‘LaunchParallelWorkers’:
>> >> parallel.c:502:2: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
>> >> int i;
>> >> ^
>> >
>> >
>> > Above warning is due to one extra semicolon added at the end of declaration line in v35-0003 patch. Please fix this in next version.
>> > + int nworkers_to_launch = Min(nworkers, pcxt->nworkers);;
>>
>> Thanks. I will fix it in the next version patch.
>>
>> >
>> > I will continue my testing on the top of v35 patch set and will post results.
>
>
> While reviewing v35 patch set and doing testing, I found that if we disable leader participation, then we are launching 1 less parallel worker than total number of indexes. (I am using max_parallel_workers = 20, max_parallel_maintenance_workers = 20)
>
> For example: If table have 3 indexes and we gave 6 parallel vacuum degree(leader participation is disabled), then I think, we should launch 3 parallel workers but we are launching 2 workers due to below check.
> + nworkers = lps->nindexes_parallel_bulkdel - 1;
> +
> + /* Cap by the worker we computed at the beginning of parallel lazy vacuum */
> + nworkers = Min(nworkers, lps->pcxt->nworkers);
>
> Please let me know your thoughts for this.
>

I think it is probably because this part of the code doesn't consider
PARALLEL_VACUUM_DISABLE_LEADER_PARTICIPATION. I think if we want we
can change it but I am slightly nervous about the code complexity this
will bring but maybe that is fine.

--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com