Re: [PATCHES] pgstat: delayed write of stats file

2007-02-08 Thread Bruce Momjian

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

2006-05-29 Thread Bruce Momjian
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

2006-05-07 Thread Tom Lane
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

2006-05-07 Thread Bruce Momjian
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

2006-05-07 Thread Tom Lane
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

2006-05-07 Thread Bruce Momjian
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

2006-04-30 Thread Magnus Hagander
> >>> 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

2006-04-30 Thread Tom Lane
"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

2006-04-27 Thread Magnus Hagander
> > > 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

2006-04-26 Thread Bruce Momjian
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

2006-04-26 Thread Tom Lane
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

2006-04-26 Thread Bruce Momjian

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

2006-04-05 Thread Magnus Hagander
> > 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

2006-04-05 Thread Tom Lane
"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

2006-04-05 Thread Magnus Hagander
> > 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

2006-04-05 Thread Alvaro Herrera
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