Re: Race condition in WaitForBackgroundWorkerStartup

On Tue, Nov 13, 2018 at 11:48 PM Tomas Vondra
<tomas.vondra@2ndquadrant.com> wrote:
>
> On Tue, 2018-11-13 at 07:57 -0600, Jeremy Finzel wrote:
> > ...
> >
> > I'm unclear on what counts as "worker initialization".  The error is
> > happening in the worker_spi_main function, not in the
> > worker_spi_launch function.  So does an immediate error in
> > worker_spi_main count as part of the worker initialization?
> >

Yeah, sort of.   Any error you get there can lead to the results you
are seeing.  If you want a more robust mechanism, you need to write
something like what we have done for parallel workers (See
WaitForParallelWorkersToAttach.).  Basically, you need your workers to
reach a specific state before which if there is any error, the main
backend should error out.

>
> I don't think it does (or should). worker_spi_main is pretty much the
> worker body, and worker initialization pretty much means just "we've
> initialized the worker process (including tracking it in shmem etc.)
> and invoked it's main function".
>
> I'd also argue the behavior is expected to depend on the machine to
> some extent - depending on speed and load the machine may hit the error
> before/after the GetBackgroundWorkerPid call, producing different
> results.
>
> So I'd say the behavior seems correct, at least from this POV.
>

+1.

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