Re: [HACKERS] Parallel tuplesort (for parallel B-Tree index creation) - Mailing list pgsql-hackers

From Peter Geoghegan
Subject Re: [HACKERS] Parallel tuplesort (for parallel B-Tree index creation)
Date
Msg-id CAH2-WznsP-0EG_WSZCOdpcuGzjo+aF-Vhaq5O7dCmehOjh2sHg@mail.gmail.com
Whole thread Raw
In response to Re: [HACKERS] Parallel tuplesort (for parallel B-Tree index creation)  (Amit Kapila <amit.kapila16@gmail.com>)
Responses Re: [HACKERS] Parallel tuplesort (for parallel B-Tree index creation)
List pgsql-hackers
On Sun, Jan 21, 2018 at 6:34 PM, Amit Kapila <amit.kapila16@gmail.com> wrote:
>> 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].

I knew that, but I was confused by your sketch of the
WaitForParallelWorkerToAttach() API [1]. Specifically, your suggestion
that the problem was unique to nbtsort.c, or was at least something
that nbtsort.c had to take a special interest in. It now appears more
like a general problem with a general solution, and likely one that
won't need *any* changes to code in places like nodeGather.c (or
nbtsort.c, in the case of my patch).

I guess that you meant that parallel CREATE INDEX is the first thing
to care about the *precise* number of nworkers_launched -- that is
kind of a new thing. That doesn't seem like it makes any practical
difference to us, though. I don't see why nbtsort.c should take a
special interest in this problem, for example by calling
WaitForParallelWorkerToAttach() itself. I may have missed something,
but right now ISTM that it would be risky to make the API anything
other than what both nodeGather.c and nbtsort.c already expect (that
they'll either have nworkers_launched workers show up, or be able to
propagate an error).

[1] https://postgr.es/m/CAA4eK1KzvXTCFF8inhcEviUPxp4yWCS3rZuwjfqMttf75x2rvA@mail.gmail.com
-- 
Peter Geoghegan


pgsql-hackers by date:

Previous
From: Thomas Munro
Date:
Subject: Re: Doc tweak for huge_pages?
Next
From: Fabien COELHO
Date:
Subject: Re: pgbench - add \if support