Apologies for the delayed response - vacation, travel, etc got in the way!
Zdenek Kotala wrote:
> I performed review of merged patch from Robert Treat. At first point
> the patch does not work (SunOS 5.11 snv_86 sun4u sparc
The attached patch fixed the regression test errors.
> However I went through a code and I have following comments:
> 1) Naming convention:
> - Some probes uses "*end", some "*done". It would be good to select
> one name.
Noted. Will use <name>-done per the convention. This change will be
included in an updated patch later since I think there are a number of
other changes that need to be made.
> - I prefer to use clog instead of slru in probes name. clog is widely
I think slru- is okay per your subsequent emails.
> - It seems to me better to have checkpoint-clog...,
> checkpoint-subtrans instead of clog-checkpoint.
Yes, I was thinking about this too, but the current names are more
consistent with the others. For example:
> - buffer-flush was originally dirty-buffer-write-start. I prefer
> Robert Lor's naming.
Actually, I made this change so the name is consistent with the other
> 2) storage read write probes
> smgr-read*, smgr-writes probes are in md.c. I personally think it make
> more sense to put them into smgr.c. Only advantage to have it in md.c
> is that actual amount of bytes is possible to monitor.
The current probes return number of bytes, that's why they are in md.c
> 3) query rewrite probe
> There are several probes for query measurement but query rewrite phase
> is missing. See analyze_and_rewrite or pg_parse_and_rewrite
I believe the rewrite time is accounted for in the query-plan probe.
Need to double check on this.
> 4) autovacuum_start
> Autovacuum_start probe is alone. I propose following probes for
Saw a number of emails on this. Will get back on this.
> 5) Need explain of usage:
> I have some doubts about following probes. Could you please explain
> usage of them? example dtrace script is welcome
> - all exec-* probes
> - mark-dirty, local-mark-dirty
Theo/Robert, do you guys have any sample scripts on the probes you added.
> 6) several comments about placement:
> I published patch on http://reviewdemo.postgresql.org/r/25/. I added
> several comments there.
> 7) SLRU/CLOG
> SLRU probes could be return more info. For example if page was in
> buffer or if physical write is not necessary and so on.
Yes, more info could be returned if we can identify specific use cases
that the new data will enable.
Robert Lor Sun Microsystems
Austin, USA http://sun.com/postgresql
In response to
pgsql-hackers by date
|Next:||From: Josh Berkus||Date: 2008-07-21 19:53:05|
|Subject: Re: Do we really want to migrate plproxy and citext into
PG core distribution?|
|Previous:||From: Tom Lane||Date: 2008-07-21 19:43:00|
|Subject: Do we really want to migrate plproxy and citext into PG core distribution?|