On Mon, Jan 22, 2018 at 12:50 AM, Peter Geoghegan <pg@bowt.ie> wrote:
> On Sat, Jan 20, 2018 at 8:38 PM, Amit Kapila <amit.kapila16@gmail.com> wrote:
>> It would have been better if there were some comments besides that
>> field, but I think it has been covered at another place in the code.
>> See comments in LaunchParallelWorkers().
>>
>> /*
>> * Start workers.
>> *
>> * The caller must be able to tolerate ending up with fewer workers than
>> * expected, so there is no need to throw an error here if registration
>> * fails. It wouldn't help much anyway, because registering the worker in
>> * no way guarantees that it will start up and initialize successfully.
>> */
>
> Why is this okay for Gather nodes, though? nodeGather.c looks at
> pcxt->nworkers_launched during initialization, and appears to at least
> trust it to indicate that more than zero actually-launched workers
> will also show up when "nworkers_launched > 0". This trust seems critical
> when parallel_leader_participation is off, because "node->nreaders ==
> 0" overrides the parallel_leader_participation GUC's setting (note
> that node->nreaders comes directly from pcxt->nworkers_launched). If
> zero workers show up, and parallel_leader_participation is off, but
> pcxt->nworkers_launched/node->nreaders is non-zero, won't the Gather
> never make forward progress?
>
Ideally, that situation should be detected and we should throw an
error, but that doesn't happen today. However, it will be handled
with Robert's patch on the other thread for CF entry [1].
[1] - https://commitfest.postgresql.org/16/1341/
--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com
