Re: [HACKERS] keeping a timestamp of the last stats reset (for a db, table and function)

2011-02-10 Thread Magnus Hagander
On Sun, Feb 6, 2011 at 08:17, Greg Smith g...@2ndquadrant.com wrote:
 Tomas Vondra wrote:

 Because when I create a database, the field is
 NULL - that's true. But once I connect to the database, the stats are
 updated and the field is set (thanks to the logic in pgstat.c).


 OK--so it does what I was hoping for, I just didn't test it the right way.
  Let's call that a documentation issue and move on.

 Attached is an updated patch that fixes the docs and some other random bits.
  Looks ready for committer to me now.  Make sure to adjust
 PGSTAT_FILE_FORMAT_ID, do a cat version bump, and set final OIDs for the new
 functions.

... and the regression tests expected output.


 -Fixed some tab/whitespace issues.  It looks like you had tab stops set at 8

I added a few more whitespace fixes, mainly in the whitespace at end
of line category (git diff shows it pretty clearly)


With that, applied. Thanks!

-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] keeping a timestamp of the last stats reset (for a db, table and function)

2011-02-06 Thread Tomas Vondra
On 6.2.2011 08:17, Greg Smith wrote:
 Below is what changed since the last posted version, mainly as feedback
 for Tomas:
 
 -Explained more clearly that pg_stat_reset and
 pg_stat_reset_single_counters will both touch the database reset time,
 and that it's initialized upon first connection to the database.
 
 -Added the reset time to the list of fields in pg_stat_database and
 pg_stat_bgwriter.
 
 -Fixed some tab/whitespace issues.  It looks like you had tab stops set
 at 8 characters during some points when you were editing non-code
 files.  Also, there were a couple of spot where you used a tab while
 text in the area used spaces.  You can normally see both types of errors
 if you read a patch, they showed up as misaligned things in the context
 diff.
 
 -Removed some extra blank lines that didn't fit the style of the
 surrounding code.
 
 Basically, all the formatting bits I'm nitpicking about I found just by
 reading the patch itself; they all stuck right out.  I'd recommend a
 pass of that before submitting things if you want to try and avoid those
 in the future.

Thanks for fixing the issues and for the review in general. I have to
tune the IDE a bit to produce the right format and be a bit more careful
when creating the patches.

regards
Tomas

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] keeping a timestamp of the last stats reset (for a db, table and function)

2011-02-05 Thread Greg Smith

Tomas Vondra wrote:

Because when I create a database, the field is
NULL - that's true. But once I connect to the database, the stats are
updated and the field is set (thanks to the logic in pgstat.c).
  


OK--so it does what I was hoping for, I just didn't test it the right 
way.  Let's call that a documentation issue and move on.


Attached is an updated patch that fixes the docs and some other random 
bits.  Looks ready for committer to me now.  Make sure to adjust 
PGSTAT_FILE_FORMAT_ID, do a cat version bump, and set final OIDs for the 
new functions.


Below is what changed since the last posted version, mainly as feedback 
for Tomas:


-Explained more clearly that pg_stat_reset and 
pg_stat_reset_single_counters will both touch the database reset time, 
and that it's initialized upon first connection to the database.


-Added the reset time to the list of fields in pg_stat_database and 
pg_stat_bgwriter.


-Fixed some tab/whitespace issues.  It looks like you had tab stops set 
at 8 characters during some points when you were editing non-code 
files.  Also, there were a couple of spot where you used a tab while 
text in the area used spaces.  You can normally see both types of errors 
if you read a patch, they showed up as misaligned things in the context 
diff.


-Removed some extra blank lines that didn't fit the style of the 
surrounding code.


Basically, all the formatting bits I'm nitpicking about I found just by 
reading the patch itself; they all stuck right out.  I'd recommend a 
pass of that before submitting things if you want to try and avoid those 
in the future.


--
Greg Smith   2ndQuadrant USg...@2ndquadrant.com   Baltimore, MD
PostgreSQL Training, Services, and 24x7 Support  www.2ndQuadrant.us
PostgreSQL 9.0 High Performance: http://www.2ndQuadrant.com/books

diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml
index ca83421..100f938 100644
*** a/doc/src/sgml/monitoring.sgml
--- b/doc/src/sgml/monitoring.sgml
*** postgres: replaceableuser/ replacea
*** 267,273 
by backends (that is, not by the background writer), how many times
those backends had to execute their own fsync calls (normally the
background writer handles those even when the backend does its own
!   write), and total buffers allocated.
   /entry
   /row
  
--- 267,273 
by backends (that is, not by the background writer), how many times
those backends had to execute their own fsync calls (normally the
background writer handles those even when the backend does its own
!   write), total buffers allocated, and time of last statistics reset.
   /entry
   /row
  
*** postgres: replaceableuser/ replacea
*** 278,286 
number of transactions committed and rolled back in that database,
total disk blocks read, total buffer hits (i.e., block
read requests avoided by finding the block already in buffer cache),
!   number of rows returned, fetched, inserted, updated and deleted, and
total number of queries cancelled due to conflict with recovery (on
!   standby servers).
   /entry
   /row
  
--- 278,286 
number of transactions committed and rolled back in that database,
total disk blocks read, total buffer hits (i.e., block
read requests avoided by finding the block already in buffer cache),
!   number of rows returned, fetched, inserted, updated and deleted, the
total number of queries cancelled due to conflict with recovery (on
!   standby servers), and time of last statistics reset.
   /entry
   /row
  
*** postgres: replaceableuser/ replacea
*** 663,668 
--- 663,681 
   /row
  
   row
+   entryliteralfunctionpg_stat_get_db_stat_reset_time/function(typeoid/type)/literal/entry
+   entrytypetimestamptz/type/entry
+   entry
+Time of the last statistics reset for the database.  Initialized to the
+system time during the first connection to each database.  The reset time
+is updated when you call functionpg_stat_reset/function on the
+database, as well as upon execution of
+functionpg_stat_reset_single_table_counters/function against any
+table or index in it.
+   /entry
+  /row
+ 
+  row
entryliteralfunctionpg_stat_get_numscans/function(typeoid/type)/literal/entry
entrytypebigint/type/entry
entry
*** postgres: replaceableuser/ replacea
*** 1125,1130 
--- 1138,1153 
 varnamebgwriter_lru_maxpages/varname parameter
/entry
   /row
+  
+  row
+   entryliteralfunctionpg_stat_get_bgwriter_stat_reset_time()/function/literal/entry
+   entrytypetimestamptz/type/entry
+   entry
+ Time of the last statistics reset for the background writer, updated
+ when executing functionpg_stat_reset_shared('bgwriter')/function
+ 

Re: [HACKERS] keeping a timestamp of the last stats reset (for a db, table and function)

