From: | Robert Haas <robertmhaas(at)gmail(dot)com> |
---|---|
To: | Itagaki Takahiro <itagaki(dot)takahiro(at)oss(dot)ntt(dot)co(dot)jp> |
Cc: | Euler Taveira de Oliveira <euler(at)timbira(dot)com>, Pgsql Hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Buffer usage in EXPLAIN and pg_stat_statements (review) |
Date: | 2009-10-05 03:36:20 |
Message-ID: | 603c8f070910042036w1bede487wf7ee31b6c05e538d@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Sun, Oct 4, 2009 at 11:22 PM, Itagaki Takahiro
<itagaki(dot)takahiro(at)oss(dot)ntt(dot)co(dot)jp> wrote:
> Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>
>> 1. 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.
>
> I have some questions:
> * Did you use single space and double spaces in your example intentionally?
No, that was unintentional.
> * Should we use lower cases here?
No. We don't anywhere else in explain.c.
> * Can I use "temp" instead of "Temp Read" to shorten the name?
I can't tell what that means without reading the source code. I think
clarity should take precedence over brevity.
>> 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.
>
> I agree to rename them into blks_*, but EXPLAIN (blocks) might be
> misleading; EXPLAIN (buffer) can be interpreted as "buffer usage",
> but normally we don't call it "block usage".
>
> My suggestion is:
> * EXPLAIN (buffers) prints (blocks read: %ld hit: %ld temp: %ld)
> * auto_explain.log_buffers are not changed
> * pg_stat_statements uses blks_hit and blks_read
I agree.
>> 4. "Instrumentation stack is broken" doesn't seem terribly helpful in
>> understanding what has gone wrong.
>
> This message is only for hackers and should not occur.
> Assert() might be ok instead.
Hmm, I think I like the idea of an Assert(). Logging a cryptic
message at DEBUG2 doesn't seem sufficient for a can't-happen condition
that probably indicates a serious bug in the code.
...Robert
From | Date | Subject | |
---|---|---|---|
Next Message | Itagaki Takahiro | 2009-10-05 05:24:14 | Re: Buffer usage in EXPLAIN and pg_stat_statements (review) |
Previous Message | Itagaki Takahiro | 2009-10-05 03:22:15 | Re: Buffer usage in EXPLAIN and pg_stat_statements (review) |