Skip site navigation (1) Skip section navigation (2)

Re: Review: DTrace probes (merged version)

From: Robert Lor <Robert(dot)Lor(at)Sun(dot)COM>
To: Zdenek Kotala <Zdenek(dot)Kotala(at)Sun(dot)COM>
Cc: PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>, Robert Treat <xzilla(at)users(dot)sourceforge(dot)net>, jesus(at)omniti(dot)com
Subject: Re: Review: DTrace probes (merged version)
Date: 2008-07-21 19:47:24
Message-ID: (view raw, whole thread or download thread mbox)
Lists: pgsql-hackers
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 
> SUNW,Sun-Fire-V240)

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 
> known.
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-checkpoint, buffer-*
xlog-checkpoint, xlog-*
>  - 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 
buffer-* probes.
> 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 
> 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
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 I added 
> several comments there.
> 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

Attachment: 8.4_merged_probes_v2.patch
Description: text/plain (45.1 KB)

In response to


pgsql-hackers by date

Next:From: Josh BerkusDate: 2008-07-21 19:53:05
Subject: Re: Do we really want to migrate plproxy and citext into PG core distribution?
Previous:From: Tom LaneDate: 2008-07-21 19:43:00
Subject: Do we really want to migrate plproxy and citext into PG core distribution?

Privacy Policy | About PostgreSQL
Copyright © 1996-2017 The PostgreSQL Global Development Group