Re: [HACKERS] parallel.c oblivion of worker-startup failures
Amit Kapila <amit.kapila16@gmail.com>
From: Amit Kapila <amit.kapila16@gmail.com>
To: Robert Haas <robertmhaas@gmail.com>
Cc: Michael Paquier <michael.paquier@gmail.com>, Tom Lane <tgl@sss.pgh.pa.us>, pgsql-hackers <pgsql-hackers@postgresql.org>
Date: 2017-12-11T04:07:39Z
Lists: pgsql-hackers
Attachments
- fix_parallel_worker_startup_failures_v1.patch (application/octet-stream) patch v1
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
Commits
-
Report an ERROR if a parallel worker fails to start properly.
- 2843c01a56eb 9.6.7 landed
- 383e4268ff8c 10.2 landed
- 2badb5afb89c 11.0 landed
-
Report failure to start a background worker.
- facd94e72f27 9.4.16 landed
- 0426a77ce465 9.5.11 landed
- b75644066091 9.6.7 landed
- a8ef4e81e6f2 10.2 landed
- 28724fd90d2f 11.0 landed