Prev: PATCH: Add hstore_to_json()
Next: Time to run initdb is mostly figure-out-the-timezone work
From: =?iso-8859-1?q?C=E9dric_Villemain?= on 18 Dec 2009 05:23
Le vendredi 18 décembre 2009 09:44:40, Takahiro Itagaki a écrit :
> We have infrastructure to count numbers buffer access in 8.5 Alpha 3.
> I'd like to add per-query buffer usage into contrib/pg_stat_statements.
>
> The pg_stat_statements view will have the same contents with
> struct BufferUsage. Fields named shared_blks_{hit|read|written},
> local_blks_{hit|read|written}, and temp_blks_{read|written}
> will be added to the view.
>
> We can determine slow queries not only based on durations but also
> based on I/O or memory access count. Also, queries with non-zero
> temp_blks_read means DBA need to consider increasing work_mem. Those
> information would be useful to find where the server's bottleneck is.
>
> Additional management costs cannot be avoided, but I think it should be
> not so high because we accumulate buffer usage only once per query,
> while EXPLAIN BUFFERS is slow because we need per-tuple calculation.
>
> I'll submit this pg_stat_statements enhancement to the next commit fest.
> Comments welcome.

Very good idea.
Perhaps it can be usefull to have the percentage for hit/read ratio computed
in the view ?

>
> Regards,
> ---
> Takahiro Itagaki
> NTT Open Source Software Center
>

--
Cédric Villemain
Administrateur de Base de Données
Cel: +33 (0)6 74 15 56 53
http://dalibo.com - http://dalibo.org
From: Takahiro Itagaki on 22 Dec 2009 03:27

Cedric Villemain <cedric.villemain(a)dalibo.com> wrote:

> Le vendredi 18 decembre 2009 09:44:40, Takahiro Itagaki a ecrit :
> > I'd like to add per-query buffer usage into contrib/pg_stat_statements.

Here is a patch to add buffer usage columns into pg_stat_statements.
It also changes initialzation of the result TupleDesc from manually
coded routines to get_call_result_type().

> Perhaps it can be usefull to have the percentage for hit/read ratio computed
> in the view ?

I think different DBAs want different derived values; Some of them might want
the buffer hit ratio, but others might request numbers per query. I'd like to
privide only raw values from pg_stat_statements to keep it simple, but I added
a computational expression of hit percentage in the documentation.

! bench=# SELECT query, calls, total_time, rows, 100.0 * shared_blks_hit /
! nullif(shared_blks_hit + shared_blks_read, 0) AS hit_percent
! FROM pg_stat_statements ORDER BY total_time DESC LIMIT 5;
! -[ RECORD 1 ]---------------------------------------------------------------------
! query | UPDATE pgbench_branches SET bbalance = bbalance + $1 WHERE bid = $2;
! calls | 3000
! total_time | 9.60900100000002
! rows | 2836
! hit_percent | 99.9778970000200936

Regards,
---
Takahiro Itagaki
NTT Open Source Software Center

From: =?ISO-8859-1?Q?C=E9dric_Villemain?= on 22 Dec 2009 05:51
2009/12/22 Takahiro Itagaki <itagaki.takahiro(a)oss.ntt.co.jp>:
>
> Cedric Villemain <cedric.villemain(a)dalibo.com> wrote:
>
>> Le vendredi 18 decembre 2009 09:44:40, Takahiro Itagaki a ecrit :
>> > I'd like to add per-query buffer usage into contrib/pg_stat_statements.
>
> Here is a patch to add buffer usage columns into pg_stat_statements.
> It also changes initialzation of the result TupleDesc from manually
> coded routines to get_call_result_type().
>
>> Perhaps it can be usefull to have the percentage for hit/read ratio computed
>> in the view ?
>
> I think different DBAs want different derived values; Some of them might want
> the buffer hit ratio, but others might request numbers per query. I'd like to
> privide only raw values from pg_stat_statements to keep it simple, but I added
> a computational expression of hit percentage in the documentation.

Yes, you are right.

>
> ! bench=# SELECT query, calls, total_time, rows, 100.0 * shared_blks_hit /
> !                nullif(shared_blks_hit + shared_blks_read, 0) AS hit_percent
> !           FROM pg_stat_statements ORDER BY total_time DESC LIMIT 5;
> ! -[ RECORD 1 ]---------------------------------------------------------------------
> ! query       | UPDATE pgbench_branches SET bbalance = bbalance + $1 WHERE bid = $2;
> ! calls       | 3000
> ! total_time  | 9.60900100000002
> ! rows        | 2836
> ! hit_percent | 99.9778970000200936
>
> Regards,
> ---
> Takahiro Itagaki
> NTT Open Source Software Center
>
>
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>
>

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

From: Robert Haas on 29 Dec 2009 23:26
On Tue, Dec 22, 2009 at 3:27 AM, Takahiro Itagaki
<itagaki.takahiro(a)oss.ntt.co.jp> wrote:
> Cedric Villemain <cedric.villemain(a)dalibo.com> wrote:
>> Le vendredi 18 decembre 2009 09:44:40, Takahiro Itagaki a ecrit :
>> > I'd like to add per-query buffer usage into contrib/pg_stat_statements.
>
> Here is a patch to add buffer usage columns into pg_stat_statements.
> It also changes initialzation of the result TupleDesc from manually
> coded routines to get_call_result_type().

I have reviewed this patch and I think it looks pretty good. A couple
of minor nits:

- There are needless whitespace changes in the definition of struct
Counters. The changes to the existing four members should be
reverted, and the new members should be made to match the existing
members.

- In the part that reads /* calc differences of buffer counters */,
all the lines go past 80 columns. I wonder if it would be better to
insert a line break just after the equals sign and indent the next
line by an extra tab stop. See, e.g. src/backend/commands/user.c line
338.

Other than that I think this is ready to commit.

....Robert

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

From: Takahiro Itagaki on 3 Jan 2010 22:17

Robert Haas <robertmhaas(a)gmail.com> wrote:

> I have reviewed this patch and I think it looks pretty good. A couple
> of minor nits:
>
> - There are needless whitespace changes in the definition of struct
> Counters. The changes to the existing four members should be
> reverted, and the new members should be made to match the existing
> members.

That's because the 'shared_blks_written' field is too long to keep the
existing indentations. Since we still have some rooms in 80 columns,
I'd like to change all of them as the previous patch.

> - In the part that reads /* calc differences of buffer counters */,
> all the lines go past 80 columns. I wonder if it would be better to
> insert a line break just after the equals sign and indent the next
> line by an extra tab stop. See, e.g. src/backend/commands/user.c line
> 338.

Ok, I'll adjust them so.

Regards,
---
Takahiro Itagaki
NTT Open Source Software Center



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

 |  Next  |  Last
Pages: 1 2
Prev: PATCH: Add hstore_to_json()
Next: Time to run initdb is mostly figure-out-the-timezone work