2011-02-04 Thread Tomas Vondra
Dne 4.2.2011 03:37, Greg Smith napsal(a):
 Thinking I should start with why I think this patch is neat...most of
 the servers I deal with are up 24x7 minus small amounts of downtime,
 presuming everyone does their job right that is.  In that environment,
 having a starting timestamp for when the last stats reset happened lets
 you quickly compute some figures in per-second terms that are pretty
 close to actual average activity on the server.  Some examples of how I
 would use this:
 
 psql -c 
 SELECT
  CAST(buffers_backend * block_size AS numeric) / seconds_uptime /
 (1024*1024)
AS backend_mb_per_sec
 FROM
  (SELECT *,extract(epoch from now()-stats_reset) AS seconds_uptime,
  (SELECT cast(current_setting('block_size') AS int8)) AS block_size
   FROM pg_stat_bgwriter) AS raw WHERE raw.seconds_uptime  0
 
 backend_mb_per_sec
 
   4.27150807681618
 
 psql -c 
 SELECT
  datname,CAST(xact_commit AS numeric) / seconds_uptime
AS commits_per_sec
 FROM
  (SELECT *,extract(epoch from now()-stats_reset) AS seconds_uptime
   FROM pg_stat_database) AS raw WHERE raw.seconds_uptime  0
 
 
  datname  |  commits_per_sec  ---+
 template1 | 0.0338722604313051
 postgres  | 0.0363144438470267
 gsmith| 0.0820573653236174
 pgbench   |  0.059147072347085
 
 Now I reset, put some load on the system and check the same stats
 afterward; watch how close these match up:
 
 $ psql -d pgbench -c select pg_stat_reset()
 $ pgbench -j 4 -c 32 -T 30 pgbench
 transaction type: TPC-B (sort of)
 scaling factor: 100
 query mode: simple
 number of clients: 32
 number of threads: 4
 duration: 30 s
 number of transactions actually processed: 6604
 tps = 207.185627 (including connections establishing)
 tps = 207.315043 (excluding connections establishing)
 
  datname  |  commits_per_sec  ---+
 pgbench   |   183.906308135572
 
 Both these examples work as I expected, and some playing around with the
 patch didn't find any serious problems with the logic it implements. 
 One issue though, an oversight I think can be improved upon; watch what
 happens when I create a new database:
 
 $ createdb blank
 $ psql -c select datname,stats_reset from pg_stat_database where
 datname='blank'
 datname | stats_reset
 -+-
 blank   |
 
 That's not really what I would hope for here.  One major sort of
 situation I'd like this feature to work against is the one where someone
 asks for help but has never touched their database stats before, which
 is exactly what I'm simulating here.  In this case that person would be
 out of luck, the opposite of the experience I'd like a newbie to have at
 this point.

Are you sure about it? Because when I create a database, the field is
NULL - that's true. But once I connect to the database, the stats are
updated and the field is set (thanks to the logic in pgstat.c).

In that case 'stat_reset=NULL' would mean 'no-one ever touched this db'
which seems quite reasonable to me.



$ createdb testdb1
$ createdb testdb2

$ psql -d testdb1 -c select stats_reset from pg_stat_database where
  datname = 'testdb2'
 stats_reset
-

(1 row)

$ psql -d testdb2 -c \q

$ psql -d testdb1 -c select stats_reset from pg_stat_database where
  datname = 'testdb2'
  stats_reset
---
 2011-02-04 19:03:23.938929+01
(1 row)



But maybe I've missed something and it does not work the way I think it
does.

regards
Tomas

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] keeping a timestamp of the last stats reset (for a db, table and function)

2011-02-03 Thread Tomas Vondra
Dne 30.1.2011 23:22, Robert Haas napsal(a):
 On Thu, Dec 23, 2010 at 2:41 PM, Tomas Vondra t...@fuzzy.cz wrote:
 OK, so here goes the simplified patch - it tracks one reset timestamp
 for a background writer and for each database.

 I think you forgot the attachment.

 Yes, I did. Thanks!
 
 This patch no longer applies.  Please update.

I've updated the patch - rebased to the current HEAD.

regards
Tomas
*** a/doc/src/sgml/monitoring.sgml
--- b/doc/src/sgml/monitoring.sgml
***
*** 663,668  postgres: replaceableuser/ replaceabledatabase/ 
replaceablehost/ re
--- 663,677 
   /row
  
   row
+   
entryliteralfunctionpg_stat_get_db_stat_reset_time/function(typeoid/type)/literal/entry
+   entrytypetimestamptz/type/entry
+   entry
+Time of the last reset of statistics for this database (as a result of 
executing
+functionpg_stat_reset/function function) or any object within it 
(table or index).
+   /entry
+  /row
+ 
+  row

entryliteralfunctionpg_stat_get_numscans/function(typeoid/type)/literal/entry
entrytypebigint/type/entry
entry
***
*** 1125,1130  postgres: replaceableuser/ replaceabledatabase/ 
replaceablehost/ re
--- 1134,1148 
 varnamebgwriter_lru_maxpages/varname parameter
/entry
   /row
+  
+  row
+   
entryliteralfunctionpg_stat_get_bgwriter_stat_reset_time()/function/literal/entry
+   entrytypetimestamptz/type/entry
+   entry
+ Time of the last reset of statistics for the background writer (as a 
result of executing
+ functionpg_stat_reset_shared('bgwriter')/function)
+   /entry
+  /row
  
   row

entryliteralfunctionpg_stat_get_buf_written_backend()/function/literal/entry
*** a/src/backend/catalog/system_views.sql
--- b/src/backend/catalog/system_views.sql
***
*** 523,529  CREATE VIEW pg_stat_database AS
  pg_stat_get_db_tuples_inserted(D.oid) AS tup_inserted,
  pg_stat_get_db_tuples_updated(D.oid) AS tup_updated,
  pg_stat_get_db_tuples_deleted(D.oid) AS tup_deleted,
! pg_stat_get_db_conflict_all(D.oid) AS conflicts
  FROM pg_database D;
  
  CREATE VIEW pg_stat_database_conflicts AS
--- 523,530 
  pg_stat_get_db_tuples_inserted(D.oid) AS tup_inserted,
  pg_stat_get_db_tuples_updated(D.oid) AS tup_updated,
  pg_stat_get_db_tuples_deleted(D.oid) AS tup_deleted,
! pg_stat_get_db_conflict_all(D.oid) AS conflicts,
! pg_stat_get_db_stat_reset_time(D.oid) AS stats_reset
  FROM pg_database D;
  
  CREATE VIEW pg_stat_database_conflicts AS
***
*** 570,576  CREATE VIEW pg_stat_bgwriter AS
  pg_stat_get_bgwriter_maxwritten_clean() AS maxwritten_clean,
  pg_stat_get_buf_written_backend() AS buffers_backend,
  pg_stat_get_buf_fsync_backend() AS buffers_backend_fsync,
! pg_stat_get_buf_alloc() AS buffers_alloc;
  
  CREATE VIEW pg_user_mappings AS
  SELECT
