Re: Explain analyze getrusage tracking

On Tue, Nov 16, 2010 at 2:43 AM, Robert Haas <robertmhaas@gmail.com> wrote:
> 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.

Well they're going to be just like the macros in instr_time and I
didn't really want to duplicate them. What's really goofy here is that
on Windows the getrusage portability wrapper is actually consing up
those times from FILETIMEs which I believe are actually just like the
instr_time data type that we're already using in those macros so it's
going of doing a silly dance converting to struct timeval just so it
can use macros that aren't convenient at all on Windows.

I definitely agree this is awkward. I would love to find a cleaner
solution here. I'll try copying the macros and see if that works out
more cleanly.

>
> 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 think we should have a project policy of always printing memory and
disk usage in kB, MB, GB etc unless they're functions returning an
integer intended for machine use. Effectively this is the dual of
accepting units on all our memory gucs. I don't know about others but
I find it pretty hard to read things like 1234567kB and compare it to
125765kB in my head.

> I continue to feel strongly that the choice of EXPLAIN format should
> only affect the format, not the choice of information to be displayed.

I don't have a good feeling for what should appear in
XML/JSON/whetever. I thought I was just implementing the consensus
which was that there wasn't much point in not including the maximum
level of detail because the consumer could decide what to display. But
having it be the same as the human readable output is also reasonable.
Whatever we're doing elsewhere I'll make this the consistent.

>  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.

This is a separate question. The numbers I'm outputing without VERBOSE
now are the numbers that I understand and which relate to the other
numbers in explain. My intent was that as we understand how the other
numbers relate to Postgres functioning we'll learn which ones to
include. But mostly they're hard to relate to Postgres and the query
execution in any way and just noise that are hard to explain to users.
I'm not even sure which ones it makes sense to sum over time -- is the
integral virtual memory usage over time something that it makes sense
to sum over time? What units is it in?

I don't see why VERBOSE isn't the right key to use. The other things
VERBOSE toggles are also extra detail that's usually useless but might
be useful if you're interested in the inner workings. Things like
which fields are being included at each level of the plan for example.

--
greg