Horton
Menu

Thread

  1. Resetting a single statistics counter

    Magnus Hagander <magnus@hagander.net> — 2010-01-24T16:37:46Z

    In the spirit of finishing off small patches, attached is the one that
implements pg_stat_reset_single(), to be able to reset stats for a
single table or function. I kind of thought it would be included in
the patch from Greg Smith for shared counters so I put it aside, but I
guess I misunderstood him there. Anyway, I polished off the final
part, and here it is.

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

  2. Re: Resetting a single statistics counter

    Tom Lane <tgl@sss.pgh.pa.us> — 2010-01-24T17:12:07Z

    Magnus Hagander <magnus@hagander.net> writes:
> In the spirit of finishing off small patches, attached is the one that
> implements pg_stat_reset_single(), to be able to reset stats for a
> single table or function. I kind of thought it would be included in
> the patch from Greg Smith for shared counters so I put it aside, but I
> guess I misunderstood him there. Anyway, I polished off the final
> part, and here it is.

This is bogus; it assumes tables and functions will not have the same
OIDs.

			regards, tom lane

  3. Re: Resetting a single statistics counter

    Magnus Hagander <magnus@hagander.net> — 2010-01-24T17:25:38Z

    2010/1/24 Tom Lane <tgl@sss.pgh.pa.us>:
> Magnus Hagander <magnus@hagander.net> writes:
>> In the spirit of finishing off small patches, attached is the one that
>> implements pg_stat_reset_single(), to be able to reset stats for a
>> single table or function. I kind of thought it would be included in
>> the patch from Greg Smith for shared counters so I put it aside, but I
>> guess I misunderstood him there. Anyway, I polished off the final
>> part, and here it is.
>
> This is bogus; it assumes tables and functions will not have the same
> OIDs.

Gah... *faceinpalms*

Off to make it two separate functions.. (seems much more user-friendly
than a single function with an extra argument, IMHO)


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

  4. Re: Resetting a single statistics counter

    Simon Riggs <simon@2ndquadrant.com> — 2010-01-24T17:41:55Z

    On Sun, 2010-01-24 at 18:25 +0100, Magnus Hagander wrote:
> 2010/1/24 Tom Lane <tgl@sss.pgh.pa.us>:
> > Magnus Hagander <magnus@hagander.net> writes:
> >> In the spirit of finishing off small patches, attached is the one that
> >> implements pg_stat_reset_single(), to be able to reset stats for a
> >> single table or function. I kind of thought it would be included in
> >> the patch from Greg Smith for shared counters so I put it aside, but I
> >> guess I misunderstood him there. Anyway, I polished off the final
> >> part, and here it is.
> >
> > This is bogus; it assumes tables and functions will not have the same
> > OIDs.
> 
> Gah... *faceinpalms*
> 
> Off to make it two separate functions.. (seems much more user-friendly
> than a single function with an extra argument, IMHO)

And a much better name also :-)

-- 
 Simon Riggs           www.2ndQuadrant.com

  5. Re: Resetting a single statistics counter

    Magnus Hagander <magnus@hagander.net> — 2010-01-24T17:45:47Z

    2010/1/24 Magnus Hagander <magnus@hagander.net>:
> 2010/1/24 Tom Lane <tgl@sss.pgh.pa.us>:
>> Magnus Hagander <magnus@hagander.net> writes:
>>> In the spirit of finishing off small patches, attached is the one that
>>> implements pg_stat_reset_single(), to be able to reset stats for a
>>> single table or function. I kind of thought it would be included in
>>> the patch from Greg Smith for shared counters so I put it aside, but I
>>> guess I misunderstood him there. Anyway, I polished off the final
>>> part, and here it is.
>>
>> This is bogus; it assumes tables and functions will not have the same
>> OIDs.
>
> Gah... *faceinpalms*
>
> Off to make it two separate functions.. (seems much more user-friendly
> than a single function with an extra argument, IMHO)

Here goes.


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

  6. Re: Resetting a single statistics counter

    Euler Taveira <euler@timbira.com> — 2010-01-24T18:04:04Z

    Magnus Hagander escreveu:
>> Off to make it two separate functions.. (seems much more user-friendly
>> than a single function with an extra argument, IMHO)
> 
+1. But as Simon said _single_ is too ugly. What about
pg_stat_reset_user_{function,relation}?

Another thing that is not a problem of your patch but it needs to be fixed is
that resetting functions remove the line from pg_stat_user_functions; that a
different behavior from other pg_stat_user_* functions.


-- 
  Euler Taveira de Oliveira
  http://www.timbira.com/

  7. Re: Resetting a single statistics counter

    Tom Lane <tgl@sss.pgh.pa.us> — 2010-01-24T18:04:22Z

    Magnus Hagander <magnus@hagander.net> writes:
> Here goes.

Looks much saner.  One minor stylistic gripe:

+Datum
+pg_stat_reset_single_table(PG_FUNCTION_ARGS)
+{
+	pgstat_reset_single_counter(PG_GETARG_OID(0), RESET_TABLE);
+
+	PG_RETURN_VOID();
+}

I don't like sticking PG_GETARG calls inline in the body of a V1-protocol
function, even in trivial cases like this.  I think better style is

	Oid	taboid = PG_GETARG_OID(0);

	pgstat_reset_single_counter(taboid, RESET_TABLE);

This approach associates a clear name and type with each argument,
thereby helping to buy back some of the readability we lose by not
being able to use regular C function declarations.  When we designed
the V1 call protocol, I had hoped we might someday have scripts that
would crosscheck such declarations against the pg_proc contents, and
I still haven't entirely given up that idea ...

			regards, tom lane

  8. Re: Resetting a single statistics counter

    Tom Lane <tgl@sss.pgh.pa.us> — 2010-01-24T18:13:52Z

    Euler Taveira de Oliveira <euler@timbira.com> writes:
> Magnus Hagander escreveu:
>> Off to make it two separate functions.. (seems much more user-friendly
>> than a single function with an extra argument, IMHO)

> +1. But as Simon said _single_ is too ugly. What about
> pg_stat_reset_user_{function,relation}?

That implies that the operations wouldn't work against system tables;
which they do.  I think a bigger problem is that "reset_single_table"
seems like it might be talking about something like a TRUNCATE, ie,
it's not clear that it means to reset counters rather than data.
The pg_stat_ prefix is some help but not enough IMO.  So I suggest
pg_stat_reset_table_counters and pg_stat_reset_function_counters.

(BTW, a similar complaint could be made about the previously committed
patch: reset shared what?)

			regards, tom lane

  9. Re: Resetting a single statistics counter

    Magnus Hagander <magnus@hagander.net> — 2010-01-24T18:21:19Z

    2010/1/24 Tom Lane <tgl@sss.pgh.pa.us>:
> Euler Taveira de Oliveira <euler@timbira.com> writes:
>> Magnus Hagander escreveu:
>>> Off to make it two separate functions.. (seems much more user-friendly
>>> than a single function with an extra argument, IMHO)
>
>> +1. But as Simon said _single_ is too ugly. What about
>> pg_stat_reset_user_{function,relation}?
>
> That implies that the operations wouldn't work against system tables;
> which they do.  I think a bigger problem is that "reset_single_table"
> seems like it might be talking about something like a TRUNCATE, ie,
> it's not clear that it means to reset counters rather than data.
> The pg_stat_ prefix is some help but not enough IMO.  So I suggest
> pg_stat_reset_table_counters and pg_stat_reset_function_counters.

Doesn't the pg_stat_ part already say this?


> (BTW, a similar complaint could be made about the previously committed
> patch: reset shared what?)

Well, it could also be made about the original pg_stat_reset()
function - reset what?


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

  10. Re: Resetting a single statistics counter

    Tom Lane <tgl@sss.pgh.pa.us> — 2010-01-24T18:33:27Z

    Magnus Hagander <magnus@hagander.net> writes:
> 2010/1/24 Tom Lane <tgl@sss.pgh.pa.us>:
>> The pg_stat_ prefix is some help but not enough IMO. So I suggest
>> pg_stat_reset_table_counters and pg_stat_reset_function_counters.

> Doesn't the pg_stat_ part already say this?

My objection is that "reset_table" sounds like something you do to a
table, not something you do to stats.  No, I don't think the prefix is
enough to clarify that.

>> (BTW, a similar complaint could be made about the previously committed
>> patch: reset shared what?)

> Well, it could also be made about the original pg_stat_reset()
> function - reset what?

In that case, there's nothing but the "stat" to suggest what gets
reset, so I think it's less likely to be misleading than the current
proposals.  But if we'd been designing all of these at once, yeah,
I'd have argued for a more verbose name for that one too.

			regards, tom lane

  11. Re: Resetting a single statistics counter

    Magnus Hagander <magnus@hagander.net> — 2010-01-24T18:40:41Z

    2010/1/24 Tom Lane <tgl@sss.pgh.pa.us>:
> Magnus Hagander <magnus@hagander.net> writes:
>> 2010/1/24 Tom Lane <tgl@sss.pgh.pa.us>:
>>> The pg_stat_ prefix is some help but not enough IMO.  So I suggest
>>> pg_stat_reset_table_counters and pg_stat_reset_function_counters.
>
>> Doesn't the pg_stat_ part already say this?
>
> My objection is that "reset_table" sounds like something you do to a
> table, not something you do to stats.  No, I don't think the prefix is
> enough to clarify that.

Fair enough, I'll just add the _counters to all three functions then.


>>> (BTW, a similar complaint could be made about the previously committed
>>> patch: reset shared what?)
>
>> Well, it could also be made about the original pg_stat_reset()
>> function - reset what?
>
> In that case, there's nothing but the "stat" to suggest what gets
> reset, so I think it's less likely to be misleading than the current
> proposals.  But if we'd been designing all of these at once, yeah,
> I'd have argued for a more verbose name for that one too.

Ok.

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

  12. Re: Resetting a single statistics counter

    Euler Taveira <euler@timbira.com> — 2010-01-24T18:50:34Z

    Tom Lane escreveu:
> That implies that the operations wouldn't work against system tables;
> which they do.  I think a bigger problem is that "reset_single_table"
> seems like it might be talking about something like a TRUNCATE, ie,
> it's not clear that it means to reset counters rather than data.
> The pg_stat_ prefix is some help but not enough IMO.  So I suggest
> pg_stat_reset_table_counters and pg_stat_reset_function_counters.
> 
Sure, much better. +1.

> (BTW, a similar complaint could be made about the previously committed
> patch: reset shared what?)
> 
BTW, what about that idea to overload pg_stat_reset()? The
pg_stat_reset_shared should be renamed to pg_stat_reset('foo') [1] where foo
is the class of objects that it is resetting. pg_stat_reset is not a so
suggestive name but that's one we already have; besides, it will be intuitive
for users.


[1] http://archives.postgresql.org/pgsql-hackers/2010-01/msg01317.php


-- 
  Euler Taveira de Oliveira
  http://www.timbira.com/

  13. Re: Resetting a single statistics counter

    Magnus Hagander <magnus@hagander.net> — 2010-01-24T19:23:19Z

    2010/1/24 Euler Taveira de Oliveira <euler@timbira.com>:
> Tom Lane escreveu:
>> That implies that the operations wouldn't work against system tables;
>> which they do.  I think a bigger problem is that "reset_single_table"
>> seems like it might be talking about something like a TRUNCATE, ie,
>> it's not clear that it means to reset counters rather than data.
>> The pg_stat_ prefix is some help but not enough IMO.  So I suggest
>> pg_stat_reset_table_counters and pg_stat_reset_function_counters.
>>
> Sure, much better. +1.
>
>> (BTW, a similar complaint could be made about the previously committed
>> patch: reset shared what?)
>>
> BTW, what about that idea to overload pg_stat_reset()? The
> pg_stat_reset_shared should be renamed to pg_stat_reset('foo') [1] where foo
> is the class of objects that it is resetting. pg_stat_reset is not a so
> suggestive name but that's one we already have; besides, it will be intuitive
> for users.

I think it's easier to use the way it is now. But yes, we could
overload it to make it:
pg_stat_reset()   : everything, like now
pg_stat_reset('bgwriter')       : what pg_stat_reset_shared() does
now. Can take more params.
pg_stat_reset('table', 'foo'::regclass);     : what
pg_stat_reset_single_table_counters does now

The advantage would be fewer functions, but I still think it's easier
to use the way we have it now.


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