--- 571,578 
  pg_stat_get_bgwriter_maxwritten_clean() AS maxwritten_clean,
  pg_stat_get_buf_written_backend() AS buffers_backend,
  pg_stat_get_buf_fsync_backend() AS buffers_backend_fsync,
! pg_stat_get_buf_alloc() AS buffers_alloc,
!   pg_stat_get_bgwriter_stat_reset_time() AS stats_reset;
  
  CREATE VIEW pg_user_mappings AS
  SELECT
*** a/src/backend/postmaster/pgstat.c
--- b/src/backend/postmaster/pgstat.c
***
*** 3160,3165  pgstat_get_db_entry(Oid databaseid, bool create)
--- 3160,3167 
result-n_conflict_bufferpin = 0;
result-n_conflict_startup_deadlock = 0;
  
+   result-stat_reset_timestamp = GetCurrentTimestamp();
+ 
memset(hash_ctl, 0, sizeof(hash_ctl));
hash_ctl.keysize = sizeof(Oid);
hash_ctl.entrysize = sizeof(PgStat_StatTabEntry);
***
*** 3438,3443  pgstat_read_statsfile(Oid onlydb, bool permanent)
--- 3440,3451 
 * load an existing statsfile.
 */
memset(globalStats, 0, sizeof(globalStats));
+   
+   /*
+* Set the current timestamp (will be kept only in case we can't load an
+* existing statsfile.
+*/
+   globalStats.stat_reset_timestamp = GetCurrentTimestamp();
  
