On Sun, Dec 13, 2009 at 7:55 PM, Takahiro Itagaki
<itagaki.takahiro@oss.ntt.co.jp> wrote:
>
> Robert Haas <robertmhaas@gmail.com> wrote:
>
>> OK, done, see attached. I also noticed when looking through this that
>> the documentation says that auto_explain.log_buffers is ignored unless
>> auto_explain.log_analyze is set. That is true and seems right to me,
>> but for some reason explain_ExecutorEnd() had been changed to set
>> es.analyze if either log_analyze or log_buffers was set.
>
> Thanks. It was my bug.
>
> Could you apply the patch? Or, may I do by myself?
Sorry, I've been meaning to look at this a little more and have gotten
distracted.
I have a question about the comment in InstrStopNode(), which reads:
"Adds delta of buffer usage to node's count and resets counter to
start so that the counters are not double counted by parent nodes."
It then calls BufferUsageAccumDiff(), but that function doesn't
actually "reset" anything, so it seems like the comment is wrong.
Two other thoughts:
1. It doesn't appear that there is any provision to ever zero
pgBufferUsage. Shouldn't we do this, say, once per explain, just to
avoid the possibility of overflowing the counters?
2. We seem to do all the work associated with pgBufferUsage even when
the "buffers" option is not passed to explain. The overhead of
incrementing the counters is probably negligible (and we were paying
the equivalent overhead before anyway) but I'm not sure whether saving
the starting counters and accumulating the deltas might be enough to
slow down EXPLAIN ANALYZE. That's sorta slow already so I'd hate to
whack it any more - have you benchmarked this at all?
...Robert
