Re: Behaviour of bgworker with SIGHUP

Hi,

Today, I tried to make fun with the new background worker processes in
9.3, but I found something disturbing, and need help to go further.

My code is available on https://github.com/gleu/stats_recorder. If you
take a look, it is basically a copy of Alvarro's worker_spi contrib
module with a few changes. It compiles, and installs OK.

With this code, when I change my config option (stats_recorder.naptime),
I see that PostgreSQL gets the new value, but my background worker
process doesn't. See these log lines:

LOG: stats recorder, worker_spi_main loop, stats_recorder_naptime is 1
LOG: stats recorder, worker_spi_main loop, stats_recorder_naptime is 1
LOG: received SIGHUP, reloading configuration files
LOG: parameter "stats_recorder.naptime" changed to "5"
LOG: stats recorder, worker_spi_sighup
LOG: stats recorder, worker_spi_main loop, stats_recorder_naptime is 1
LOG: stats recorder, worker_spi_main loop, stats_recorder_naptime is 1

Is it the work of the function (pointed by bgw_sighup) to get the new
config values from the postmaster? and if so, how can I get these new
values?

I thought the configuration reloading would work just like a shared
library but it doesn't seem so. I wondered if it was because I had the
sighup function (initialized with bgw_sighup), so I got rid of it. The
new behaviour was actually more surprising as it launched _PG_init each
time I did a "pg_ctl reload".

LOG: stats recorder, worker_spi_main loop, stats_recorder_naptime is 1
LOG: stats recorder, worker_spi_main loop, stats_recorder_naptime is 1
LOG: received SIGHUP, reloading configuration files
LOG: stats_recorder, _PG_init
FATAL: cannot create PGC_POSTMASTER variables after startup
LOG: worker process: stats recorder (PID 5435) exited with exit code 1

Is it the expected behaviour?

Thanks.

Regards.

--
Guillaume
http://blog.guillaume.lelarge.info
http://www.dalibo.com

Guillaume Lelarge wrote:
> Hi,
>
> Today, I tried to make fun with the new background worker processes in
> 9.3, but I found something disturbing, and need help to go further.

Thanks.

> Is it the work of the function (pointed by bgw_sighup) to get the new
> config values from the postmaster? and if so, how can I get these new
> values?

You probably want to have the sighup handler set a flag, and then call
ProcessConfigFile(PGC_SIGHUP) in your main loop when the flag is set.
Search for got_SIGHUP in postgres.c.

I think this (have a config option, and have SIGHUP work as expected)
would be useful to demo in worker_spi, if you care to submit a patch.

> I thought the configuration reloading would work just like a shared
> library but it doesn't seem so.

Yeah, you need to handle that manually, because you're running your own
process now.

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

On Mon, 2012-12-31 at 11:03 -0300, Alvaro Herrera wrote:
> Guillaume Lelarge wrote:
> > Hi,
> >
> > Today, I tried to make fun with the new background worker processes in
> > 9.3, but I found something disturbing, and need help to go further.
>
> Thanks.
>
> > Is it the work of the function (pointed by bgw_sighup) to get the new
> > config values from the postmaster? and if so, how can I get these new
> > values?
>
> You probably want to have the sighup handler set a flag, and then call
> ProcessConfigFile(PGC_SIGHUP) in your main loop when the flag is set.
> Search for got_SIGHUP in postgres.c.
>

Thanks for the tip. It works great.

> I think this (have a config option, and have SIGHUP work as expected)
> would be useful to demo in worker_spi, if you care to submit a patch.
>

Yeah, I would love too. Reading the code of worker_spi, we could add one
or three parameters: a naptime, and the schemaname for both bgprocess.
One would be enough or do you prefer all three?

> > I thought the configuration reloading would work just like a shared
> > library but it doesn't seem so.
>
> Yeah, you need to handle that manually, because you're running your own
> process now.
>

That makes sense, thanks.

--
Guillaume
http://blog.guillaume.lelarge.info
http://www.dalibo.com

Guillaume Lelarge wrote:
> On Mon, 2012-12-31 at 11:03 -0300, Alvaro Herrera wrote:

> > I think this (have a config option, and have SIGHUP work as expected)
> > would be useful to demo in worker_spi, if you care to submit a patch.
>
> Yeah, I would love too. Reading the code of worker_spi, we could add one
> or three parameters: a naptime, and the schemaname for both bgprocess.
> One would be enough or do you prefer all three?

I got no problem with three.

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

On Mon, 2012-12-31 at 12:54 -0300, Alvaro Herrera wrote:
> Guillaume Lelarge wrote:
> > On Mon, 2012-12-31 at 11:03 -0300, Alvaro Herrera wrote:
>
> > > I think this (have a config option, and have SIGHUP work as expected)
> > > would be useful to demo in worker_spi, if you care to submit a patch.
> >
> > Yeah, I would love too. Reading the code of worker_spi, we could add one
> > or three parameters: a naptime, and the schemaname for both bgprocess.
> > One would be enough or do you prefer all three?
>
> I got no problem with three.
>

OK, will do on wednesday.

Thanks.

--
Guillaume
http://blog.guillaume.lelarge.info
http://www.dalibo.com

On Mon, 2012-12-31 at 17:44 -0300, Alvaro Herrera wrote:
> Alvaro Herrera wrote:
> > Guillaume Lelarge wrote:
> > > On Mon, 2012-12-31 at 11:03 -0300, Alvaro Herrera wrote:
> >
> > > > I think this (have a config option, and have SIGHUP work as expected)
> > > > would be useful to demo in worker_spi, if you care to submit a patch.
> > >
> > > Yeah, I would love too. Reading the code of worker_spi, we could add one
> > > or three parameters: a naptime, and the schemaname for both bgprocess.
> > > One would be enough or do you prefer all three?
> >
> > I got no problem with three.
>
> Actually, it occurs to me that it might be useful to demonstrate having
> the number of processes be configurable: so we could use just two
> settings, naptime and number of workers. Have each worker just use a
> hardcoded schema, say "worker_spi_%d" or something like that.
>

Here you go.

worker_spi.naptime is the naptime between two checks.
worker_spi.total_workers is the number of workers to launch at
postmaster start time. The first one can change with a sighup, the last
one obviously needs a restart.

--
Guillaume
http://blog.guillaume.lelarge.info
http://www.dalibo.com

Guillaume Lelarge wrote:

> worker_spi.naptime is the naptime between two checks.
> worker_spi.total_workers is the number of workers to launch at
> postmaster start time. The first one can change with a sighup, the last
> one obviously needs a restart.

Many thanks. Pushed as
http://git.postgresql.org/pg/commitdiff/e543631f3c162ab5f6020b1d0209e0353ca2229a
along a few other tweaks. I hope the code is more useful as a sample now.

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