Re: [PATCHES] [HACKERS] Stats processor not restarting
Tom Lane wrote:
Bruce Momjian [EMAIL PROTECTED] writes:
Tom Lane wrote:
Not really, but maybe it would be sensible to reset last_pgstat_start_time
when doing a database-wide restart?
You mean like this, attached?
I'd be fairly surprised if resetting the variable in the child process
had any effect on the postmaster...
Yep, me too. ;-)
I think a correct fix would involve exposing either the variable or a
routine to zero it from pgstat.c, and having postmaster.c do that at
the points where it intentionally SIGQUITs the stats collector.
OK, patch attached. I just reset the value in all places where we were
killing the pgstat process and not immediately exiting ourselves.
--
Bruce Momjian [EMAIL PROTECTED] http://momjian.us
EnterpriseDB http://www.enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
Index: src/backend/postmaster/pgstat.c
===
RCS file: /cvsroot/pgsql/src/backend/postmaster/pgstat.c,v
retrieving revision 1.149
diff -c -c -r1.149 pgstat.c
*** src/backend/postmaster/pgstat.c 16 Mar 2007 17:57:36 - 1.149
--- src/backend/postmaster/pgstat.c 22 Mar 2007 14:40:36 -
***
*** 572,577
--- 572,581
return 0;
}
+ void allow_immediate_pgstat_restart(void)
+ {
+ last_pgstat_start_time = 0;
+ }
/*
* Public functions used by backends follow
Index: src/backend/postmaster/postmaster.c
===
RCS file: /cvsroot/pgsql/src/backend/postmaster/postmaster.c,v
retrieving revision 1.526
diff -c -c -r1.526 postmaster.c
*** src/backend/postmaster/postmaster.c 7 Mar 2007 13:35:02 - 1.526
--- src/backend/postmaster/postmaster.c 22 Mar 2007 14:40:37 -
***
*** 1896,1902
--- 1896,1905
signal_child(PgArchPID, SIGQUIT);
/* Tell pgstat to shut down too; nothing left for it to
do */
if (PgStatPID != 0)
+ {
signal_child(PgStatPID, SIGQUIT);
+ allow_immediate_pgstat_restart();
+ }
/* Tell autovac launcher to shut down too */
if (AutoVacPID != 0)
signal_child(AutoVacPID, SIGTERM);
***
*** 1952,1958
--- 1955,1964
signal_child(PgArchPID, SIGQUIT);
/* Tell pgstat to shut down too; nothing left for it to
do */
if (PgStatPID != 0)
+ {
signal_child(PgStatPID, SIGQUIT);
+ allow_immediate_pgstat_restart();
+ }
/* Tell autovac launcher to shut down too */
if (AutoVacPID != 0)
signal_child(AutoVacPID, SIGTERM);
***
*** 2241,2247
--- 2247,2256
signal_child(PgArchPID, SIGQUIT);
/* Tell pgstat to shut down too; nothing left for it to do */
if (PgStatPID != 0)
+ {
signal_child(PgStatPID, SIGQUIT);
+ allow_immediate_pgstat_restart();
+ }
/* Tell autovac launcher to shut down too */
if (AutoVacPID != 0)
signal_child(AutoVacPID, SIGTERM);
***
*** 2404,2409
--- 2413,2419
SIGQUIT,
(int)
PgStatPID)));
signal_child(PgStatPID, SIGQUIT);
+ allow_immediate_pgstat_restart();
}
/* We do NOT restart the syslogger */
Index: src/include/pgstat.h
===
RCS file: /cvsroot/pgsql/src/include/pgstat.h,v
retrieving revision 1.55
diff -c -c -r1.55 pgstat.h
*** src/include/pgstat.h16 Mar 2007 17:57:36 - 1.55
--- src/include/pgstat.h22 Mar 2007 14:40:38 -
***
*** 369,375
extern void pgstat_init(void);
extern intpgstat_start(void);
extern void pgstat_reset_all(void);
!
#ifdef EXEC_BACKEND
extern void PgstatCollectorMain(int argc, char *argv[]);
#endif
--- 369,375
extern void pgstat_init(void);
extern intpgstat_start(void);
extern void pgstat_reset_all(void);
! extern void allow_immediate_pgstat_restart(void);
#ifdef EXEC_BACKEND
extern void PgstatCollectorMain(int argc, char *argv[]);
#endif
---(end of broadcast)---
TIP 4: Have you searched our
Re: [PATCHES] [HACKERS] Stats processor not restarting
Applied. I did the case where we exit right away too, just for
consistency.
---
Bruce Momjian wrote:
Tom Lane wrote:
Bruce Momjian [EMAIL PROTECTED] writes:
Tom Lane wrote:
Not really, but maybe it would be sensible to reset
last_pgstat_start_time
when doing a database-wide restart?
You mean like this, attached?
I'd be fairly surprised if resetting the variable in the child process
had any effect on the postmaster...
Yep, me too. ;-)
I think a correct fix would involve exposing either the variable or a
routine to zero it from pgstat.c, and having postmaster.c do that at
the points where it intentionally SIGQUITs the stats collector.
OK, patch attached. I just reset the value in all places where we were
killing the pgstat process and not immediately exiting ourselves.
--
Bruce Momjian [EMAIL PROTECTED] http://momjian.us
EnterpriseDB http://www.enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
Index: src/backend/postmaster/pgstat.c
===
RCS file: /cvsroot/pgsql/src/backend/postmaster/pgstat.c,v
retrieving revision 1.149
diff -c -c -r1.149 pgstat.c
*** src/backend/postmaster/pgstat.c 16 Mar 2007 17:57:36 - 1.149
--- src/backend/postmaster/pgstat.c 22 Mar 2007 14:40:36 -
***
*** 572,577
--- 572,581
return 0;
}
+ void allow_immediate_pgstat_restart(void)
+ {
+ last_pgstat_start_time = 0;
+ }
/*
* Public functions used by backends follow
Index: src/backend/postmaster/postmaster.c
===
RCS file: /cvsroot/pgsql/src/backend/postmaster/postmaster.c,v
retrieving revision 1.526
diff -c -c -r1.526 postmaster.c
*** src/backend/postmaster/postmaster.c 7 Mar 2007 13:35:02 -
1.526
--- src/backend/postmaster/postmaster.c 22 Mar 2007 14:40:37 -
***
*** 1896,1902
--- 1896,1905
signal_child(PgArchPID, SIGQUIT);
/* Tell pgstat to shut down too; nothing left for it to
do */
if (PgStatPID != 0)
+ {
signal_child(PgStatPID, SIGQUIT);
+ allow_immediate_pgstat_restart();
+ }
/* Tell autovac launcher to shut down too */
if (AutoVacPID != 0)
signal_child(AutoVacPID, SIGTERM);
***
*** 1952,1958
--- 1955,1964
signal_child(PgArchPID, SIGQUIT);
/* Tell pgstat to shut down too; nothing left for it to
do */
if (PgStatPID != 0)
+ {
signal_child(PgStatPID, SIGQUIT);
+ allow_immediate_pgstat_restart();
+ }
/* Tell autovac launcher to shut down too */
if (AutoVacPID != 0)
signal_child(AutoVacPID, SIGTERM);
***
*** 2241,2247
--- 2247,2256
signal_child(PgArchPID, SIGQUIT);
/* Tell pgstat to shut down too; nothing left for it to do */
if (PgStatPID != 0)
+ {
signal_child(PgStatPID, SIGQUIT);
+ allow_immediate_pgstat_restart();
+ }
/* Tell autovac launcher to shut down too */
if (AutoVacPID != 0)
signal_child(AutoVacPID, SIGTERM);
***
*** 2404,2409
--- 2413,2419
SIGQUIT,
(int)
PgStatPID)));
signal_child(PgStatPID, SIGQUIT);
+ allow_immediate_pgstat_restart();
}
/* We do NOT restart the syslogger */
Index: src/include/pgstat.h
===
RCS file: /cvsroot/pgsql/src/include/pgstat.h,v
retrieving revision 1.55
diff -c -c -r1.55 pgstat.h
*** src/include/pgstat.h 16 Mar 2007 17:57:36 - 1.55
--- src/include/pgstat.h 22 Mar 2007 14:40:38 -
***
*** 369,375
extern void pgstat_init(void);
extern int pgstat_start(void);
extern void pgstat_reset_all(void);
!
#ifdef EXEC_BACKEND
extern void PgstatCollectorMain(int argc, char *argv[]);
#endif
--- 369,375
extern void pgstat_init(void);
extern int pgstat_start(void);
extern void pgstat_reset_all(void);
! extern
Re: [PATCHES] [HACKERS] Stats processor not restarting
Tom Lane wrote:
Magnus Hagander [EMAIL PROTECTED] writes:
Bah, sorry about the noise. It was the effect of
PGSTAT_RESTART_INTERVAL.
Do we want to add some logging when we don't restart it due to repeated
failures?
Not really, but maybe it would be sensible to reset last_pgstat_start_time
when doing a database-wide restart? The motivation for the timeout was
to reduce cycle wastage if pgstat crashed by itself, but when you've
deliberately SIGQUITed it, that hardly seems to apply ...
You mean like this, attached?
--
Bruce Momjian [EMAIL PROTECTED] http://momjian.us
EnterpriseDB http://www.enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
Index: src/backend/postmaster/pgstat.c
===
RCS file: /cvsroot/pgsql/src/backend/postmaster/pgstat.c,v
retrieving revision 1.149
diff -c -c -r1.149 pgstat.c
*** src/backend/postmaster/pgstat.c 16 Mar 2007 17:57:36 - 1.149
--- src/backend/postmaster/pgstat.c 22 Mar 2007 02:03:31 -
***
*** 1929,1934
--- 1929,1937
static void
pgstat_exit(SIGNAL_ARGS)
{
+ /* allow stats to restart immediately after SIGQUIT */
+ last_pgstat_start_time = 0;
+
need_exit = true;
}
---(end of broadcast)---
TIP 1: if posting/reading through Usenet, please send an appropriate
subscribe-nomail command to [EMAIL PROTECTED] so that your
message can get through to the mailing list cleanly
Re: [PATCHES] [HACKERS] Stats processor not restarting
Bruce Momjian [EMAIL PROTECTED] writes: Tom Lane wrote: Not really, but maybe it would be sensible to reset last_pgstat_start_time when doing a database-wide restart? You mean like this, attached? I'd be fairly surprised if resetting the variable in the child process had any effect on the postmaster... I think a correct fix would involve exposing either the variable or a routine to zero it from pgstat.c, and having postmaster.c do that at the points where it intentionally SIGQUITs the stats collector. regards, tom lane ---(end of broadcast)--- TIP 6: explain analyze is your friend
