Review: DTrace probes (merged version)

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 in probes 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


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@[EMAIL PROTECTED]
)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers