On Wed, Sep 30, 2009 at 10:40 PM, Itagaki Takahiro <itagaki.takah...@oss.ntt.co.jp> wrote: > > Euler Taveira de Oliveira <eu...@timbira.com> wrote: > >> > But there are some confusions in postgres; ReadBufferCount and >> > BufferHitCount are used for "get" and "hit", but "heap_blks_read" >> > and "heap_blks_hit" are used for "read" and "hit" in pg_statio_all_tables. >> I see. :( > > I fixed the confusions of get, hit and read in your patch. > long num_hit = ReadBufferCount + ReadLocalBufferCount; > long num_read = num_hit - BufferHitCount - LocalBufferHitCount; > should be > long num_get = ReadBufferCount + ReadLocalBufferCount; > long num_hit = BufferHitCount + LocalBufferHitCount; > long num_read = num_get - num_hit; > > ReadBufferCount means "number of buffer access" :( > > Patch attached.
I took a look at this today and I have a couple of comments. The basic functionality looks useful, but I think the terminology is too terse. Specific commens: 1. In the EXPLAIN output, I think that the buffers information should be output on its own line, rather than appended to the line that already contains costs and execution times. The current output doesn't include the word "buffers" or "blocks" anywhere, which seems to me to be a critical flaw. I would suggest something like "Blocks Read: %ld Hit: %ld Temp Read: %ld\n". See the way we handle output of sort type and space usage, for example. 2. Similarly, in pg_stat_statements, the Counters structure could easily use the same names for the structure members that we already use in e.g. pg_stat_database - blks_hit, blks_read, and, say, blks_temp_read. In fact I tend to think we should stick with "blocks" rather than "buffers" overall, for consistency with what the system does elsewhere. 3. With respect to the doc changes in explain.sgml, we consistently use "disk" rather than "disc" in the documentation; but it may not be necessary to use that word at all, and I think the paragraph can be tightened up a bit: "Include information on the number of blocks read, the number of those that are hits (already in shared buffers and do not need to be read in), and the number of those that are reads on temporary, backend-local buffers. This parameter requires that the <literal>ANALYZE</literal> parameter also be used. This parameter defaults to <literal>FALSE</literal>". 4. "Instrumentation stack is broken" doesn't seem terribly helpful in understanding what has gone wrong. ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