/*
 * Try to open the status file. If it doesn't exist, the backends simply
***
*** 4052,4057  pgstat_recv_resetcounter(PgStat_MsgResetcounter *msg, int 
len)
--- 4060,4067 
dbentry-n_tuples_deleted = 0;
dbentry-last_autovac_time = 0;
  
+   dbentry-stat_reset_timestamp = GetCurrentTimestamp();
+ 
memset(hash_ctl, 0, sizeof(hash_ctl));
hash_ctl.keysize = sizeof(Oid);
hash_ctl.entrysize = sizeof(PgStat_StatTabEntry);
***
*** 4083,4088  

Re: [HACKERS] keeping a timestamp of the last stats reset (for a db, table and function)

2011-02-03 Thread Greg Smith
Thinking I should start with why I think this patch is neat...most of 
the servers I deal with are up 24x7 minus small amounts of downtime, 
presuming everyone does their job right that is.  In that environment, 
having a starting timestamp for when the last stats reset happened lets 
you quickly compute some figures in per-second terms that are pretty 
close to actual average activity on the server.  Some examples of how I 
would use this:


psql -c 
SELECT
 CAST(buffers_backend * block_size AS numeric) / seconds_uptime / 
(1024*1024)

   AS backend_mb_per_sec
FROM
 (SELECT *,extract(epoch from now()-stats_reset) AS seconds_uptime,
 (SELECT cast(current_setting('block_size') AS int8)) AS block_size
  FROM pg_stat_bgwriter) AS raw WHERE raw.seconds_uptime  0

backend_mb_per_sec

  4.27150807681618

psql -c 
SELECT
 datname,CAST(xact_commit AS numeric) / seconds_uptime
   AS commits_per_sec
FROM
 (SELECT *,extract(epoch from now()-stats_reset) AS seconds_uptime
  FROM pg_stat_database) AS raw WHERE raw.seconds_uptime  0


 datname  |  commits_per_sec  
---+

template1 | 0.0338722604313051
postgres  | 0.0363144438470267
gsmith| 0.0820573653236174
pgbench   |  0.059147072347085

Now I reset, put some load on the system and check the same stats 
afterward; watch how close these match up:


$ psql -d pgbench -c select pg_stat_reset()
$ pgbench -j 4 -c 32 -T 30 pgbench
transaction type: TPC-B (sort of)
scaling factor: 100
query mode: simple
number of clients: 32
number of threads: 4
duration: 30 s
number of transactions actually processed: 6604
tps = 207.185627 (including connections establishing)
tps = 207.315043 (excluding connections establishing)

 datname  |  commits_per_sec  
---+

pgbench   |   183.906308135572

Both these examples work as I expected, and some playing around with the 
patch didn't find any serious problems with the logic it implements.  
One issue though, an oversight I think can be improved upon; watch what 
happens when I create a new database:


$ createdb blank
$ psql -c select datname,stats_reset from pg_stat_database where 
datname='blank'

datname | stats_reset
-+-
blank   |

That's not really what I would hope for here.  One major sort of 
situation I'd like this feature to work against is the one where someone 
asks for help but has never touched their database stats before, which 
is exactly what I'm simulating here.  In this case that person would be 
out of luck, the opposite of the experience I'd like a newbie to have at 
this point.


The logic Tomas put in here to initialize things in the face of never 
having a stat reset is reasonable.  But I think to really be complete, 
this needs to hook database creation and make sure the value gets 
initialized with the current timestamp, not just be blank.  Do that, and 
I think this will make a nice incremental feature on top of the existing 
stats structure.


--
Greg Smith   2ndQuadrant USg...@2ndquadrant.com   Baltimore, MD
PostgreSQL Training, Services, and 24x7 Support  www.2ndQuadrant.us
PostgreSQL 9.0 High Performance: http://www.2ndQuadrant.com/books


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] keeping a timestamp of the last stats reset (for a db, table and function)

2011-01-30 Thread Robert Haas
On Thu, Dec 23, 2010 at 2:41 PM, Tomas Vondra t...@fuzzy.cz wrote:
 OK, so here goes the simplified patch - it tracks one reset timestamp
 for a background writer and for each database.

 I think you forgot the attachment.

 Yes, I did. Thanks!

This patch no longer applies.  Please update.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] keeping a timestamp of the last stats reset (for a db, table and function)

2011-01-04 Thread Greg Smith

t...@fuzzy.cz wrote:

- I really am not sure about the changes made in pg_proc.h. I'm not sure
how to assign OIDs to the new functions (I've simply chosen values that
are were not used in this file), and I'm not sure about the other columns
(I've copied and modified another function with the same parameter/return
types)
  


The description of the columns is at the beginning of pg_proc.h, the 
part that begins with CATALOG(pg_proc,1255)...  The descriptions of some 
of the first 11 fields are mostly straighforward.  The first fun part is 
that how may times the information expected in the second VARIABLE 
LENGTH FIELDS section repeats varies based on the parameters listed.  
The other thing that's usually confusing is that the types for the 
values are all expressed as type OID numbers.  For example, if you see 
25, that's the OID of the text type.  You can see the whole list with:


select oid,typname from pg_type;

And if you go back to the file with that list in handle, a lot more of 
it should make sense.  If you see a multiple parameter type list like 
23 21, that's a function whose input values are of types (int4,int2),


As for getting a new OID, if you go into src/include/catalog/ and run 
the unused_oids script, it will give you some help in figuring which 
have been used and which not.  It's not worth getting too stressed about 
the number you choose in the patch submission, because commits between 
when you got your free number and when your patch is considered for 
commit can make your choice worthless anyway.  There's a process 
referred to as catversion bump, where the database catalog version get 
updated to reflect things like new pg_proc information, that committers 
take care of as one of the last adjustments before final commit.  Doing 
a final correction to the OID choice is a part every committer knows to 
look at.


I wrote a talk that covered some additional trivia in this area, as well 
as other things people tend to get confused about in the source code, 
that you can find at 
http://www.pgcon.org/2010/schedule/attachments/142_HackingWithUDFs.pdf ; 
that might be helpful for some other things you might wonder about 
eventually.


--
Greg Smith   2ndQuadrant USg...@2ndquadrant.com   Baltimore, MD
PostgreSQL Training, Services and Supportwww.2ndQuadrant.us
PostgreSQL 9.0 High Performance: http://www.2ndQuadrant.com/books


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] keeping a timestamp of the last stats reset (for a db, table and function)

2011-01-04 Thread Greg Smith

Tomas Vondra wrote:

OK, so here goes the simplified patch - it tracks one reset timestamp
for a background writer and for each database.
  


Adding timestamps like this was something I wanted to do after adding 
pg_stat_reset_shared to 9.0, so since you've beaten me to it I'll review 
your patch and make sure it all works the way I was hoping instead.  The 
whole per-table idea was never going to fly given how few people touch 
this area at all, but the way you're tracking things now seems reasonable.


When you post an updated version of a patch that's already being tracked 
on the CommitFest app, please try to remember to add that update to the 
tracker there.  I just did that for this 12/23 update so that's covered 
already.


Next problem is that the preferred method for submitted patches uses 
context diffs.  See http://wiki.postgresql.org/wiki/Working_with_Git for 
some information about the somewhat annoying way you have to setup git 
to generate those.  Don't worry about that for this round though.  I 
don't care about the diff formatting given the code involved, but it's 
something you should sort out if you do another update.



PS: I've noticed Magnus posted a patch to track recovery conflicts,
adding a new view pg_stat_database_conflicts. I have not checked it yet
but it should not influence this patch.
  


I need to do some testing of that anyway, so I'll take a look at any 
potential clash as part of my review.  I want to check how this 
interacts with track_functions resets too.


--
Greg Smith   2ndQuadrant USg...@2ndquadrant.com   Baltimore, MD
PostgreSQL Training, Services and Supportwww.2ndQuadrant.us
PostgreSQL 9.0 High Performance: http://www.2ndQuadrant.com/books


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] keeping a timestamp of the last stats reset (for a db, table and function)

2010-12-23 Thread Tomas Vondra
Dne 20.12.2010 00:03, Tom Lane napsal(a):
 I wrote:
 That is not the number of interest.  The number of interest is that it's
 8 bytes added onto a struct that currently contains 11 of 'em; in other
 words a 9% increase in the size of the stats file, and consequently
 about a 9% increase in the cost of updating it.
 
 Wups, sorry, I was looking at the wrong struct.  It's actually an
 addition of 1 doubleword to a struct of 21 of 'em, or about 5%.
 That's still an awful lot in comparison to the prospective usefulness
 of the information.
 
   regards, tom lane
 

OK, so here goes the simplified patch - it tracks one reset timestamp
for a background writer and for each database.

regards
Tomas

PS: I've noticed Magnus posted a patch to track recovery conflicts,
adding a new view pg_stat_database_conflicts. I have not checked it yet
but it should not influence this patch.

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] keeping a timestamp of the last stats reset (for a db, table and function)

2010-12-23 Thread Robert Haas
2010/12/23 Tomas Vondra t...@fuzzy.cz:
 Dne 20.12.2010 00:03, Tom Lane napsal(a):
 I wrote:
 That is not the number of interest.  The number of interest is that it's
 8 bytes added onto a struct that currently contains 11 of 'em; in other
 words a 9% increase in the size of the stats file, and consequently
 about a 9% increase in the cost of updating it.

 Wups, sorry, I was looking at the wrong struct.  It's actually an
 addition of 1 doubleword to a struct of 21 of 'em, or about 5%.
 That's still an awful lot in comparison to the prospective usefulness
 of the information.

                       regards, tom lane


 OK, so here goes the simplified patch - it tracks one reset timestamp
 for a background writer and for each database.

I think you forgot the attachment.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] keeping a timestamp of the last stats reset (for a db, table and function)

2010-12-23 Thread Tomas Vondra
Dne 23.12.2010 20:09, Robert Haas napsal(a):
 2010/12/23 Tomas Vondra t...@fuzzy.cz:
 Dne 20.12.2010 00:03, Tom Lane napsal(a):
 I wrote:
 That is not the number of interest.  The number of interest is that it's
 8 bytes added onto a struct that currently contains 11 of 'em; in other
 words a 9% increase in the size of the stats file, and consequently
 about a 9% increase in the cost of updating it.

 Wups, sorry, I was looking at the wrong struct.  It's actually an
 addition of 1 doubleword to a struct of 21 of 'em, or about 5%.
 That's still an awful lot in comparison to the prospective usefulness
 of the information.

   regards, tom lane


 OK, so here goes the simplified patch - it tracks one reset timestamp
 for a background writer and for each database.
 
 I think you forgot the attachment.

Yes, I did. Thanks!

Tomas

From da2154954a547c143cd0d130597411911f339c07 Mon Sep 17 00:00:00 2001
From: vampire vamp...@rimmer.reddwarf
Date: Thu, 23 Dec 2010 18:40:48 +0100
Subject: [PATCH] Track timestamp of the last stats reset (for each database and 
a background writer).

This is simplified version of the previous patch, that kept a timestamp for each
database, table, index and function and for the background writer. This does not
keep the timestamp for individual objects, but if stats for a 
table/index/function
are reset, the timestamp for the whole database is updated.
---
 doc/src/sgml/monitoring.sgml |   18 ++
 src/backend/catalog/system_views.sql |6 --
 src/backend/postmaster/pgstat.c  |   13 +
 src/backend/utils/adt/pgstatfuncs.c  |   26 ++
 src/include/catalog/pg_proc.h|4 
 src/include/pgstat.h |5 +
 6 files changed, 70 insertions(+), 2 deletions(-)

diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml
index 5fd0213..99e091d 100644
--- a/doc/src/sgml/monitoring.sgml
+++ b/doc/src/sgml/monitoring.sgml
@@ -600,6 +600,15 @@ postgres: replaceableuser/ replaceabledatabase/ 
replaceablehost/ re
  /row
 
  row
+  
entryliteralfunctionpg_stat_get_db_stat_reset_time/function(typeoid/type)/literal/entry
+  entrytypetimestamptz/type/entry
+  entry
+   Time of the last reset of statistics for this database (as a result of 
executing
+   functionpg_stat_reset/function function) or any object within it 
(table or index).
+  /entry
+ /row
+
+ row
   
entryliteralfunctionpg_stat_get_numscans/function(typeoid/type)/literal/entry
   entrytypebigint/type/entry
   entry
@@ -1062,6 +1071,15 @@ postgres: replaceableuser/ replaceabledatabase/ 
replaceablehost/ re
varnamebgwriter_lru_maxpages/varname parameter
   /entry
  /row
+ 
+ row
+  
entryliteralfunctionpg_stat_get_bgwriter_stat_reset_time()/function/literal/entry
+  entrytypetimestamptz/type/entry
+  entry
+Time of the last reset of statistics for the background writer (as a 
result of executing
+functionpg_stat_reset_shared('bgwriter')/function)
+  /entry
+ /row
 
  row
   
entryliteralfunctionpg_stat_get_buf_written_backend()/function/literal/entry
diff --git a/src/backend/catalog/system_views.sql 
b/src/backend/catalog/system_views.sql
index 346eaaf..da92438 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -502,7 +502,8 @@ CREATE VIEW pg_stat_database AS
 pg_stat_get_db_tuples_fetched(D.oid) AS tup_fetched,
 pg_stat_get_db_tuples_inserted(D.oid) AS tup_inserted,
 pg_stat_get_db_tuples_updated(D.oid) AS tup_updated,
-pg_stat_get_db_tuples_deleted(D.oid) AS tup_deleted
+pg_stat_get_db_tuples_deleted(D.oid) AS tup_deleted,
+   pg_stat_get_db_stat_reset_time(D.oid) AS stats_reset
 FROM pg_database D;
 
 CREATE VIEW pg_stat_user_functions AS
@@ -538,7 +539,8 @@ CREATE VIEW pg_stat_bgwriter AS
 pg_stat_get_bgwriter_maxwritten_clean() AS maxwritten_clean,
 pg_stat_get_buf_written_backend() AS buffers_backend,
 pg_stat_get_buf_fsync_backend() AS buffers_backend_fsync,
-pg_stat_get_buf_alloc() AS buffers_alloc;
+pg_stat_get_buf_alloc() AS buffers_alloc,
+   pg_stat_get_bgwriter_stat_reset_time() AS stats_reset;
 
 CREATE VIEW pg_user_mappings AS
 SELECT
diff --git a/src/backend/postmaster/pgstat.c b/src/backend/postmaster/pgstat.c
index 856daa7..66e0b69 100644
--- a/src/backend/postmaster/pgstat.c
+++ b/src/backend/postmaster/pgstat.c
@@ -3130,6 +3130,8 @@ pgstat_get_db_entry(Oid databaseid, bool create)
result-n_tuples_deleted = 0;
result-last_autovac_time = 0;
 
+   result-stat_reset_timestamp = GetCurrentTimestamp();
+
memset(hash_ctl, 0, sizeof(hash_ctl));
hash_ctl.keysize = sizeof(Oid);
hash_ctl.entrysize = sizeof(PgStat_StatTabEntry);
@@ -3408,6 

Re: [HACKERS] keeping a timestamp of the last stats reset (for a db, table and function)

2010-12-19 Thread tv
 Tomas Vondra t...@fuzzy.cz writes:
 I've done several small changes to the patch, namely

 - added docs for the functions (in SGML)
 - added the same thing for background writer

 So I think now it's 'complete' and I'll add it to the commit fest in a
 few minutes.

 Please split this into separate patches for database-level and
 table-level tracking, because I think it's highly likely that the latter
 will get rejected.  We have had multiple complaints already about the
 size of the stats file for databases with many tables.  I don't believe
 that it's worth bloating the per-table entries even further with this
 information.  Tracking reset time it per-database doesn't seem like a
 problem though.

Sorry, it's not possible to split this patch into two, and then choose
just one or both pieces. I could split it, but using just the
'per-database' part would not make sense.

The problems is that right now, when stat's are reset for a single table
or function, the timestamp is set just for that one item, not for the
whole database. So just a blind split of the patch (and using just the
per-database part) would mean the info about tables/functions is lost.

I can prepare an alternative patch, using just per-database timestamps. So
even a reset for a single table/function would set the timestamp for the
whole database.

Tomas


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] keeping a timestamp of the last stats reset (for a db, table and function)

2010-12-19 Thread Tom Lane
t...@fuzzy.cz writes:
 I can prepare an alternative patch, using just per-database timestamps. So
 even a reset for a single table/function would set the timestamp for the
 whole database.

That seems like quite a bizarre definition.  What I was envisioning was
that we'd track only the time of the last whole-database stats reset.

regards, tom lane

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] keeping a timestamp of the last stats reset (for a db, table and function)

2010-12-19 Thread Jim Nasby
On Dec 18, 2010, at 8:51 PM, Tom Lane wrote:
 Tomas Vondra t...@fuzzy.cz writes:
 I've done several small changes to the patch, namely
 
 - added docs for the functions (in SGML)
 - added the same thing for background writer
 
 So I think now it's 'complete' and I'll add it to the commit fest in a
 few minutes.
 
 Please split this into separate patches for database-level and
 table-level tracking, because I think it's highly likely that the latter
 will get rejected.  We have had multiple complaints already about the
 size of the stats file for databases with many tables.  I don't believe
 that it's worth bloating the per-table entries even further with this
 information.  Tracking reset time it per-database doesn't seem like a
 problem though.

Is there a reason this info needs to be tracked in the stats table? I know it's 
the most obvious place, but since we're worried about the size of it, what 
about tracking it in pg_class or somewhere else?
--
Jim C. Nasby, Database Architect   j...@nasby.net
512.569.9461 (cell) http://jim.nasby.net



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] keeping a timestamp of the last stats reset (for a db, table and function)

2010-12-19 Thread Tomas Vondra
Dne 19.12.2010 17:26, Tom Lane napsal(a):
 That seems like quite a bizarre definition.  What I was envisioning was
 that we'd track only the time of the last whole-database stats reset.

Well, but that does not quite work. I need is to know whether the
snapshot is 'consistent' i.e. whether I can compare it to the previous
snapshot and get meaningful results (so that I can perform some analysis
on the difference).

And by 'consistent' I mean that none of the counters was reset since the
previous snapshot. The most obvious and most flexible way to do this is
keeping track of the last reset for each of the counters, which is
exactly what I've done in the patch.

The other possibility I've offered in my previous post is to keep just a
per-database timestamp, and set it whenever some stats in the database
are reset (table/index/function counters or all stats). It definitely is
not as flexible as the previous solution, but it gives me at least some
info that something was reset. But I'd have to throw away the whole
snapshot - in the previous case I could do analysis at least on the
counters that were not reset.

The solution you've offered - keeping only the per-database timestamp,
and not updating it when a table/index/function stats are reset, that's
completely useless for me. It simply does not provide an answer to the
question Is this snapshot consistent?

Tomas

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] keeping a timestamp of the last stats reset (for a db, table and function)

2010-12-19 Thread Tomas Vondra
Dne 19.12.2010 19:13, Jim Nasby napsal(a):
 Is there a reason this info needs to be tracked in the stats table?
 I know it's the most obvious place, but since we're worried about the
 size of it, what about tracking it in pg_class or somewhere else?

I guess this is the best place for this kind of info, as it is directly
related to the stats items. Well, maybe we could place it to pg_class,
but is it a wise idea?

Tomas

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] keeping a timestamp of the last stats reset (for a db, table and function)

2010-12-19 Thread Tom Lane
Tomas Vondra t...@fuzzy.cz writes:
 Dne 19.12.2010 17:26, Tom Lane napsal(a):
 That seems like quite a bizarre definition.  What I was envisioning was
 that we'd track only the time of the last whole-database stats reset.

 Well, but that does not quite work. I need is to know whether the
 snapshot is 'consistent' i.e. whether I can compare it to the previous
 snapshot and get meaningful results (so that I can perform some analysis
 on the difference).

Oh, I see.  Yeah, if that's what you want it for then partial resets
have to change the timestamp too.  I guess if we are careful to document
it properly, this won't be too horrid.

regards, tom lane

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] keeping a timestamp of the last stats reset (for a db, table and function)

2010-12-19 Thread Tomas Vondra
Dne 19.12.2010 20:28, Tom Lane napsal(a):
 Tomas Vondra t...@fuzzy.cz writes:
 Dne 19.12.2010 17:26, Tom Lane napsal(a):
 That seems like quite a bizarre definition.  What I was envisioning was
 that we'd track only the time of the last whole-database stats reset.
 
 Well, but that does not quite work. I need is to know whether the
 snapshot is 'consistent' i.e. whether I can compare it to the previous
 snapshot and get meaningful results (so that I can perform some analysis
 on the difference).
 
 Oh, I see.  Yeah, if that's what you want it for then partial resets
 have to change the timestamp too.  I guess if we are careful to document
 it properly, this won't be too horrid.
 
   regards, tom lane
 

Well, maybe. I'd prefer the timestamp for each item, as that allows me
to determine which stats were not reset and analyze at least those data.

Plus I won't have time to write the other patch for at least a week, so
let's see whether there are serious objections agains the current patch.

I've never had problems with the pgstat.dat file, but I understand
others might, although this adds just 8 bytes to each item.

regards
Tomas

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] keeping a timestamp of the last stats reset (for a db, table and function)

2010-12-19 Thread Tom Lane
Tomas Vondra t...@fuzzy.cz writes:
 Plus I won't have time to write the other patch for at least a week, so
 let's see whether there are serious objections agains the current patch.

If you think this objection is not serious, you're mistaken.

 I've never had problems with the pgstat.dat file, but I understand
 others might, although this adds just 8 bytes to each item.

That is not the number of interest.  The number of interest is that it's
8 bytes added onto a struct that currently contains 11 of 'em; in other
words a 9% increase in the size of the stats file, and consequently
about a 9% increase in the cost of updating it.

What is bothering me about that is this: how many of our users ever
reset the stats at all?  Of those, how many reset the stats for just
some tables and not all of them?  And of those, how many care about
having the database remember when they did it, at a per-table level?
I think the answer to each of those questions is not many, and so
the fraction of our users who will get a benefit from that added
overhead is epsilon cubed.  But approximately 100% of our users will
be paying the overhead.

That just doesn't look like a good tradeoff from here.

regards, tom lane

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] keeping a timestamp of the last stats reset (for a db, table and function)

2010-12-19 Thread Tom Lane
I wrote:
 That is not the number of interest.  The number of interest is that it's
 8 bytes added onto a struct that currently contains 11 of 'em; in other
 words a 9% increase in the size of the stats file, and consequently
 about a 9% increase in the cost of updating it.

Wups, sorry, I was looking at the wrong struct.  It's actually an
addition of 1 doubleword to a struct of 21 of 'em, or about 5%.
That's still an awful lot in comparison to the prospective usefulness
of the information.

regards, tom lane

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] keeping a timestamp of the last stats reset (for a db, table and function)

2010-12-19 Thread Tomas Vondra
Dne 19.12.2010 23:58, Tom Lane napsal(a):
 Tomas Vondra t...@fuzzy.cz writes:
  Plus I won't have time to write the other patch for at least a week, so
  let's see whether there are serious objections agains the current patch.
 If you think this objection is not serious, you're mistaken.

I know there were problems with pgstats.stat and I/O (for example this
thread discussing an I/O storm
http://archives.postgresql.org/pgsql-hackers/2008-01/msg01095.php).

But I thought those issues were already resolved and I have not noticed
any such issue recently. Am I missing something?

 What is bothering me about that is this: how many of our users ever
 reset the stats at all?  Of those, how many reset the stats for just
 some tables and not all of them?  And of those, how many care about
 having the database remember when they did it, at a per-table level?
 I think the answer to each of those questions is not many, and so
 the fraction of our users who will get a benefit from that added
 overhead is epsilon cubed.  But approximately 100% of our users will
 be paying the overhead.

Sure, I understand that only a fraction of the users will use the patch
I've proposed. That's why I'm saying that the per-database timestamp
would be sufficient (although I'd prefer the per-record timestamp).

regards
Tomas

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] keeping a timestamp of the last stats reset (for a db, table and function)

2010-12-18 Thread Tomas Vondra
Dne 12.12.2010 03:47, Robert Haas napsal(a):
 On Sat, Dec 11, 2010 at 4:40 PM,  t...@fuzzy.cz wrote:
 Hello

 you have to respect pg coding style:

 a) not too long lines
 b) not C++ line comments

 OK, thanks for the notice. I've fixed those two problems.
 
 Please add this to the currently-open commitfest.
 
 https://commitfest.postgresql.org/action/commitfest_view/open

I've done several small changes to the patch, namely

- added docs for the functions (in SGML)
- added the same thing for background writer

So I think now it's 'complete' and I'll add it to the commit fest in a
few minutes.

regards
Tomas
diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml
index 5fd0213..40c6c6b 100644
--- a/doc/src/sgml/monitoring.sgml
+++ b/doc/src/sgml/monitoring.sgml
@@ -600,6 +600,15 @@ postgres: replaceableuser/ replaceabledatabase/ 
replaceablehost/ re
  /row
 
  row
+  
entryliteralfunctionpg_stat_get_db_last_stat_reset_time/function(typeoid/type)/literal/entry
+  entrytypetimestamptz/type/entry
+  entry
+   Time of the last reset of statistics for this database (as a result of 
executing
+  functionpg_stat_reset/function function)
+  /entry
+ /row
+
+ row
   
entryliteralfunctionpg_stat_get_numscans/function(typeoid/type)/literal/entry
   entrytypebigint/type/entry
   entry
@@ -725,6 +734,15 @@ postgres: replaceableuser/ replaceabledatabase/ 
replaceablehost/ re
  /row
 
  row
+  
entryliteralfunctionpg_stat_get_last_stat_reset_time/function(typeoid/type)/literal/entry
+  entrytypetimestamptz/type/entry
+  entry
+   Time of the last reset of statistics on this table (as a result of 
executing
+  functionpg_stat_reset/function or 
functionpg_stat_reset_single_table_counters/function)
+  /entry
+ /row
+
+ row
   
entryliteralfunctionpg_stat_get_vacuum_count/function(typeoid/type)/literal/entry
   entrytypebigint/type/entry
   entry
@@ -879,6 +897,15 @@ postgres: replaceableuser/ replaceabledatabase/ 
replaceablehost/ re
  /row
 
  row
+  
entryliteralfunctionpg_stat_get_function_last_stat_reset_time/function(typeoid/type)/literal/entry
+  entrytypetimestamptz/type/entry
+  entry
+   Time of the last reset of statistics for this function (as a result of 
executing
+  functionpg_stat_reset/function or 
functionpg_stat_reset_single_function_counters/function)
+  /entry
+ /row
+
+ row
   
entryliteralfunctionpg_stat_get_xact_function_calls/function(typeoid/type)/literal/entry
   entrytypebigint/type/entry
   entry
@@ -1064,6 +1091,15 @@ postgres: replaceableuser/ replaceabledatabase/ 
replaceablehost/ re
  /row
 
  row
+  
entryliteralfunctionpg_stat_get_bgwriter_last_stat_reset_time()/function/literal/entry
+  entrytypetimestamptz/type/entry
+  entry
+   Time of the last reset of statistics for the background writer (as a 
result of executing
+  functionpg_stat_reset_shared('bgwriter')/function)
+  /entry
+ /row
+
+ row
   
entryliteralfunctionpg_stat_get_buf_written_backend()/function/literal/entry
   entrytypebigint/type/entry
   entry
diff --git a/src/backend/catalog/system_views.sql 
b/src/backend/catalog/system_views.sql
index 346eaaf..3e1cca3 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -310,6 +310,7 @@ CREATE VIEW pg_stat_all_tables AS
 pg_stat_get_last_autovacuum_time(C.oid) as last_autovacuum,
 pg_stat_get_last_analyze_time(C.oid) as last_analyze,
 pg_stat_get_last_autoanalyze_time(C.oid) as last_autoanalyze,
+pg_stat_get_last_stat_reset_time(C.oid) as last_stat_reset,
 pg_stat_get_vacuum_count(C.oid) AS vacuum_count,
 pg_stat_get_autovacuum_count(C.oid) AS autovacuum_count,
 pg_stat_get_analyze_count(C.oid) AS analyze_count,
@@ -502,7 +503,8 @@ CREATE VIEW pg_stat_database AS
 pg_stat_get_db_tuples_fetched(D.oid) AS tup_fetched,
 pg_stat_get_db_tuples_inserted(D.oid) AS tup_inserted,
 pg_stat_get_db_tuples_updated(D.oid) AS tup_updated,
-pg_stat_get_db_tuples_deleted(D.oid) AS tup_deleted
+pg_stat_get_db_tuples_deleted(D.oid) AS tup_deleted,
+pg_stat_get_db_last_stat_reset_time(D.oid) AS last_stat_reset
 FROM pg_database D;
 
 CREATE VIEW pg_stat_user_functions AS
@@ -512,7 +514,8 @@ CREATE VIEW pg_stat_user_functions AS
 P.proname AS funcname,
 pg_stat_get_function_calls(P.oid) AS calls,
 pg_stat_get_function_time(P.oid) / 1000 AS total_time,
-pg_stat_get_function_self_time(P.oid) / 1000 AS self_time
+pg_stat_get_function_self_time(P.oid) / 1000 AS self_time,
+pg_stat_get_function_last_stat_reset_time(P.oid) AS last_stat_reset
 FROM pg_proc P LEFT JOIN pg_namespace N ON (N.oid = 

Re: [HACKERS] keeping a timestamp of the last stats reset (for a db, table and function)

2010-12-18 Thread Tom Lane
Tomas Vondra t...@fuzzy.cz writes:
 I've done several small changes to the patch, namely

 - added docs for the functions (in SGML)
 - added the same thing for background writer

 So I think now it's 'complete' and I'll add it to the commit fest in a
 few minutes.

Please split this into separate patches for database-level and
table-level tracking, because I think it's highly likely that the latter
will get rejected.  We have had multiple complaints already about the
size of the stats file for databases with many tables.  I don't believe
that it's worth bloating the per-table entries even further with this
information.  Tracking reset time it per-database doesn't seem like a
problem though.

regards, tom lane

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] keeping a timestamp of the last stats reset (for a db, table and function)

2010-12-11 Thread Pavel Stehule
Hello

you have to respect pg coding style:

a) not too long lines
b) not C++ line comments

Zdar

Pavel

2010/12/11  t...@fuzzy.cz:
 Hi everyone,

 I just wrote my first patch, and I need to know whether I missed something
 or not. I haven't used C for a really long time, so sickbags on standby,
 and if you notice something really stupid don't hesitate to call me an
 asshole (according to Simon Phipps that proves we are a healthy open
 community).

 So what the patch does (or should do)? It tracks when were the stats for a
 given object (database, table or function) reset for the last time. This
 is useful when you do snapshots of the stats for analysis - when comparing
 two snapshots, you have to know whether the stats were reset (in that case
 the analysis usually yields random noise and automatic tools get confused
 by this).

 Tom Lane already recommended a workaround - firing the DBA who randomly
 resets statistics, but that's not a good solution I think. First, you have
 to be superior to the DBA to be able to fire him ;-) Second, everyone
 makes a mistake from time to time. Third, when there are functions to
 reset stats, it's nice to provide such info as it makes life much easier.

 And there are cases when you don't reset the stats explicitly but the data
 are actually gone - e.g. when after a restore or upgrade (OK, this is
 solvable using pg_postmaster_start_time).

 In short, I think it's a useful feature (I need it and I think there are
 others). And I think it's not disruptive.

 So what the patch actually does:

 - extends PgStat_StatDBEntry, PgStat_StatTableEntry and
 PgStat_StatFuncEntry with a new field (stat_reset_timestamp)

 - adds functions to read current value from these fields
 (pg_stat_get_db_last_stat_reset_time, pg_stat_get_last_stat_reset_time and
 pg_stat_get_function_last_stat_reset_time)

 - extends the system views with calls to these functions
 (pg_stat_database, pg_stat_user_functions and pg_stat_all_tables)

 The values are set like this:

 - when a database is created, current timestamp is stored in
 PgStat_StatDBEntry.stat_reset_timestamp
 - by default all tables/functions inherit this timestamp
 - when stats for a given table / function are reset, current timestamp is
 stored in the stat_reset_timestamp (this happens in
 pgstat_recv_resetsinglecounter)
 - when stats for the whole database are reset, everything starts from
 scratch (this happens in pgstat_recv_resetcounter)

 What I'm not sure about:

 - I really am not sure about the changes made in pg_proc.h. I'm not sure
 how to assign OIDs to the new functions (I've simply chosen values that
 are were not used in this file), and I'm not sure about the other columns
 (I've copied and modified another function with the same parameter/return
 types)

 - I'm not sure if there are any other ways how the stat entries can be
 created. I've found two ways - directly (when asked for the stats e.g.
 from pgstat_get_tab_entry), and indirectly (when processing stats from a
 backend e.g. in pgstat_recv_tabstat).

 regards
 Tomas

 --
 Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
 To make changes to your subscription:
 http://www.postgresql.org/mailpref/pgsql-hackers



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] keeping a timestamp of the last stats reset (for a db, table and function)

2010-12-11 Thread tv
 Hello

 you have to respect pg coding style:

 a) not too long lines
 b) not C++ line comments

OK, thanks for the notice. I've fixed those two problems.

regards
Tomasdiff --git a/src/backend/catalog/system_views.sql 
b/src/backend/catalog/system_views.sql
index 346eaaf..0ee59b1 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -310,6 +310,7 @@ CREATE VIEW pg_stat_all_tables AS
 pg_stat_get_last_autovacuum_time(C.oid) as last_autovacuum,
 pg_stat_get_last_analyze_time(C.oid) as last_analyze,
 pg_stat_get_last_autoanalyze_time(C.oid) as last_autoanalyze,
+   pg_stat_get_last_stat_reset_time(C.oid) as last_stat_reset,
 pg_stat_get_vacuum_count(C.oid) AS vacuum_count,
 pg_stat_get_autovacuum_count(C.oid) AS autovacuum_count,
 pg_stat_get_analyze_count(C.oid) AS analyze_count,
@@ -502,7 +503,8 @@ CREATE VIEW pg_stat_database AS
 pg_stat_get_db_tuples_fetched(D.oid) AS tup_fetched,
 pg_stat_get_db_tuples_inserted(D.oid) AS tup_inserted,
 pg_stat_get_db_tuples_updated(D.oid) AS tup_updated,
-pg_stat_get_db_tuples_deleted(D.oid) AS tup_deleted
+pg_stat_get_db_tuples_deleted(D.oid) AS tup_deleted,
+pg_stat_get_db_last_stat_reset_time(D.oid) AS last_stat_reset
 FROM pg_database D;
 
 CREATE VIEW pg_stat_user_functions AS
@@ -512,7 +514,8 @@ CREATE VIEW pg_stat_user_functions AS
 P.proname AS funcname,
 pg_stat_get_function_calls(P.oid) AS calls,
 pg_stat_get_function_time(P.oid) / 1000 AS total_time,
-pg_stat_get_function_self_time(P.oid) / 1000 AS self_time
+pg_stat_get_function_self_time(P.oid) / 1000 AS self_time,
+pg_stat_get_function_last_stat_reset_time(P.oid) AS last_stat_reset
 FROM pg_proc P LEFT JOIN pg_namespace N ON (N.oid = P.pronamespace)
 WHERE P.prolang != 12  -- fast check to eliminate built-in functions
   AND pg_stat_get_function_calls(P.oid) IS NOT NULL;
diff --git a/src/backend/postmaster/pgstat.c b/src/backend/postmaster/pgstat.c
index c3c136a..f20a9ea 100644
--- a/src/backend/postmaster/pgstat.c
+++ b/src/backend/postmaster/pgstat.c
@@ -249,6 +249,8 @@ static void pgstat_sighup_handler(SIGNAL_ARGS);
 static PgStat_StatDBEntry *pgstat_get_db_entry(Oid databaseid, bool create);
 static PgStat_StatTabEntry *pgstat_get_tab_entry(PgStat_StatDBEntry *dbentry,
 Oid tableoid, bool create);
+static PgStat_StatFuncEntry *pgstat_get_func_entry(PgStat_StatDBEntry *dbentry,
+Oid funcoid, bool create);
 static void pgstat_write_statsfile(bool permanent);
 static HTAB *pgstat_read_statsfile(Oid onlydb, bool permanent);
 static void backend_read_statsfile(void);
@@ -3129,7 +3131,9 @@ pgstat_get_db_entry(Oid databaseid, bool create)
result-n_tuples_updated = 0;
result-n_tuples_deleted = 0;
result-last_autovac_time = 0;
-
+   
+   result-stat_reset_timestamp = GetCurrentTimestamp();
+   
memset(hash_ctl, 0, sizeof(hash_ctl));
hash_ctl.keysize = sizeof(Oid);
hash_ctl.entrysize = sizeof(PgStat_StatTabEntry);
@@ -3196,11 +3200,48 @@ pgstat_get_tab_entry(PgStat_StatDBEntry *dbentry, Oid 
tableoid, bool create)
result-autovac_vacuum_count = 0;
result-analyze_count = 0;
result-autovac_analyze_count = 0;
+   
+   /* inherit stat time reset from dbentry */
+   result-stat_reset_timestamp = dbentry-stat_reset_timestamp;
+   
}
 
