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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
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
Attachments:
worker_spi.patchtext/x-patch; charset=UTF-8; name=worker_spi.patchDownload
diff --git a/contrib/worker_spi/worker_spi.c b/contrib/worker_spi/worker_spi.c
index 6da747b..4b6de45 100644
--- a/contrib/worker_spi/worker_spi.c
+++ b/contrib/worker_spi/worker_spi.c
@@ -35,12 +35,16 @@
#include "lib/stringinfo.h"
#include "utils/builtins.h"
#include "utils/snapmgr.h"
+#include "tcop/utility.h"
PG_MODULE_MAGIC;
void _PG_init(void);
+static bool got_sighup = false;
static bool got_sigterm = false;
+static int worker_spi_naptime = 1;
+static int worker_spi_total_workers = 2;
typedef struct worktable
@@ -65,6 +69,7 @@ static void
worker_spi_sighup(SIGNAL_ARGS)
{
elog(LOG, "got sighup!");
+ got_sighup = true;
if (MyProc)
SetLatch(&MyProc->procLatch);
}
@@ -176,13 +181,22 @@ worker_spi_main(void *main_arg)
*/
rc = WaitLatch(&MyProc->procLatch,
WL_LATCH_SET | WL_TIMEOUT | WL_POSTMASTER_DEATH,
- 1000L);
+ worker_spi_naptime*1000L);
ResetLatch(&MyProc->procLatch);
/* emergency bailout if postmaster has died */
if (rc & WL_POSTMASTER_DEATH)
proc_exit(1);
+ /*
+ * In case of a sighup, just reload the configuration.
+ */
+ if (got_sighup)
+ {
+ got_sighup = false;
+ ProcessConfigFile(PGC_SIGHUP);
+ }
+
StartTransactionCommand();
SPI_connect();
PushActiveSnapshot(GetTransactionSnapshot());
@@ -225,11 +239,40 @@ _PG_init(void)
{
BackgroundWorker worker;
worktable *table;
+ unsigned int i;
+ char name[20];
+
+ /* get the configuration */
+ DefineCustomIntVariable("worker_spi.naptime",
+ "Duration between each check (in seconds).",
+ NULL,
+ &worker_spi_naptime,
+ 1,
+ 1,
+ INT_MAX,
+ PGC_SIGHUP,
+ 0,
+ NULL,
+ NULL,
+ NULL);
+ DefineCustomIntVariable("worker_spi.total_workers",
+ "Number of workers.",
+ NULL,
+ &worker_spi_total_workers,
+ 2,
+ 1,
+ 100,
+ PGC_POSTMASTER,
+ 0,
+ NULL,
+ NULL,
+ NULL);
/* register the worker processes. These values are common for both */
worker.bgw_flags = BGWORKER_SHMEM_ACCESS |
BGWORKER_BACKEND_DATABASE_CONNECTION;
worker.bgw_start_time = BgWorkerStart_RecoveryFinished;
+ worker.bgw_restart_time = BGW_NEVER_RESTART;
worker.bgw_main = worker_spi_main;
worker.bgw_sighup = worker_spi_sighup;
worker.bgw_sigterm = worker_spi_sigterm;
@@ -242,22 +285,17 @@ _PG_init(void)
* memory in the child process; and if we fork and then exec, the exec'd
* process will run this code again, and so the memory is also valid there.
*/
- table = palloc(sizeof(worktable));
- table->schema = pstrdup("schema1");
- table->name = pstrdup("counted");
+ for (i = 1; i <= worker_spi_total_workers; i++)
+ {
+ sprintf(name, "worker %d", i);
+ worker.bgw_name = pstrdup(name);
- worker.bgw_name = "SPI worker 1";
- worker.bgw_restart_time = BGW_NEVER_RESTART;
- worker.bgw_main_arg = (void *) table;
- RegisterBackgroundWorker(&worker);
-
- /* Values for the second worker */
- table = palloc(sizeof(worktable));
- table->schema = pstrdup("our schema2");
- table->name = pstrdup("counted rows");
-
- worker.bgw_name = "SPI worker 2";
- worker.bgw_restart_time = 2;
- worker.bgw_main_arg = (void *) table;
- RegisterBackgroundWorker(&worker);
+ table = palloc(sizeof(worktable));
+ sprintf(name, "schema%d", i);
+ table->schema = pstrdup(name);
+ table->name = pstrdup("counted");
+ worker.bgw_main_arg = (void *) table;
+
+ RegisterBackgroundWorker(&worker);
+ }
}
Import Notes
Reply to msg id not found: 20121231204430.GR4363@alvh.no-ip.org
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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers