Thread: pgsql: Add stats for min, max, mean, stddev times to pg_stat_statements

pgsql: Add stats for min, max, mean, stddev times to pg_stat_statements

From
Andrew Dunstan
Date:
Add stats for min, max, mean, stddev times to pg_stat_statements.

The new fields are min_time, max_time, mean_time and stddev_time.

Based on an original patch from Mitsumasa KONDO, modified by me. Reviewed by Petr Jelínek.

Branch
------
master

Details
-------
http://git.postgresql.org/pg/commitdiff/717f709532642f5f7756785c9be17b7ffcec8ae8

Modified Files
--------------
contrib/pg_stat_statements/Makefile                |    5 +-
.../pg_stat_statements--1.2--1.3.sql               |   47 +++++++++++
.../pg_stat_statements/pg_stat_statements--1.2.sql |   44 ----------
.../pg_stat_statements/pg_stat_statements--1.3.sql |   48 +++++++++++
contrib/pg_stat_statements/pg_stat_statements.c    |   87 +++++++++++++++++++-
.../pg_stat_statements/pg_stat_statements.control  |    2 +-
doc/src/sgml/pgstatstatements.sgml                 |   28 +++++++
7 files changed, 212 insertions(+), 49 deletions(-)


Andrew Dunstan <andrew@dunslane.net> writes:
> Add stats for min, max, mean, stddev times to pg_stat_statements.

The buildfarm is quite unhappy with this patch.  Kinda looks like
it was never tested on 32-bit machines.

            regards, tom lane


Re: pgsql: Add stats for min, max, mean, stddev times to pg_stat_statements

From
Andrew Dunstan
Date:
On 03/27/2015 05:53 PM, Tom Lane wrote:
> Andrew Dunstan <andrew@dunslane.net> writes:
>> Add stats for min, max, mean, stddev times to pg_stat_statements.
> The buildfarm is quite unhappy with this patch.  Kinda looks like
> it was never tested on 32-bit machines.
>
>


I have committed a fix which is turning the buildfarm back to green.

However, it is moaning about the code in the sqrtd() function. I'm
wondering if we shouldn't just rip that out and use the library sqrt()
function. It's not called for every statement processed, only each time
the function is called (for each row).

cheers

andrew


Re: pgsql: Add stats for min, max, mean, stddev times to pg_stat_statements

From
Petr Jelinek
Date:
On 27/03/15 23:26, Andrew Dunstan wrote:
>
> On 03/27/2015 05:53 PM, Tom Lane wrote:
>> Andrew Dunstan <andrew@dunslane.net> writes:
>>> Add stats for min, max, mean, stddev times to pg_stat_statements.
>> The buildfarm is quite unhappy with this patch.  Kinda looks like
>> it was never tested on 32-bit machines.
>>
>>
>
>
> I have committed a fix which is turning the buildfarm back to green.
>
> However, it is moaning about the code in the sqrtd() function. I'm
> wondering if we shouldn't just rip that out and use the library sqrt()
> function. It's not called for every statement processed, only each time
> the function is called (for each row).
>

Or you could change the long long int to int64.

But in general I wouldn't be too much against ripping it off, it's only
a bit of CPU when reading the stats...

--
  Petr Jelinek                  http://www.2ndQuadrant.com/
  PostgreSQL Development, 24x7 Support, Training & Services


Andrew Dunstan <andrew@dunslane.net> writes:
> However, it is moaning about the code in the sqrtd() function. I'm
> wondering if we shouldn't just rip that out and use the library sqrt()
> function. It's not called for every statement processed, only each time
> the function is called (for each row).

[ looks... ]  +1.  I'm skeptical that that's even a win at all on modern
hardware; sqrt() is a primitive operation on nearly anything these days.

Also, quite aside from the error of supposing that long long int is
the same size as double, I'm pretty sure this would fail miserably on
non-IEEE-float hardware; and it may well have endianness issues too.
Since the code isn't actually being executed on the buildfarm, only
compiled, we have no good way to tell whether it would produce sane
results everywhere.

            regards, tom lane


I wrote:
> Andrew Dunstan <andrew@dunslane.net> writes:
>> However, it is moaning about the code in the sqrtd() function. I'm
>> wondering if we shouldn't just rip that out and use the library sqrt()
>> function. It's not called for every statement processed, only each time
>> the function is called (for each row).

> [ looks... ]  +1.  I'm skeptical that that's even a win at all on modern
> hardware; sqrt() is a primitive operation on nearly anything these days.

I did some quick comparisons on a reasonably current server
(Xeon E5-2609 running RHEL6.6), and found that this:

    volatile double x;
    volatile double y = 1.23456;

    for (i = 0; i < n; i++)
    {
        x = sqrtd(y);
    }

takes about 5.6 nsec per iteration, while with plain sqrt() it's
about 8.8 nsec.  So while there is a measurable gain (on this hardware
anyway) IMO it is absolutely not worth taking any portability risks for.

            regards, tom lane