Re: Explain analyze getrusage tracking

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Greg Stark <stark(at)mit(dot)edu>
Cc: "<pgsql-hackers(at)postgresql(dot)org>" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Explain analyze getrusage tracking
Date: 2010-11-16 02:43:05
Message-ID: AANLkTik960v3Cpok_GYSQXiRsy4uuQbN7XO32zfEwjP3@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Sun, Nov 14, 2010 at 9:33 PM, Greg Stark <stark(at)mit(dot)edu> wrote:
> This is an update to my earlier patch to add getrusage resource
> tracking to EXPLAIN ANALYZE.
>
> With this patch you get something like:
>
>                                                  QUERY PLAN
> --------------------------------------------------------------------------------------------------------------
>  Seq Scan on i  (cost=0.00..6919.44 rows=262144 width=101) (actual
> time=17.240..1123.751 rows=262144 loops=1)
>   Resources: sys=210.000ms user=430.000ms read=33.6MB
>   Buffers: shared read=4298
>  Total runtime: 1548.651 ms
> (4 rows)
>
> The main change is to make it work under Windows. At least I think the
> changes should make it work under Windows, I haven't been able to test
> it. Actually I'm not to happy with the way I did it, I would be more
> inclined to hack the getrusagestub,h definition of struct rusage to
> have an instr_time in it so that we can use the same macros directly.
> But that's more changes than I would be happy making without being
> able to compile them to test them.

I don't really think these changes to the INSTR macros make much
sense. The macros don't really add much notational convenience;
they're mostly wrappers to make the WIN32 and non-WIN32 cases work
similarly for the instrumentation stuff, so hacking them up to use
them for this doesn't seem like it adds anything. Just do whatever it
is you need to do, or define macros locally in explain.c.

It doesn't make much sense to me to normalize the memory for this
output to a variable unit when the other memory values we use in
explain.c are still going to be printed as kB. I think we should just
print it in kB and call it good. Alternatively, we could apply the
same normalization algorithm across the board, but I don't think
that's as good.

I continue to feel strongly that the choice of EXPLAIN format should
only affect the format, not the choice of information to be displayed.
Using the verbose option to control how much data the resource option
prints is, I think, not a good idea. If you want to have two modes,
one for partial rusage data and one for full rusage data, you can just
as easily implement EXPLAIN (RESOURCE [PARTIAL|FULL]). I believe that
the existing grammar is adequate to support that; you'd just need to
write the appropriate DefElem-parsing code. But personally I'd just
print the whole kit and kaboodle regardless.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Greg Smith 2010-11-16 02:49:08 Re: Count backend self-sync calls
Previous Message Robert Haas 2010-11-16 02:28:00 Re: unlogged tables