Re: dynamic background workers

On Thu, Jun 20, 2013 at 9:57 AM, Markus Wanner <markus@bluegap.ch> wrote:
> That sounds like a good simplification. Even if it's an O(n) operation,
> the n in question here has relatively low practical limits. It's
> unlikely to be much of a concern, I guess.

The constant factor is also very small.  Generally, I would expect
num_worker_processes <~ # CPUs, and scanning a 32, 64, or even 128
element array is not a terribly time-consuming operation.  We might
need to re-think this when systems with 4096 processors become
commonplace, but considering how many other things would also need to
be fixed to work well in that universe, I'm not too concerned about it
just yet.

One thing I think we probably want to explore in the future, for both
worker backends and regular backends, is pre-forking.  We could avoid
some of the latency associated with starting up a new backend or
opening a new connection in that way.  However, there are quite a few
details to be thought through there, so I'm not eager to pursue that
just yet.  Once we have enough infrastructure to implement meaningful
parallelism, we can benchmark it and find out where the bottlenecks
are, and which solutions actually help most.

> Back then, I solved it by having a "fork request slot". After starting,
> the bgworker then had to clear that slot and register with a coordinator
> process (i.e. the original requestor), so that one learned its fork
> request was successful. At some point I expanded that to multiple
> request slots to better handle multiple concurrent fork requests.
> However, it was difficult to get right and requires more IPC than your
> approach.

I do think we need a mechanism to allow the backend that requested the
bgworker to know whether or not the bgworker got started, and whether
it unexpectedly died.  Once we get to the point of calling user code
within the bgworker process, it can use any number of existing
mechanisms to make sure that it won't die without notifying the
backend that started it (short of a PANIC, in which case it won't
matter anyway).  But we need a way to report failures that happen
before that point.  I have some ideas about that, but decided to leave
them for future passes.  The remit of this patch is just to make it
possible to dynamically register bgworkers.  Allowing a bgworker to be
"tied" to the session that requested it via some sort of feedback loop
is a separate project - which I intend to tackle before CF2, assuming
this gets committed (and so far nobody is objecting to that).

>> I have attempted to
>> make the code that transfers the shared_memory state into the
>> postmaster's private state as paranoid as humanly possible.  The
>> precautions taken are documented in the comments.  Conversely, when a
>> background worker flagged as BGW_NEVER_RESTART is considered for
>> restart (and we decide against it), the corresponding slot in the
>> shared memory array is marked as no longer in use, allowing it to be
>> reused for a new registration.
>
> Sounds like the postmaster is writing to shared memory. Not sure why
> I've been trying so hard to avoid that, though. After all, it can hardly
> hurt itself *writing* to shared memory.

I think there's ample room for paranoia about postmaster interaction
with shared memory, but all it's doing is setting a flag, which is no
different from what CheckPostmasterSignal() already does.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company