Re: [PATCHES] pgstat: delayed write of stats file
I assume we no longer need this stats patch from May of 2006.
---
Bruce Momjian wrote:
> Tom Lane wrote:
> > Bruce Momjian writes:
> > > Have we seen any such failures since the first day they appeared?
> >
> > agouti blew up about the same time you typed that, so yes it's still
> > a problem.
> >
> > http://www.pgbuildfarm.org/cgi-bin/show_log.pl?nm=agouti&dt=2006-05-08%2003:15:01
>
> Delay pgstat file write patch reverted.
>
> --
> Bruce Momjian http://candle.pha.pa.us
> EnterpriseDBhttp://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.123
> retrieving revision 1.124
> diff -c -r1.123 -r1.124
> *** src/backend/postmaster/pgstat.c 20 Apr 2006 10:51:32 - 1.123
> --- src/backend/postmaster/pgstat.c 27 Apr 2006 00:06:58 - 1.124
> ***
> ***
> *** 28,33
> --- 28,34
> #include
> #include
> #include
> + #include
>
> #include "pgstat.h"
>
> ***
> *** 66,77
>* Timer definitions.
>* --
>*/
> - #define PGSTAT_STAT_INTERVAL500 /* How often to write
> the status file;
> -
> * in milliseconds. */
>
> ! #define PGSTAT_RESTART_INTERVAL 60 /* How often to attempt to
> restart a
> !
> * failed statistics collector; in
> !
> * seconds. */
>
> /* --
>* Amount of space reserved in pgstat_recvbuffer().
> --- 67,81
>* Timer definitions.
>* --
>*/
>
> ! /* How often to write the status file, in milliseconds. */
> ! #define PGSTAT_STAT_INTERVAL(5*60*1000)
> !
> ! /*
> ! * How often to attempt to restart a failed statistics collector; in ms.
> ! * Must be at least PGSTAT_STAT_INTERVAL.
> ! */
> ! #define PGSTAT_RESTART_INTERVAL (5*60*1000)
>
> /* --
>* Amount of space reserved in pgstat_recvbuffer().
> ***
> *** 172,182
> static void pgstat_write_statsfile(void);
> static void pgstat_read_statsfile(HTAB **dbhash, Oid onlydb,
> PgStat_StatBeEntry **betab,
> ! int *numbackends);
> static void backend_read_statsfile(void);
>
> static void pgstat_setheader(PgStat_MsgHdr *hdr, StatMsgType mtype);
> static void pgstat_send(void *msg, int len);
>
> static void pgstat_recv_bestart(PgStat_MsgBestart *msg, int len);
> static void pgstat_recv_beterm(PgStat_MsgBeterm *msg, int len);
> --- 176,187
> static void pgstat_write_statsfile(void);
> static void pgstat_read_statsfile(HTAB **dbhash, Oid onlydb,
> PgStat_StatBeEntry **betab,
> ! int *numbackends, bool rewrite);
> static void backend_read_statsfile(void);
>
> static void pgstat_setheader(PgStat_MsgHdr *hdr, StatMsgType mtype);
> static void pgstat_send(void *msg, int len);
> + static void pgstat_send_rewrite(void);
>
> static void pgstat_recv_bestart(PgStat_MsgBestart *msg, int len);
> static void pgstat_recv_beterm(PgStat_MsgBeterm *msg, int len);
> ***
> *** 1449,1454
> --- 1454,1477
> #endif
> }
>
> + /*
> + * pgstat_send_rewrite() -
> + *
> + * Send a command to the collector to rewrite the stats file.
> + * --
> + */
> + static void
> + pgstat_send_rewrite(void)
> + {
> + PgStat_MsgRewrite msg;
> +
> + if (pgStatSock < 0)
> + return;
> +
> + pgstat_setheader(&msg.m_hdr, PGSTAT_MTYPE_REWRITE);
> + pgstat_send(&msg, sizeof(msg));
> + }
> +
>
> /* --
>* PgstatBufferMain() -
> ***
> *** 1549,1555
> fd_set rfds;
> int readPipe;
> int len = 0;
> ! struct itimerval timeout;
> boolneed_timer = false;
>
> MyProcPid = getpid(); /* reset MyProcPid */
> --- 1572,1578
> fd_set rfds;
> int readPipe;
> int len = 0;
> ! struct itimerval timeout, canceltimeout;
> boolneed_timer = false;
>
> MyProcPid = getpid(); /* reset MyProcPid */
> ***
> *** 1604,1615
> timeout.it_value.tv_sec = PGSTAT_STAT_INTERVAL / 1000;
> timeout.it_value.tv_usec = PGSTAT_STAT_INTERVAL % 1000;
>
> /*
>* Read in an existing statistics stats file or initialize the stats to
>
Re: [PATCHES] pgstat: delayed write of stats file
Tom Lane wrote:
> Bruce Momjian writes:
> > Have we seen any such failures since the first day they appeared?
>
> agouti blew up about the same time you typed that, so yes it's still
> a problem.
>
> http://www.pgbuildfarm.org/cgi-bin/show_log.pl?nm=agouti&dt=2006-05-08%2003:15:01
Delay pgstat file write patch reverted.
--
Bruce Momjian http://candle.pha.pa.us
EnterpriseDBhttp://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.123
retrieving revision 1.124
diff -c -r1.123 -r1.124
*** src/backend/postmaster/pgstat.c 20 Apr 2006 10:51:32 - 1.123
--- src/backend/postmaster/pgstat.c 27 Apr 2006 00:06:58 - 1.124
***
***
*** 28,33
--- 28,34
#include
#include
#include
+ #include
#include "pgstat.h"
***
*** 66,77
* Timer definitions.
* --
*/
- #define PGSTAT_STAT_INTERVAL 500 /* How often to write the
status file;
-
* in milliseconds. */
! #define PGSTAT_RESTART_INTERVAL 60/* How often to attempt to
restart a
!
* failed statistics collector; in
!
* seconds. */
/* --
* Amount of space reserved in pgstat_recvbuffer().
--- 67,81
* Timer definitions.
* --
*/
! /* How often to write the status file, in milliseconds. */
! #define PGSTAT_STAT_INTERVAL (5*60*1000)
!
! /*
! *How often to attempt to restart a failed statistics collector; in ms.
! *Must be at least PGSTAT_STAT_INTERVAL.
! */
! #define PGSTAT_RESTART_INTERVAL (5*60*1000)
/* --
* Amount of space reserved in pgstat_recvbuffer().
***
*** 172,182
static void pgstat_write_statsfile(void);
static void pgstat_read_statsfile(HTAB **dbhash, Oid onlydb,
PgStat_StatBeEntry **betab,
! int *numbackends);
static void backend_read_statsfile(void);
static void pgstat_setheader(PgStat_MsgHdr *hdr, StatMsgType mtype);
static void pgstat_send(void *msg, int len);
static void pgstat_recv_bestart(PgStat_MsgBestart *msg, int len);
static void pgstat_recv_beterm(PgStat_MsgBeterm *msg, int len);
--- 176,187
static void pgstat_write_statsfile(void);
static void pgstat_read_statsfile(HTAB **dbhash, Oid onlydb,
PgStat_StatBeEntry **betab,
! int *numbackends, bool rewrite);
static void backend_read_statsfile(void);
static void pgstat_setheader(PgStat_MsgHdr *hdr, StatMsgType mtype);
static void pgstat_send(void *msg, int len);
+ static void pgstat_send_rewrite(void);
static void pgstat_recv_bestart(PgStat_MsgBestart *msg, int len);
static void pgstat_recv_beterm(PgStat_MsgBeterm *msg, int len);
***
*** 1449,1454
--- 1454,1477
#endif
}
+ /*
+ * pgstat_send_rewrite() -
+ *
+ *Send a command to the collector to rewrite the stats file.
+ * --
+ */
+ static void
+ pgstat_send_rewrite(void)
+ {
+ PgStat_MsgRewrite msg;
+
+ if (pgStatSock < 0)
+ return;
+
+ pgstat_setheader(&msg.m_hdr, PGSTAT_MTYPE_REWRITE);
+ pgstat_send(&msg, sizeof(msg));
+ }
+
/* --
* PgstatBufferMain() -
***
*** 1549,1555
fd_set rfds;
int readPipe;
int len = 0;
! struct itimerval timeout;
boolneed_timer = false;
MyProcPid = getpid(); /* reset MyProcPid */
--- 1572,1578
fd_set rfds;
int readPipe;
int len = 0;
! struct itimerval timeout, canceltimeout;
boolneed_timer = false;
MyProcPid = getpid(); /* reset MyProcPid */
***
*** 1604,1615
timeout.it_value.tv_sec = PGSTAT_STAT_INTERVAL / 1000;
timeout.it_value.tv_usec = PGSTAT_STAT_INTERVAL % 1000;
/*
* Read in an existing statistics stats file or initialize the stats to
* zero.
*/
pgStatRunningInCollector = true;
! pgstat_read_statsfile(&pgStatDBHash, InvalidOid, NULL, NULL);
/*
* Create the known backends table
--- 1627,1641
timeout.it_value.tv_sec = PGSTAT_STAT_INTERVAL / 1000;
timeout.it_value.tv_usec = PGSTAT_STAT_INTERVAL % 1000;
+ /* Values set to zero will cancel the active timer
Re: [PATCHES] pgstat: delayed write of stats file
Bruce Momjian writes: > Have we seen any such failures since the first day they appeared? agouti blew up about the same time you typed that, so yes it's still a problem. http://www.pgbuildfarm.org/cgi-bin/show_log.pl?nm=agouti&dt=2006-05-08%2003:15:01 regards, tom lane ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [PATCHES] pgstat: delayed write of stats file
Tom Lane wrote: > Bruce Momjian writes: > > Was this fixed somehow? I don't see any more buildfarm failures, but I > > didn't see a fix go in either. It is very possible the pgport fix I did > > yesterday is related, but I have not applied it yet. > > It's an intermittent failure; the fact you don't see any at this instant > doesn't mean it's fixed. I don't think your patch of yesterday is > related, since we're seeing the problem on non-WIN32 machines. Have we seen any such failures since the first day they appeared? -- Bruce Momjian http://candle.pha.pa.us EnterpriseDBhttp://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] pgstat: delayed write of stats file
Bruce Momjian writes: > Was this fixed somehow? I don't see any more buildfarm failures, but I > didn't see a fix go in either. It is very possible the pgport fix I did > yesterday is related, but I have not applied it yet. It's an intermittent failure; the fact you don't see any at this instant doesn't mean it's fixed. I don't think your patch of yesterday is related, since we're seeing the problem on non-WIN32 machines. regards, tom lane ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] pgstat: delayed write of stats file
Magnus Hagander wrote: > > >>> This was not ready to be applied, was it? > > > > > I don't recall any specific objections that weren't answered. > > > > How about the fact that it's already caused one buildfarm failure? > > > > http://www.pgbuildfarm.org/cgi-bin/show_log.pl?nm=wasp&dt=2006 > -04-30%2003:05:01 > > > Well, that objection certainly wasn't raised before since it wasn't on > buildfarm, and it passed the regression tests many times on my systems. > > Oh, and apparantly it caused the same issue on firefly. I don't really > see the connection between these two platforms and why it should happen > just there, though. Any ideas on that part? Was this fixed somehow? I don't see any more buildfarm failures, but I didn't see a fix go in either. It is very possible the pgport fix I did yesterday is related, but I have not applied it yet. -- Bruce Momjian http://candle.pha.pa.us EnterpriseDBhttp://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] pgstat: delayed write of stats file
> >>> This was not ready to be applied, was it? > > > I don't recall any specific objections that weren't answered. > > How about the fact that it's already caused one buildfarm failure? > > http://www.pgbuildfarm.org/cgi-bin/show_log.pl?nm=wasp&dt=2006 -04-30%2003:05:01 Well, that objection certainly wasn't raised before since it wasn't on buildfarm, and it passed the regression tests many times on my systems. Oh, and apparantly it caused the same issue on firefly. I don't really see the connection between these two platforms and why it should happen just there, though. Any ideas on that part? //Magnus ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] pgstat: delayed write of stats file
"Magnus Hagander" <[EMAIL PROTECTED]> writes: >>> This was not ready to be applied, was it? > I don't recall any specific objections that weren't answered. How about the fact that it's already caused one buildfarm failure? http://www.pgbuildfarm.org/cgi-bin/show_log.pl?nm=wasp&dt=2006-04-30%2003:05:01 regards, tom lane ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [PATCHES] pgstat: delayed write of stats file
> > > Magnus Hagander wrote: > > >> Per some earlier discussion, here is an attempt at > implementing a > > >> "delayed write" of the pgstats file, to decrease the > write activity > > >> on that file. > > > > This was not ready to be applied, was it? "An attempt" > doesn't sound > > to me like Magnus thought it was ready for prime time, and my > > recollection of the thread is that there were objections. > > I don't remember any objection. Magnus, Tom, revert? I don't recall any specific objections that weren't answered. There was some (short) talk about having a "backoff" so it won't update the file too often if the user requests it, but I don't think we ever concluded if that was necessary or not. If it is, I think we can add that on top of what's there now, and there's no need to revert. If there were/are other objections that I missed, a revert might be needed. Can't really comment since I missed them in that case ;-) I think the use of "attempt" was more that I expected more comments, and also somewhat in light of the concurrent discussions about getting rid of the code to accept delayed destroy messages. //Magnus ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [PATCHES] pgstat: delayed write of stats file
Tom Lane wrote: > Bruce Momjian writes: > > Patch applied. Thanks. > > > Magnus Hagander wrote: > >> Per some earlier discussion, here is an attempt at implementing a > >> "delayed write" of the pgstats file, to decrease the write activity on > >> that file. > > This was not ready to be applied, was it? "An attempt" doesn't sound > to me like Magnus thought it was ready for prime time, and my > recollection of the thread is that there were objections. I don't remember any objection. Magnus, Tom, revert? -- Bruce Momjian http://candle.pha.pa.us EnterpriseDBhttp://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + ---(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] pgstat: delayed write of stats file
Bruce Momjian writes: > Patch applied. Thanks. > Magnus Hagander wrote: >> Per some earlier discussion, here is an attempt at implementing a >> "delayed write" of the pgstats file, to decrease the write activity on >> that file. This was not ready to be applied, was it? "An attempt" doesn't sound to me like Magnus thought it was ready for prime time, and my recollection of the thread is that there were objections. regards, tom lane ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] pgstat: delayed write of stats file
Patch applied. Thanks. --- Magnus Hagander wrote: > Per some earlier discussion, here is an attempt at implementing a > "delayed write" of the pgstats file, to decrease the write activity on > that file. > > It changes so the file is only written once every 5 minutes (changeable > of course, I just picked something) instead of once every half second. > It's still written when the stats collector shuts down, just as before. > And it is now also written on backend request. A backend requests a > rewrite by simply sending a special stats message. It operates on the > assumption that the backends aren't actually going to read the > statistics file very often, compared to how frequent it's written today. > > > //Magnus Content-Description: pgstat_delay.patch [ Attachment, skipping... ] > > ---(end of broadcast)--- > TIP 2: Don't 'kill -9' the postmaster -- Bruce Momjian http://candle.pha.pa.us EnterpriseDBhttp://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [PATCHES] pgstat: delayed write of stats file
> > And it is now also written on backend request. A backend requests a
> > rewrite by simply sending a special stats message. It
> operates on the
> > assumption that the backends aren't actually going to read the
> > statistics file very often, compared to how frequent it's
> written today.
>
> While that would be better under low load, seems like it
> would be markedly worse under high load (ie, if someone
> actually is watching the stats constantly).
Only if "constantly" means "more than twice per second" does it get
worse than today (and of course assuming you run it in different
transactions, because it still caches the values inside a transaction
just as before).
I was considering implementing some sort of backoff ("already written
this half second, won't write a new one"), but I couldn't really come up
with a plausible scenario for it. For myself, even when I run a monitor
set to refresh often, that's set to once every 10 seconds - if it's
configged to do it "really often", that's once / second, which is still
better than before. And I'd not normally run it set that fast on a
system that's performance sensitive...
(And if I set it to > 1/0.5 sec I won't even get updated data on current
versions, whereas with this patch I do - at the expense of performance)
//Magnus
---(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] pgstat: delayed write of stats file
"Magnus Hagander" <[EMAIL PROTECTED]> writes: > And it is now also written on backend request. A backend requests a > rewrite by simply sending a special stats message. It operates on the > assumption that the backends aren't actually going to read the > statistics file very often, compared to how frequent it's written today. While that would be better under low load, seems like it would be markedly worse under high load (ie, if someone actually is watching the stats constantly). regards, tom lane ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [PATCHES] pgstat: delayed write of stats file
> > Per some earlier discussion, here is an attempt at implementing a > > "delayed write" of the pgstats file, to decrease the write > activity on > > that file. > > > > It changes so the file is only written once every 5 minutes > > (changeable of course, I just picked something) instead of > once every half second. > > It's still written when the stats collector shuts down, > just as before. > > And it is now also written on backend request. A backend requests a > > rewrite by simply sending a special stats message. It > operates on the > > assumption that the backends aren't actually going to read the > > statistics file very often, compared to how frequent it's > written today. > > If somebody tries to read something from the pgstat file, is > the backend going to request a rewrite and wait until it is complete? Not sure which part you mean, so I'll answer both ;) Scenario: One backend reads the file. Second backend requests rewrite. The write is initiated and later renamed()d into place. As the first backend still has the file open it'll just continue to read off that version of the file. The requesting backend will wait for the file to change, and then read the new version. So it'll wait for a complete rewrite of the file (with a timeout of course, in which case we'll read the old file and report a warning). //Magnus ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] pgstat: delayed write of stats file
Magnus Hagander wrote: > Per some earlier discussion, here is an attempt at implementing a > "delayed write" of the pgstats file, to decrease the write activity on > that file. > > It changes so the file is only written once every 5 minutes (changeable > of course, I just picked something) instead of once every half second. > It's still written when the stats collector shuts down, just as before. > And it is now also written on backend request. A backend requests a > rewrite by simply sending a special stats message. It operates on the > assumption that the backends aren't actually going to read the > statistics file very often, compared to how frequent it's written today. If somebody tries to read something from the pgstat file, is the backend going to request a rewrite and wait until it is complete? -- Alvaro Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