return result;
 }
 
+/*
+ * Lookup the hash table entry for the specified function. If no hash
+ * table entry exists, initialize it, if the create parameter is true.
+ * Else, return NULL.
+ */
+static PgStat_StatFuncEntry *
+pgstat_get_func_entry(PgStat_StatDBEntry *dbentry, Oid tableoid, bool create)
+{
+   PgStat_StatFuncEntry *result;
+   boolfound;
+   HASHACTION  action = (create ? HASH_ENTER : HASH_FIND);
+
+   /* Lookup or create the hash table entry for this table */
+   result = (PgStat_StatFuncEntry *) hash_search(dbentry-functions,
+   
 tableoid,
+   
 action, found);
+
+   if (!create  !found)
+   return NULL;
+
+   /* If not found, initialize the new one. */
+   if (!found)
+   {
+   result-f_numcalls = 0;
+   result-f_time = 0;
+   result-f_time_self = 0;
+   
+   /* inherit stat time reset from dbentry */
+   result-stat_reset_timestamp = 

Re: [HACKERS] keeping a timestamp of the last stats reset (for a db, table and function)

2010-12-11 Thread Robert Haas
On Sat, Dec 11, 2010 at 4:40 PM,  t...@fuzzy.cz wrote:
 Hello

 you have to respect pg coding style:

 a) not too long lines
 b) not C++ line comments

 OK, thanks for the notice. I've fixed those two problems.

Please add this to the currently-open commitfest.

https://commitfest.postgresql.org/action/commitfest_view/open

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers