keeping a timestamp of the last stats reset (for a db, table and function)

Hello

you have to respect pg coding style:

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

Zdar

Pavel

2010/12/11 <tv@fuzzy.cz>:

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
Tomas

On Sat, Dec 11, 2010 at 4:40 PM, <tv@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

Dne 12.12.2010 03:47, Robert Haas napsal(a):

On Sat, Dec 11, 2010 at 4:40 PM, <tv@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

Tomas Vondra <tv@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

Tomas Vondra <tv@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

tv@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

On Dec 18, 2010, at 8:51 PM, Tom Lane wrote:

Tomas Vondra <tv@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 jim@nasby.net
512.569.9461 (cell) http://jim.nasby.net

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

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

Tomas Vondra <tv@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

Dne 19.12.2010 20:28, Tom Lane napsal(a):

Tomas Vondra <tv@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

Tomas Vondra <tv@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

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

Dne 19.12.2010 23:58, Tom Lane napsal(a):

Tomas Vondra <tv@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

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.

2010/12/23 Tomas Vondra <tv@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

Dne 23.12.2010 20:09, Robert Haas napsal(a):

2010/12/23 Tomas Vondra <tv@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

tv@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 US greg@2ndQuadrant.com Baltimore, MD
PostgreSQL Training, Services and Support www.2ndQuadrant.us
"PostgreSQL 9.0 High Performance": http://www.2ndQuadrant.com/books