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 pgman@candle.pha.pa.us 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=agoutidt=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 arpa/inet.h
#include signal.h
#include time.h
+ #include sys/stat.h
#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
* zero.
*/
pgStatRunningInCollector = true;
!
Re: [PATCHES] pgstat: delayed write of stats file
Tom Lane wrote:
Bruce Momjian pgman@candle.pha.pa.us 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=agoutidt=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 arpa/inet.h
#include signal.h
#include time.h
+ #include sys/stat.h
#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;
+ /*
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=waspdt=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
Bruce Momjian pgman@candle.pha.pa.us 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
Tom Lane wrote: Bruce Momjian pgman@candle.pha.pa.us 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 pgman@candle.pha.pa.us 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=agoutidt=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
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=waspdt=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
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=waspdt=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 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
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
Bruce Momjian pgman@candle.pha.pa.us 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
Tom Lane wrote: Bruce Momjian pgman@candle.pha.pa.us 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
[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. //Magnus pgstat_delay.patch Description: pgstat_delay.patch ---(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
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 [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
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
