Re: [HACKERS] Clearing global statistics
On Mon, Jan 18, 2010 at 11:02, Magnus Hagander mag...@hagander.net wrote: On Mon, Jan 18, 2010 at 00:52, Greg Smith g...@2ndquadrant.com wrote: Magnus Hagander wrote: Maybe this should be Unrecognized reset target: %s, target, and also a errhint() saying which targets are allowed. Thoughts? That seems reasonable. The other thing I realized is that I forgot to add the new function to the right place in doc/src/sgml/func.sgml : indexterm primarypg_stat_reset_shared/primary /indexterm I can send an updated patch with both of these things fixed tomorrow. Given that we're talking 5 lines of change here, if it's easier for you to just patch a working copy you've already started on that's fine too. I can deal with that, I just wanted to make sure we're in agreement. Applied with agreed upon change and some minor styling/formatting changes. The mention you have about documentation above is incorrect - the other pg_stat_xyz functions aren't listed there, so neither should this one. At least from AFAICT ;) -- 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] Clearing global statistics
On Mon, Jan 18, 2010 at 00:52, Greg Smith g...@2ndquadrant.com wrote: Magnus Hagander wrote: Maybe this should be Unrecognized reset target: %s, target, and also a errhint() saying which targets are allowed. Thoughts? That seems reasonable. The other thing I realized is that I forgot to add the new function to the right place in doc/src/sgml/func.sgml : indexterm primarypg_stat_reset_shared/primary /indexterm I can send an updated patch with both of these things fixed tomorrow. Given that we're talking 5 lines of change here, if it's easier for you to just patch a working copy you've already started on that's fine too. I can deal with that, I just wanted to make sure we're in agreement. -- 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] Clearing global statistics
2010/1/14 Greg Smith g...@2ndquadrant.com:
Itagaki Takahiro wrote:
To be honest, I have a plan to add performance statistics counters to
postgres. It is not bgwriter's counters, but cluster-level. I'd like
to use your infrastructure in my work, too :)
Attached patch provides just that. It still works basically the same as my
earlier version, except you pass it a name of what you want to reset, and if
you don't give it the only valid one right now ('bgwriter') it rejects it
(for now):
gsmith=# select checkpoints_req,buffers_alloc from pg_stat_bgwriter;
checkpoints_req | buffers_alloc
-+---
4 | 129
(1 row)
gsmith=# select pg_stat_reset_shared('bgwriter');
pg_stat_reset_shared
--
(1 row)
gsmith=# select checkpoints_req,buffers_alloc from pg_stat_bgwriter;
checkpoints_req | buffers_alloc
-+---
0 | 7
(1 row)
gsmith=# select pg_stat_reset_shared('rubbish');
ERROR: Unrecognized reset target
I turn the input text into an enum choice as part of composing the message to
the stats collector. If you wanted to add some other shared cluster-wide
reset capabilities into there you could re-use most of this infrastructure.
Just add an extra enum value, map the text into that enum, and write the
actual handler that does the reset work. Should be able to reuse the same new
message type and external UI I implemented for this specific clearing feature.
Yeah, that seems fine.
I didn't see any interaction to be concerned about here with Magnus's
suggestion he wanted to target stats reset on objects such as a single table
at some point.
Agreed.
The main coding choice I wasn't really sure about is how I flag the error
case where you pass bad in. I do that validation and throw
ERRCODE_SYNTAX_ERROR before composing the message to the stats collector.
Didn't know if I should create a whole new error code just for this specific
case or if reusing another error code was more appropriate. Also, I didn't
actually have the collector process itself validate the data at all, it just
quietly ignores bad messages on the presumption everything is already being
checked during message creation. That seems consistent with the other code
here--the other message handlers only seem to throw errors when something
really terrible happens, not when they just don't find something useful to do.
Creating a whole new error code seems completely wrong.
ERRCODE_SYNTAX_ERROR seems fine to me. As does the not checking the
other options.
I looked the patch over, and I only really see one thing to comment on which is:
+ if (strcmp(target, bgwriter) == 0)
+ msg.m_resettarget = RESET_BGWRITER;
+ else
+ {
+ ereport(ERROR,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+errmsg(Unrecognized reset target)));
+ }
Maybe this should be Unrecognized reset target: %s, target, and also
a errhint() saying which targets are allowed. Thoughts?
As for the discussion about if this is useful or not, I definitely
think it is. Yes, there are certainly general improvements that could
be done to the collection mechanism to make some things easier, but
that doesn't mean we shouldn't make the current solution more useful
until we (potentially) have it.
--
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] Clearing global statistics
Magnus Hagander wrote: Maybe this should be Unrecognized reset target: %s, target, and also a errhint() saying which targets are allowed. Thoughts? That seems reasonable. The other thing I realized is that I forgot to add the new function to the right place in doc/src/sgml/func.sgml : indexterm primarypg_stat_reset_shared/primary /indexterm I can send an updated patch with both of these things fixed tomorrow. Given that we're talking 5 lines of change here, if it's easier for you to just patch a working copy you've already started on that's fine too. -- Greg Smith2ndQuadrant Baltimore, MD PostgreSQL Training, Services and Support g...@2ndquadrant.com www.2ndQuadrant.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] Clearing global statistics
On Thu, Jan 14, 2010 at 1:11 PM, Tom Lane t...@sss.pgh.pa.us wrote: Greg Smith g...@2ndquadrant.com writes: Tom Lane wrote: Actually, that brings up a more general question: what's with the enthusiasm for clearing statistics *at all*? ... Right now, you're still carrying around the history of the bad period forever though, and every check of the pg_stat_bgwriter requires manually subtracting the earlier values out. Seems like a more appropriate solution would be to make it easier to do that subtraction, ie, make it easier to capture the values at a given time point and then get deltas from there. It's more general (you could have multiple saved sets of values), and doesn't require superuser permissions to do, and doesn't have the same potential for damn-I-wish-I-hadn't-done-that moments. True, but it's also more complicated to use. Most systems I'm familiar with[1] that have performance counters just provide an option to clear them. Despite the disadvantages you cite, it seems to be fairly useful in practice; anyway, I have found it so. ...Robert [1] The other design I've seen is a system that automatically resets, say, once a day. It retains the statistics for the 24-hour period between the most recent two resets, and the statistics for the partial period following the last reset. But that doesn't seem appropriate for PostgreSQL -- 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] Clearing global statistics
2010/1/12 Greg Smith g...@2ndquadrant.com:
Magnus Hagander wrote:
I have on my TODO to implement the ability to do stats reset on a
single object (say, one table only). Please take this into
consideration when you design/name this, so theres no unnecessary
overlap :-) Same goes for the stats message itself.
The idea suggested upthread was to add this:
pg_stat_reset( which text )
which := 'buffers' | 'checkpoints' | 'tables' | 'functions' | ...
Now, the way the pg_stat_bgwriter tables are computed, it doesn't actually
make sense to separate out clearing the buffers/checkpoints stats, since one
of those values is in both categories: buffers_checkpoint. They're really
all too tightly coupled to break them apart. So I was thinking of this:
pg_stat_reset( which text )
which := 'bgwriter' | ...
I could convert the patch I've got to be an initial implementation of this
new pg_stat_reset with a parameter, laying some useful groundwork in the
process too. Then people who want to reset more things can just re-use that
same outline and message passing mechanism, just adding comparisons for new
text and a handler to go with it--not even touching the catalog again.
This may not mesh well with what you plan though. If pg_stat_reset is
updated to reset stats on an individual table, that could be a second version
that takes in a regclass:
pg_stat_reset('tablename'::regclass)
But that seems like a confusing bit of overloading--I can easily see people
thinking that pg_stat_reset('bgwriter') would be resetting the stats for a
relation named 'bgwriter' rather than what it actually does if I build it
that way.
So, combining with Peter's naming suggestion, I think what I should build is:
pg_stat_reset_shared( which text )
which := 'bgwriter' | ...
Which satisfies what I'm looking for now, and future patches that need to
reset other shared across the cluster statistics can re-use this without
needing to add a whole new function/stats message. I think that satisfies
the cross-section of planned use cases we're expecting now best.
Any comments before I update my patch to do that?
Are you planning to get this in for the CF? (Yes, I realize there are
only hours left). This is functionality I'd *really* like to see in
8.5, so I'll be happy to work with you to get that committed inside or
outside CF bounds, but it's easier if it's on there for reviewers ;)
(plus, the outside cf bounds really only works *before* the
commitfest :P)
--
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] Clearing global statistics
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Greg Smith wrote: Magnus Hagander wrote: I have on my TODO to implement the ability to do stats reset on a single object (say, one table only). Please take this into consideration when you design/name this, so theres no unnecessary overlap :-) Same goes for the stats message itself. [.] Any comments before I update my patch to do that? Hello One thing I miss from the statistics you can get via pg_stat_* is information about how long we have been collecting stats (or in other words, when was the last time the stats were reset) Statistics without time period information are unfortunately not very usefull for a DBA :-( Before 8.3, we had the stats_reset_on_server_start parameter and the pg_postmaster_start_time() function. This was an easy way of resetting *all* statistics delivered by pg_stat_* and knowing when this was done. We were able to produce stats with information about sec/hours/days average values in an easy way. I tried to discuss this some time ago but we did not get anywhere, Ref: http://archives.postgresql.org/pgsql-general/2009-07/msg00614.php Maybe this time? :-) Is there any chance of implementing a way of knowing when was the last time statistics delivered via pg_stat_* were reset? regards, - -- Rafael Martinez, r.m.guerr...@usit.uio.no Center for Information Technology Services University of Oslo, Norway PGP Public Key: http://folk.uio.no/rafael/ -BEGIN PGP SIGNATURE- Version: GnuPG v1.2.7 (GNU/Linux) iD8DBQFLTx9cBhuKQurGihQRAnTZAJ9afYGu4UShAha0L6Z3OFyqgJ6SJQCffEow sfFKKoT3ODap6JRpn2I1IfI= =bCqY -END PGP SIGNATURE- -- 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] Clearing global statistics
Rafael Martinez r.m.guerr...@usit.uio.no writes: Is there any chance of implementing a way of knowing when was the last time statistics delivered via pg_stat_* were reset? Actually, that brings up a more general question: what's with the enthusiasm for clearing statistics *at all*? ISTM that's something you should do only in dire emergencies, like the collector went haywire and has now got a bunch of garbage numbers. The notion of resetting subsets of the stats seems even more dubious, because now you have numbers that aren't mutually comparable. So I fail to understand why the desire to expend valuable development time on any of this. 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] Clearing global statistics
Tom Lane wrote:
Actually, that brings up a more general question: what's with the
enthusiasm for clearing statistics *at all*? ISTM that's something
you should do only in dire emergencies, like the collector went
haywire and has now got a bunch of garbage numbers. The notion of
resetting subsets of the stats seems even more dubious, because now
you have numbers that aren't mutually comparable. So I fail to
understand why the desire to expend valuable development time on
any of this.
When doing checkpoint tuning, the usual thing you start with is by
considering the ratio of time to segment-based checkpoints, along with
the corresponding balance of buffers written by the backends vs. the
checkpoint. When that shows poor behavior, typically because
checkpoint_segments is too low, you change its value and then resume
monitoring at the new setting. Right now, you're still carrying around
the history of the bad period forever though, and every check of the
pg_stat_bgwriter requires manually subtracting the earlier values out.
What people would like to do is reset those after adjusting
checkpoint_segments, and then you can eyeball the proportions directly
instead. That's exactly what the patch does. If I didn't see this
request in the field every month I wouldn't have spent a minute on a
patch to add it.
There was a suggestion that subsets of the data I'm clearing might be
useful to target, which I rejected on the bounds that it made it
possible to get an inconsistent set of results as you're concerned
about. You really need to clear everything that shows up in
pg_stat_bgwriter or not touch it at all. The main use case I'm trying
to support is the person who just made a config change and now wants to do:
select pg_stat_reset();
select pg_stat_reset_shared('bgwriter');
So that all of the stats they're now dealing with are from the same
post-tuning time period. Having numbers that are mutually comparable
across the whole system is exactly the reason why this new call is
needed, because there's this one part you just can't touch.
--
Greg Smith2ndQuadrant Baltimore, MD
PostgreSQL Training, Services and Support
g...@2ndquadrant.com www.2ndQuadrant.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] Clearing global statistics
Euler Taveira de Oliveira wrote: Greg Smith escreveu: pg_stat_reset( which text ) which := 'buffers' | 'checkpoints' | 'tables' | 'functions' | ... What about adding 'all' too? Or the idea is resetting all global counters when we call pg_stat_reset() (without parameters)? Once there's more than one piece to clear maybe adding in an 'all' target makes sense. In the context of the update patch I've finished, it just doesn't make sense given the code involved. -- Greg Smith2ndQuadrant Baltimore, MD PostgreSQL Training, Services and Support g...@2ndquadrant.com www.2ndQuadrant.com
Re: [HACKERS] Clearing global statistics
2010/1/14 Tom Lane t...@sss.pgh.pa.us: Rafael Martinez r.m.guerr...@usit.uio.no writes: Is there any chance of implementing a way of knowing when was the last time statistics delivered via pg_stat_* were reset? Actually, that brings up a more general question: what's with the enthusiasm for clearing statistics *at all*? ISTM that's something you should do only in dire emergencies, like the collector went haywire and has now got a bunch of garbage numbers. The notion of resetting subsets of the stats seems even more dubious, because now you have numbers that aren't mutually comparable. So I fail to understand why the desire to expend valuable development time on any of this. s/collector/application/ and you've got one reason. Example, that I hit the other day. Examining pg_stat_user_functions shows one function taking much longer than you'd expect. Called about 6 million times, total time about 7 days spent. Reason turned out to be a missing index. Without clearing the stats, it'll take a *long* time before the average goes down enough to make it possible to use the simple SELECT self_time/calls FROM pg_stat_user_functions WHERE... to monitor. Sure, if you have a system that graphs it, it'll update properly, but for the quick manual checks, that view suddenly becomes a lot less ueful. -- 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] Clearing global statistics
Rafael Martinez wrote:
One thing I miss from the statistics you can get via pg_stat_* is
information about how long we have been collecting stats (or in other
words, when was the last time the stats were reset)
I've considered adding this for the same reasons you're asking about it,
but am not happy with the trade-offs involved. The problem is that you
have to presume the server was running the entirety of the time since
stats were reset for that data to be useful. So unless people are in
that situation, they're going to get data that may not represent what
they think it does. Realistically, if you want a timestamp that always
means something useful you have to rest the stats at every server start,
which leads us to:
Before 8.3, we had the stats_reset_on_server_start parameter and the
pg_postmaster_start_time() function. This was an easy way of resetting
*all* statistics delivered by pg_stat_* and knowing when this was done.
We were able to produce stats with information about sec/hours/days
average values in an easy way.
With this new feature I'm submitting, you can adjust your database
startup scripts to make this happen again. Start the server,
immediately loop over every database and call pg_stat_reset on them all,
and call pg_stat_reset_shared('bgwriter'). Now you've got completely
cleared stats that are within a second or two of
pg_postmaster_start_time(), should be close enough to most purposes.
Theoretically we could automate that better, but I've found it hard to
justify working on given that it's not that difficult to handle outside
of the database once the individual pieces are exposed.
--
Greg Smith2ndQuadrant Baltimore, MD
PostgreSQL Training, Services and Support
g...@2ndquadrant.com www.2ndQuadrant.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] Clearing global statistics
Greg Smith g...@2ndquadrant.com writes: Tom Lane wrote: Actually, that brings up a more general question: what's with the enthusiasm for clearing statistics *at all*? ... Right now, you're still carrying around the history of the bad period forever though, and every check of the pg_stat_bgwriter requires manually subtracting the earlier values out. Seems like a more appropriate solution would be to make it easier to do that subtraction, ie, make it easier to capture the values at a given time point and then get deltas from there. It's more general (you could have multiple saved sets of values), and doesn't require superuser permissions to do, and doesn't have the same potential for damn-I-wish-I-hadn't-done-that moments. 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] Clearing global statistics
Tom Lane wrote: Seems like a more appropriate solution would be to make it easier to do that subtraction, ie, make it easier to capture the values at a given time point and then get deltas from there. It's more general (you could have multiple saved sets of values), and doesn't require superuser permissions to do, and doesn't have the same potential for damn-I-wish-I-hadn't-done-that moments. You can make the same argument about the existing pg_stat_reset mechanism. I would love to completely rework the stats infrastructure so that it's easier to capture values with timestamps, compute diffs, and do trending. However, I'm not sure the database itself is necessarily the best place to do that at anyway. People who know what they're doing are already handling this exact job using external tools that grab regular snapshots for that purpose, so why try to duplicate that work? I'm trying to triage here, to scrub off the worst of the common problems. I would never claim this is the perfect direction to follow forever. There are a number of people who consider the inability to reset the pg_stat_bgwriter stats in any way a bug that's gone unfixed for two versions now. Your larger point that this style of implementation is not ideal as a long-term way to manage statistics I would completely agree with, I just don't have the time to spend on a major rewrite to improve that. What I can offer is a fix for the most common issue I get complaints about, in the form of a tool much more likely to be used correctly by people who go looking for it than misused IMHO. -- Greg Smith2ndQuadrant Baltimore, MD PostgreSQL Training, Services and Support g...@2ndquadrant.com www.2ndQuadrant.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] Clearing global statistics
Greg Smith wrote:
Before 8.3, we had the stats_reset_on_server_start parameter and
the pg_postmaster_start_time() function. This was an easy way of
resetting *all* statistics delivered by pg_stat_* and knowing when
this was done. We were able to produce stats with information about
sec/hours/days average values in an easy way.
With this new feature I'm submitting, you can adjust your database
startup scripts to make this happen again. Start the server,
immediately loop over every database and call pg_stat_reset on them
all, and call pg_stat_reset_shared('bgwriter'). Now you've got
completely cleared stats that are within a second or two of
pg_postmaster_start_time(), should be close enough to most purposes.
Theoretically we could automate that better, but I've found it hard
to justify working on given that it's not that difficult to handle
outside of the database once the individual pieces are exposed.
Great, this is good enough and we get what we need. Thanks :-)
regards
--
Rafael Martinez, r.m.guerr...@usit.uio.no
Center for Information Technology Services
University of Oslo, Norway
PGP Public Key: http://folk.uio.no/rafael/
--
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] Clearing global statistics
Itagaki Takahiro wrote:
To be honest, I have a plan to add performance statistics counters to
postgres. It is not bgwriter's counters, but cluster-level. I'd like
to use your infrastructure in my work, too :)
Attached patch provides just that. It still works basically the same as
my earlier version, except you pass it a name of what you want to reset,
and if you don't give it the only valid one right now ('bgwriter') it
rejects it (for now):
gsmith=# select checkpoints_req,buffers_alloc from pg_stat_bgwriter;
checkpoints_req | buffers_alloc
-+---
4 | 129
(1 row)
gsmith=# select pg_stat_reset_shared('bgwriter');
pg_stat_reset_shared
--
(1 row)
gsmith=# select checkpoints_req,buffers_alloc from pg_stat_bgwriter;
checkpoints_req | buffers_alloc
-+---
0 | 7
(1 row)
gsmith=# select pg_stat_reset_shared('rubbish');
ERROR: Unrecognized reset target
I turn the input text into an enum choice as part of composing the
message to the stats collector. If you wanted to add some other shared
cluster-wide reset capabilities into there you could re-use most of this
infrastructure. Just add an extra enum value, map the text into that
enum, and write the actual handler that does the reset work. Should be
able to reuse the same new message type and external UI I implemented
for this specific clearing feature.
I didn't see any interaction to be concerned about here with Magnus's
suggestion he wanted to target stats reset on objects such as a single
table at some point.
The main coding choice I wasn't really sure about is how I flag the
error case where you pass bad in. I do that validation and throw
ERRCODE_SYNTAX_ERROR before composing the message to the stats
collector. Didn't know if I should create a whole new error code just
for this specific case or if reusing another error code was more
appropriate. Also, I didn't actually have the collector process itself
validate the data at all, it just quietly ignores bad messages on the
presumption everything is already being checked during message creation.
That seems consistent with the other code here--the other message
handlers only seem to throw errors when something really terrible
happens, not when they just don't find something useful to do.
--
Greg Smith2ndQuadrant Baltimore, MD
PostgreSQL Training, Services and Support
g...@2ndquadrant.com www.2ndQuadrant.com
diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml
index 1f70fd4..b630bc8 100644
*** a/doc/src/sgml/monitoring.sgml
--- b/doc/src/sgml/monitoring.sgml
*** postgres: replaceableuser/ replacea
*** 918,923
--- 918,934
(requires superuser privileges)
/entry
/row
+
+ row
+ entryliteralfunctionpg_stat_reset_shared/function()/literal/entry
+ entrytypetext/type/entry
+ entry
+Reset some of the shared statistics counters for the database cluster to
+zero (requires superuser privileges). Calling
+literalpg_stat_reset_shared('bgwriter')/ will zero all the values shown by
+structnamepg_stat_bgwriter/.
+ /entry
+ /row
/tbody
/tgroup
/table
diff --git a/src/backend/postmaster/pgstat.c b/src/backend/postmaster/pgstat.c
index 4fd2abf..7335a50 100644
*** a/src/backend/postmaster/pgstat.c
--- b/src/backend/postmaster/pgstat.c
*** static void pgstat_recv_tabstat(PgStat_M
*** 270,275
--- 270,276
static void pgstat_recv_tabpurge(PgStat_MsgTabpurge *msg, int len);
static void pgstat_recv_dropdb(PgStat_MsgDropdb *msg, int len);
static void pgstat_recv_resetcounter(PgStat_MsgResetcounter *msg, int len);
+ static void pgstat_recv_resetsharedcounter(PgStat_MsgResetsharedcounter *msg, int len);
static void pgstat_recv_autovac(PgStat_MsgAutovacStart *msg, int len);
static void pgstat_recv_vacuum(PgStat_MsgVacuum *msg, int len);
static void pgstat_recv_analyze(PgStat_MsgAnalyze *msg, int len);
*** pgstat_reset_counters(void)
*** 1153,1158
--- 1154,1190
pgstat_send(msg, sizeof(msg));
}
+ /* --
+ * pgstat_reset_shared_counters() -
+ *
+ * Tell the statistics collector to reset cluster-wide shared counters.
+ * --
+ */
+ void
+ pgstat_reset_shared_counters(const char *target)
+ {
+ PgStat_MsgResetsharedcounter msg;
+
+ if (pgStatSock 0)
+ return;
+
+ if (!superuser())
+ ereport(ERROR,
+ (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+ errmsg(must be superuser to reset statistics counters)));
+
+ if (strcmp(target, bgwriter) == 0)
+ msg.m_resettarget = RESET_BGWRITER;
+ else
+ {
+ ereport(ERROR,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg(Unrecognized reset target)));
+ }
+
+ pgstat_setheader(msg.m_hdr, PGSTAT_MTYPE_RESETSHAREDCOUNTER);
+ pgstat_send(msg, sizeof(msg));
+ }
/* --
* pgstat_report_autovac() -
***
Re: [HACKERS] Clearing global statistics
Magnus Hagander wrote:
I have on my TODO to implement the ability to do stats reset on a
single object (say, one table only). Please take this into
consideration when you design/name this, so theres no unnecessary
overlap :-) Same goes for the stats message itself.
The idea suggested upthread was to add this:
pg_stat_reset( which text )
which := 'buffers' | 'checkpoints' | 'tables' | 'functions' | ...
Now, the way the pg_stat_bgwriter tables are computed, it doesn't
actually make sense to separate out clearing the buffers/checkpoints
stats, since one of those values is in both categories:
buffers_checkpoint. They're really all too tightly coupled to break
them apart. So I was thinking of this:
pg_stat_reset( which text )
which := 'bgwriter' | ...
I could convert the patch I've got to be an initial implementation of
this new pg_stat_reset with a parameter, laying some useful groundwork
in the process too. Then people who want to reset more things can just
re-use that same outline and message passing mechanism, just adding
comparisons for new text and a handler to go with it--not even touching
the catalog again.
This may not mesh well with what you plan though. If pg_stat_reset is
updated to reset stats on an individual table, that could be a second
version that takes in a regclass:
pg_stat_reset('tablename'::regclass)
But that seems like a confusing bit of overloading--I can easily see
people thinking that pg_stat_reset('bgwriter') would be resetting the
stats for a relation named 'bgwriter' rather than what it actually does
if I build it that way.
So, combining with Peter's naming suggestion, I think what I should
build is:
pg_stat_reset_shared( which text )
which := 'bgwriter' | ...
Which satisfies what I'm looking for now, and future patches that need
to reset other shared across the cluster statistics can re-use this
without needing to add a whole new function/stats message. I think that
satisfies the cross-section of planned use cases we're expecting now best.
Any comments before I update my patch to do that?
--
Greg Smith2ndQuadrant Baltimore, MD
PostgreSQL Training, Services and Support
g...@2ndquadrant.com www.2ndQuadrant.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] Clearing global statistics
Greg Smith escreveu: pg_stat_reset( which text ) which := 'buffers' | 'checkpoints' | 'tables' | 'functions' | ... What about adding 'all' too? Or the idea is resetting all global counters when we call pg_stat_reset() (without parameters)? -- Euler Taveira de Oliveira http://www.timbira.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] Clearing global statistics
On sön, 2009-12-06 at 19:50 -0500, Greg Smith wrote: The fact that you're asking the question this way suggests to me I've named this completely wrong. pg_stat_reset_global only resets the bits global to all databases. It doesn't touch any of the database-specific things that pg_stat_reset can handle right now. At the moment, the only global information is what's in pg_stat_bgwriter: buffer statistics and checkpoint stats. I'm thinking that I should rename this new function to pg_stat_reset_bgwriter so it's obvious how limited its target is. Using either global or cluster for the name is just going to leave people thinking it acts across a much larger area than it does. The term shared is used elsewhere to describe the, well, shared catalogs. -- 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] Clearing global statistics
2009/12/7 Greg Smith g...@2ndquadrant.com: Itagaki Takahiro wrote: Greg Smith g...@2ndquadrant.com wrote: I'm thinking that I should rename this new function to pg_stat_reset_bgwriter so it's obvious how limited its target is. I don't think it is a good name because we might have another cluster-level statictics not related with bgwriter in the future. I hope you will suggest extensible method when you improve this area. I follow what you mean now. I'll take a look at allowing pg_stat_reset to act on an input as you suggested, rather than adding more of these UDFs for every time somebody adds a new area they want to target clearing. I have on my TODO to implement the ability to do stats reset on a single object (say, one table only). Please take this into consideration when you design/name this, so theres no unnecessary overlap :-) Same goes for the stats message itself. -- 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] Clearing global statistics
Greg Smith g...@2ndquadrant.com wrote: This implements the TODO item Allow the clearing of cluster-level statistics, based on previous discussion ending at http://archives.postgresql.org/pgsql-hackers/2009-03/msg00920.php -Not sure if this should be named pg_stat_rest_global (to match the way these are called global stats in the source) or pg_stat_reset_cluster. Picked the former for V1, not attached to that decision at all. Might even make sense to use a name that makes it obvious the pg_stat_bgwriter data is what's targeted. A couple of comments: * We will be able to reset global counters and current database's counters. Do we need to have a method to reset other databases' counters? Or, will pg_stat_reset_global just reset counters of all databases? * Is it useful to have a method to reset counters separately? For example, pg_stat_reset( which text ) which := 'buffers' | 'checkpoints' | 'tables' | 'functions' | ... Regards, --- ITAGAKI Takahiro NTT Open Source Software Center -- 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] Clearing global statistics
Itagaki Takahiro wrote: Greg Smith g...@2ndquadrant.com wrote: -Not sure if this should be named pg_stat_rest_global (to match the way these are called global stats in the source) or pg_stat_reset_cluster. Picked the former for V1, not attached to that decision at all. Might even make sense to use a name that makes it obvious the pg_stat_bgwriter data is what's targeted. A couple of comments: * We will be able to reset global counters and current database's counters. Do we need to have a method to reset other databases' counters? Or, will pg_stat_reset_global just reset counters of all databases? * Is it useful to have a method to reset counters separately? For example, pg_stat_reset( which text ) which := 'buffers' | 'checkpoints' | 'tables' | 'functions' | ... The fact that you're asking the question this way suggests to me I've named this completely wrong. pg_stat_reset_global only resets the bits global to all databases. It doesn't touch any of the database-specific things that pg_stat_reset can handle right now. At the moment, the only global information is what's in pg_stat_bgwriter: buffer statistics and checkpoint stats. I'm thinking that I should rename this new function to pg_stat_reset_bgwriter so it's obvious how limited its target is. Using either global or cluster for the name is just going to leave people thinking it acts across a much larger area than it does. -- Greg Smith2ndQuadrant Baltimore, MD PostgreSQL Training, Services and Support g...@2ndquadrant.com www.2ndQuadrant.com
Re: [HACKERS] Clearing global statistics
Greg Smith g...@2ndquadrant.com wrote: I'm thinking that I should rename this new function to pg_stat_reset_bgwriter so it's obvious how limited its target is. I don't think it is a good name because we might have another cluster-level statictics not related with bgwriter in the future. I hope you will suggest extensible method when you improve this area. To be honest, I have a plan to add performance statistics counters to postgres. It is not bgwriter's counters, but cluster-level. I'd like to use your infrastructure in my work, too :) Regards, --- ITAGAKI Takahiro NTT Open Source Software Center -- 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] Clearing global statistics
Itagaki Takahiro wrote: Greg Smith g...@2ndquadrant.com wrote: I'm thinking that I should rename this new function to pg_stat_reset_bgwriter so it's obvious how limited its target is. I don't think it is a good name because we might have another cluster-level statictics not related with bgwriter in the future. I hope you will suggest extensible method when you improve this area. I follow what you mean now. I'll take a look at allowing pg_stat_reset to act on an input as you suggested, rather than adding more of these UDFs for every time somebody adds a new area they want to target clearing. -- Greg Smith2ndQuadrant Baltimore, MD PostgreSQL Training, Services and Support g...@2ndquadrant.com www.2ndQuadrant.com
