Re: Review: Extra Daemons / bgworker

Markus Wanner wrote:
> On 11/28/2012 03:51 PM, Alvaro Herrera wrote:
> > I remember your patchset. I didn't look at it for this round, for no
> > particular reason. I did look at KaiGai's submission from two
> > commitfests ago, and also at a patch from Simon which AFAIK was never
> > published openly. Simon's patch merged the autovacuum code to make
> > autovac workers behave like bgworkers as handled by his patch, just like
> > you suggest. To me it didn't look all that good; I didn't have the guts
> > for that, hence the separation. If later bgworkers are found to work
> > well enough, we can "port" autovac workers to use this framework, but
> > for now it seems to me that the conservative thing is to leave autovac
> > untouched.
>
> Hm.. interesting to see how the same idea grows into different patches
> and gets refined until we end up with something considered committable.
>
> Do you remember anything in particular that didn't look good? Would you
> help reviewing a patch on top of bgworker-7 that changed autovacuum into
> a bgworker?

I'm not really that interested in it currently ... and there are enough
other patches to review. I would like bgworkers to mature a bit more
and get some heavy real world testing before we change autovacuum.

> > How are you envisioning that the requests would occur?
>
> Just like av_launcher does it now: set a flag in shared memory and
> signal the postmaster (PMSIGNAL_START_AUTOVAC_WORKER).

I'm not sure how this works. What process is in charge of setting such
a flag?

> >> In assign_maxconnections, et al, GetNumRegisteredBackgroundWorkers() is
> >> used in relation to MAX_BACKENDS or to calculate MaxBackends. That seems
> >> to "leak" the bgworkers that registered with neither
> >> BGWORKER_SHMEM_ACCESS nor BGWORKER_BACKEND_DATABASE_CONNECTION set. Or
> >> is there any reason to discount such extra daemon processes?
> >
> > No, I purposefully let those out, because those don't need a PGPROC. In
> > fact that seems to me to be the whole point of non-shmem-connected
> > workers: you can have as many as you like and they won't cause a
> > backend-side impact. You can use such a worker to connect via libpq to
> > the server, for example.
>
> Hm.. well, in that case, the shmem-connected ones are *not* counted. If
> I create and run an extensions that registers 100 shmem-connected
> bgworkers, I cannot connect to a system with max_connections=100
> anymore, because bgworkers then occupy all of the connections, already.

This is actually a very silly bug: it turns out that guc.c obtains the
count of workers before workers have actually registered. So this
necessitates some reshuffling.

> Or put another way: max_connections should always be the
> max number of *client* connections the DBA wants to allow.

Completely agreed.

--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services