On Fri, Dec 8, 2017 at 9:45 PM, Robert Haas <robertmhaas@gmail.com> wrote:
> On Fri, Dec 8, 2017 at 4:56 AM, Amit Kapila <amit.kapila16@gmail.com> wrote:
>> I think the optimization you are suggesting has a merit over what I
>> have done in the patch. However, one point to note is that we are
>> anyway going to call that function down in the path(
>> WaitForParallelWorkersToExit->WaitForBackgroundWorkerShutdown->GetBackgroundWorkerPid),
>> so we might want to take the advantage of calling it first time as
>> well. We can actually cache the status of workers that have returned
>> BGWH_STOPPED and use it later so that we don't need to make this
>> function call again for workers which are already stopped. We can
>> even do what Tom is suggesting to avoid calling it for workers which
>> are known to be launched, but I am really not sure if that function
>> call is costly enough that we need to maintain one extra state to
>> avoid that.
>
> Well, let's do what optimization we can without making it too complicated.
>
Okay, see the attached and let me know if that suffices the need?
>> While looking into this code path, I wonder how much we are gaining by
>> having two separate calls (WaitForParallelWorkersToFinish and
>> WaitForParallelWorkersToExit) to check the status of workers after
>> finishing the parallel execution? They are used together in rescan
>> code path, so apparently there is no benefit calling them separately
>> there. OTOH, during shutdown of nodes, it will often be the case that
>> they will be called in a short duration after each other except for
>> the case where we need to shut down the node for the early exit like
>> when there is a limit clause or such.
>
> I'm not 100% sure either, but if we're going to do anything about
> that, it seems like a topic for another patch.
>
makes sense.
--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com