Some belated patch review for "Buffers" explain analyze patch

From: Greg Stark <stark(at)mit(dot)edu>
To: "<pgsql-hackers(at)postgresql(dot)org>" <pgsql-hackers(at)postgresql(dot)org>
Subject: Some belated patch review for "Buffers" explain analyze patch
Date: 2010-02-09 19:50:19
Message-ID: 407d949e1002091150j49ce8458u4704e53bed21e91d@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

I was recently experimenting with explain analyze and I realized there
are two things arguably wrong with the "Buffers" output in explain
analyze:

Firstly, it's printing out a number of buffers. We spent a lot of
effort making all GUC variables use units of memory like "kB" and "MB"
so the user should never have to be doing arithmetic in units of
buffers. I think these values should be printed in similar units.
Heikki did express concern that sometimes users want to know iops but
there's no real way for us to report iops since even random reads
might not really be random and they might be from cache. Whereas the
amount of memory is always a meaningful number.

I already have a patch to do this but it's a bit grotty -- do we want
to have a generic format string in snprintf in case we need it
somewhere else other than explain.c?

Secondly, I think it's printing the total buffer usage for that node
across the whole query -- not the average per iteration. I agree that
the average is probably more confusing but it's what we do for every
other stat. Do we want to be consistent? Call out the inconsistency
somehow, perhaps by tagging it "Total Buffer Usage:" or something like
that?

--
greg

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Heikki Linnakangas 2010-02-09 20:03:41 Re: CreateFakeRelcacheEntry versus Hot Standby
Previous Message Heikki Linnakangas 2010-02-09 19:38:07 Re: Why doesn't smgr_redo truncate the VM fork too?