I performed review of merged patch from Robert Treat. At first point the patch
does not work (SunOS 5.11 snv_86 sun4u sparc SUNW,Sun-Fire-V240)
truncate ... ok alter_table ... FAILED (test process exited with exit code 2)
sequence ... ok polymorphism ... ok rowtypes ... ok returning ...
ok largeobject ... FAILED (test process exited with exit code 2) xml ... ok
test stats ... FAILED (test process exited with exit code 2)
test tablespace ... FAILED (test process exited with exit code 2)
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. - I prefer to use clog instead of slru
inprobes name. clog is widely known. - It seems to me better to have checkpoint-clog..., checkpoint-subtrans
instead of clog-checkpoint. - buffer-flush was originally dirty-buffer-write-start. I prefer Robert Lor's
naming.
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.
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
4) autovacuum_start
Autovacuum_start probe is alone. I propose following probes for completeness:
proc-autovacuum-start
proc-autovacuum-stop
proc-bgwriter-start
proc-bgwriter-stop
proc-backend-start
proc-backend-stop
proc-master-start
proc-master-stop
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
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.
That's all for this moment
Zdenek
--
Zdenek Kotala Sun Microsystems
Prague, Czech Republic http://sun.com/postgresql
