Review: DTrace probes (merged version)

From: Zdenek Kotala <Zdenek(dot)Kotala(at)Sun(dot)COM>
To: PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>, Robert Lor <Robert(dot)Lor(at)Sun(dot)COM>, Robert Treat <xzilla(at)users(dot)sourceforge(dot)net>, jesus(at)omniti(dot)com
Subject: Review: DTrace probes (merged version)
Date: 2008-07-04 14:38:06
Message-ID: 486E35CE.4070309@sun.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Alvaro Herrera 2008-07-04 15:05:40 Re: Review: DTrace probes (merged version)
Previous Message Bernd Helmle 2008-07-04 13:18:23 Re: log_rotation_age integer overflow display quirk