Thread: Re: [GENERAL] Stats Collector
> Looks to me, someone forgot something. That would be me and now I
> remember that I originally wanted to add some utility command for that.
>
> What you need in the meantime is a little C function that calls
>
> void pgstat_reset_counters(void);
>
> I might find the time tomorrow to write one for you if you don't know
> how.
Is this the kind of thing you mean?
#include "postgres.h"
#include "fmgr.h"
extern Datum pg_reset_stats(PG_FUNCTION_ARGS);
PG_FUNCTION_INFO_V1(pg_reset_stats);
Datum
pg_reset_stats(PG_FUNCTION_ARGS)
{ void pgstat_reset_counters(void);
PG_RETURN_VOID();
}
With this code I get this:
test=# select pg_reset_stats();
ERROR: Unable to look up type id 0
I'm creating it like this:
create or replace function pg_reset_stats() returns opaque as
'/home/chriskl/local/lib/postgresql/pg_reset_stats.so' language 'C';
Is it something to do with the return type being declared wrongly?
Hmm...the manual indicates that opaque functions cannot be called directly -
so what the heck do I do?
Also, where would I put this function in the main postgres source and how
would I modify initdb?
Chris
"Christopher Kings-Lynne" <chriskl@familyhealth.com.au> writes:
> Is it something to do with the return type being declared wrongly?
Yup. Make it return a useless '1' or 'true' or some such.
regards, tom lane
OK, now I run it and it does absolutely nothing to the pg_stat_all_tables relation for instance. In fact, it seems to do nothing at all - does the reset function even work? Chris > -----Original Message----- > From: pgsql-hackers-owner@postgresql.org > [mailto:pgsql-hackers-owner@postgresql.org]On Behalf Of Tom Lane > Sent: Monday, 29 July 2002 2:19 PM > To: Christopher Kings-Lynne > Cc: Jan Wieck; Hackers > Subject: Re: [HACKERS] [GENERAL] Stats Collector > > > "Christopher Kings-Lynne" <chriskl@familyhealth.com.au> writes: > > Is it something to do with the return type being declared wrongly? > > Yup. Make it return a useless '1' or 'true' or some such. > > regards, tom lane > > ---------------------------(end of broadcast)--------------------------- > TIP 4: Don't 'kill -9' the postmaster >
> OK, now I run it and it does absolutely nothing to the pg_stat_all_tables > relation for instance. In fact, it seems to do nothing at all - does the > reset function even work? OK, I'm an idiot, I was calling the funciton like this: void blah(void) which actually does nothing. It all works now and I have just submitted it to -patches as a new contrib, but it probably should make its way into the backend one day. Chris
Christopher Kings-Lynne wrote: > > OK, now I run it and it does absolutely nothing to the pg_stat_all_tables > > relation for instance. In fact, it seems to do nothing at all - does the > > reset function even work? > > OK, I'm an idiot, I was calling the funciton like this: void blah(void) > which actually does nothing. > > It all works now and I have just submitted it to -patches as a new contrib, > but it probably should make its way into the backend one day. OK, the big question is how do we want to make stats reset visible to users? The current patch uses a function call. Is that how we want to do it? -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 853-3000+ If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania19026
Bruce Momjian <pgman@candle.pha.pa.us> writes:
>> It all works now and I have just submitted it to -patches as a new contrib,
>> but it probably should make its way into the backend one day.
> OK, the big question is how do we want to make stats reset visible to
> users? The current patch uses a function call. Is that how we want to
> do it?
Should we make it visible at all? I'm concerned about security.
regards, tom lane
Tom Lane wrote: > Bruce Momjian <pgman@candle.pha.pa.us> writes: > >> It all works now and I have just submitted it to -patches as a new contrib, > >> but it probably should make its way into the backend one day. > > > OK, the big question is how do we want to make stats reset visible to > > users? The current patch uses a function call. Is that how we want to > > do it? > > Should we make it visible at all? I'm concerned about security. Yea, as long as it is in contrib, only the super-user can install it, but once installed, anyone can run it, I think. A function seems like the wrong way to go on this. SET has super-user protections we could use to control this but I am not sure what SET syntax to use. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 853-3000+ If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania19026
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> A function seems like the wrong way to go on this. SET has super-user
> protections we could use to control this but I am not sure what SET
> syntax to use.
I don't like SET for it --- SET is for setting state that will persist
over some period of time, not for taking one-shot actions. We could
perhaps use a function that checks that it's been called by the
superuser.
However, the real question is what is the use-case for this feature
anyway. Why should people want to reset the stats while the system
is running? If we had a clear example then it might be more apparent
what restrictions to place on it.
regards, tom lane
On Tue, Jul 30, 2002 at 04:21:24PM -0400, Tom Lane wrote: > However, the real question is what is the use-case for this feature > anyway. Why should people want to reset the stats while the system > is running? If we had a clear example then it might be more apparent > what restrictions to place on it. Well, you might want to use the statistics as part of a monthly reporting cycle, for instance. You could archive the old results, and then reset, so that you have information by month. Or you might have made a number of changes to a database which has been running for a while, and want to see whether the changes have had the desired effect. (Say, whether some new index has helped things.) Or you might want to track whether some training you've done for your users has been effective in teaching them how to do certain things, which has resulted in a reduction of rolled back transactions. Or you might just want to reduce your overhead. If you want statistics, but you are not allowed to shut down your database, you have to keep the statistics until the next planned service outage. Maybe you're generating a lot of data; it'd be nice to keep overhead light on your production machines, so you could reset every week. Those are some things I can think of off the top of my head. I can appreciate the security concern, however. And you could probably work around things in such a way that you could get all of this anyway. Still, if it's possible, it'd be nice to have. A -- ---- Andrew Sullivan 87 Mowat Avenue Liberty RMS Toronto, Ontario Canada <andrew@libertyrms.info> M6K 3E3 +1 416 646 3304 x110
Tom Lane wrote: > Bruce Momjian <pgman@candle.pha.pa.us> writes: > > A function seems like the wrong way to go on this. SET has super-user > > protections we could use to control this but I am not sure what SET > > syntax to use. > > I don't like SET for it --- SET is for setting state that will persist > over some period of time, not for taking one-shot actions. We could > perhaps use a function that checks that it's been called by the > superuser. > > However, the real question is what is the use-case for this feature > anyway. Why should people want to reset the stats while the system > is running? If we had a clear example then it might be more apparent > what restrictions to place on it. Yep, I think Andrew explained possible uses. You may want to reset the counters and run a benchmark to look at the results. Should we have RESET clear the counter, perhaps RESET STATCOLLECTOR? I don't think we have other RESET variables that can't be SET, but I don't see a problem with it. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 853-3000+ If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania19026
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> Tom Lane wrote:
>> I don't like SET for it --- SET is for setting state that will persist
>> over some period of time, not for taking one-shot actions. We could
>> perhaps use a function that checks that it's been called by the
>> superuser.
> Should we have RESET clear the counter, perhaps RESET STATCOLLECTOR?
> I don't think we have other RESET variables that can't be SET, but I
> don't see a problem with it.
RESET is just a variant form of SET. It's not for one-shot actions
either (and especially not for one-shot actions against state that's
not accessible to SHOW or SET...)
I still like the function-call approach better.
regards, tom lane
Tom Lane wrote: > Bruce Momjian <pgman@candle.pha.pa.us> writes: > > Tom Lane wrote: > >> I don't like SET for it --- SET is for setting state that will persist > >> over some period of time, not for taking one-shot actions. We could > >> perhaps use a function that checks that it's been called by the > >> superuser. > > > Should we have RESET clear the counter, perhaps RESET STATCOLLECTOR? > > I don't think we have other RESET variables that can't be SET, but I > > don't see a problem with it. > > RESET is just a variant form of SET. It's not for one-shot actions > either (and especially not for one-shot actions against state that's > not accessible to SHOW or SET...) > > I still like the function-call approach better. OK, so you are suggesting a function call, and a check in there to make sure it is the superuser. Comments? -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 853-3000+ If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania19026
> Or you might have made a number of changes to a database which has > been running for a while, and want to see whether the changes have > had the desired effect. (Say, whether some new index has helped > things.) Well out stats had gotten up in to the millions and hence were useless when I made some query changes designed to remove a lot of seqscans. I also made some changes that might have caused indices to no longer be used, and hence I want to know if they ever switch from 0 uses to some uses... If only the super user can install it - then surely the superuser can GRANT usage permissions on it? Otherwise, how do I put in a superuser check? Do I just do it ALTER TABLE-style? Chris
> Bruce Momjian <pgman@candle.pha.pa.us> writes:
> >> It all works now and I have just submitted it to -patches as a
> new contrib,
> >> but it probably should make its way into the backend one day.
>
> > OK, the big question is how do we want to make stats reset visible to
> > users? The current patch uses a function call. Is that how we want to
> > do it?
>
> Should we make it visible at all? I'm concerned about security.
The function it's calling in the backend:
void
pgstat_reset_counters(void)
{ PgStat_MsgResetcounter msg;
if (pgStatSock < 0) return;
if (!superuser()) elog(ERROR, "Only database superusers can reset statistic
counters");
pgstat_setheader(&msg.m_hdr, PGSTAT_MTYPE_RESETCOUNTER); pgstat_send(&msg, sizeof(msg));
}
Note it does actually check that you're a superuser...
Chris