Thread: Proposed changes to DTrace probe implementation

Proposed changes to DTrace probe implementation

From
Robert Lor
Date:
Motivation:
To enable probes to work with Mac OS X Leopard and other OSes that will
support DTrace in the future. Bug filed on this issue
http://archives.postgresql.org/pgsql-bugs/2007-10/msg00201.php

The problem:
There are two different mechanisms to implementing application level
probes in DTrace as explained here
http://blogs.sun.com/ahl/entry/user_land_tracing_gets_better. I
originally chose the first mechanism (using DTRACE_PROBEn macros) for
simplicity, but Leopard's DTrace implementation only supports the second
mechanism (using macros from dtrace generated header file). The second
mechanism was introduced as an enhancement, and one of the benefits is
type checking.

Proposed changes:
* Switch from using DTRACE_PROBEn macros to the dynamically generated
macros (remove pg_trace.h)
* Use "dtrace -h" to create a header file that contains the dynamically
generated macros to be used in the source code instead of the
DTRACE_PROBEn macros.
* Create a new header file called probes_null.h to map the generated
macros to no-ops
* This is unrelated to making DTrace work on Leopard, but I'd like to
propose that we split the probes into generic database and Postgres
specific providers, called "database" and "postgresql" respectively.
Other databases will be adding DTrace probes as well, and we want to use
"database" as the provider for probes that are common to all databases.
This way scripts that use the common provider will work on databases
that implement the common probes.

With the proposed changes, the steps for adding new probes are slightly
different, but the ways the probes are used will not change.

Steps for adding new probes now:
1) Add probe name to probes.d
2) Add probe to source code using one of PG_TRACEn macros
3) Recompile

Steps for adding probes with proposed changes:
1) Add probe name to probes.d (probes.h will be created at build time)
2) Add null macros to probes_null.h
3) Add probe to source code using the dynamically generated macro from
probes.h
4) Recompile

Preliminary patch is attached. There is one known issue with the patch.
When compiling outside of the source tree, probes.d needs to be
symlinked to the source tree. I haven't figured out how to copy the file
to the build tree yet.  I'm sure this is trivial for you gurus out there!

Comments?

Regards,
-Robert
? src/include/probes_null.h
Index: src/Makefile
===================================================================
RCS file: /projects/cvsroot/pgsql/src/Makefile,v
retrieving revision 1.42
diff -r1.42 Makefile
16a17,19
> ifeq ($(enable_dtrace), yes)
>     $(DTRACE) -h -s $(top_builddir)/src/backend/utils/probes.d -o $(top_builddir)/src/include/probes.h
> endif
Index: src/backend/Makefile
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/Makefile,v
retrieving revision 1.125
diff -r1.125 Makefile
22a23
> ifeq ($(PORTNAME), solaris)
25a27
> endif
143a146
> ifeq ($(PORTNAME), solaris)
145a149
> endif
Index: src/backend/access/transam/xact.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/access/transam/xact.c,v
retrieving revision 1.257
diff -r1.257 xact.c
1482c1482
<     PG_TRACE1(transaction__start, vxid.localTransactionId);
---
>     DATABASE_TRANSACTION_START(vxid.localTransactionId);
1607c1607
<     PG_TRACE1(transaction__commit, MyProc->lxid);
---
>     DATABASE_TRANSACTION_COMMIT(MyProc->lxid);
1993c1993
<     PG_TRACE1(transaction__abort, MyProc->lxid);
---
>     DATABASE_TRANSACTION_ABORT(MyProc->lxid);
Index: src/backend/storage/lmgr/lock.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/storage/lmgr/lock.c,v
retrieving revision 1.181
diff -r1.181 lock.c
790c790
<         PG_TRACE2(lock__startwait, locktag->locktag_field2, lockmode);
---
>         POSTGRESQL_LOCK_STARTWAIT(locktag->locktag_field2, lockmode);
794c794
<         PG_TRACE2(lock__endwait, locktag->locktag_field2, lockmode);
---
>         POSTGRESQL_LOCK_ENDWAIT(locktag->locktag_field2, lockmode);
Index: src/backend/storage/lmgr/lwlock.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/storage/lmgr/lwlock.c,v
retrieving revision 1.50
diff -r1.50 lwlock.c
450c450
<         PG_TRACE2(lwlock__startwait, lockid, mode);
---
>         POSTGRESQL_LWLOCK_STARTWAIT(lockid, mode);
461c461
<         PG_TRACE2(lwlock__endwait, lockid, mode);
---
>         POSTGRESQL_LWLOCK_ENDWAIT(lockid, mode);
472c472
<     PG_TRACE2(lwlock__acquire, lockid, mode);
---
>     POSTGRESQL_LWLOCK_ACQUIRE(lockid, mode);
543c543
<         PG_TRACE2(lwlock__condacquire__fail, lockid, mode);
---
>         POSTGRESQL_LWLOCK_CONDACQUIRE_FAIL(lockid, mode);
549c549
<         PG_TRACE2(lwlock__condacquire, lockid, mode);
---
>         POSTGRESQL_LWLOCK_CONDACQUIRE(lockid, mode);
634c634
<     PG_TRACE1(lwlock__release, lockid);
---
>     POSTGRESQL_LWLOCK_RELEASE(lockid);
Index: src/backend/utils/probes.d
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/utils/probes.d,v
retrieving revision 1.2
diff -r1.2 probes.d
10c10
< provider postgresql {
---
> provider database {
14a15,19
>
> };
>
> provider postgresql {
>
Index: src/include/c.h
===================================================================
RCS file: /projects/cvsroot/pgsql/src/include/c.h,v
retrieving revision 1.222
diff -r1.222 c.h
60c60,64
< #include "pg_trace.h"
---
> #ifdef ENABLE_DTRACE
> #include "probes.h"
> #else
> #include "probes_null.h"
> #endif
#ifndef    _PROBES_NULL_H
#define    _PROBES_NULL_H

#define    POSTGRESQL_LOCK_ENDWAIT(arg0, arg1)
#define    POSTGRESQL_LOCK_ENDWAIT_ENABLED()

#define    POSTGRESQL_LOCK_STARTWAIT(arg0, arg1)
#define    POSTGRESQL_LOCK_STARTWAIT_ENABLED()

#define    POSTGRESQL_LWLOCK_ACQUIRE(arg0, arg1)
#define    POSTGRESQL_LWLOCK_ACQUIRE_ENABLED()

#define    POSTGRESQL_LWLOCK_CONDACQUIRE(arg0, arg1)
#define    POSTGRESQL_LWLOCK_CONDACQUIRE_ENABLED()

#define    POSTGRESQL_LWLOCK_CONDACQUIRE_FAIL(arg0, arg1)
#define    POSTGRESQL_LWLOCK_CONDACQUIRE_FAIL_ENABLED()

#define    POSTGRESQL_LWLOCK_ENDWAIT(arg0, arg1)
#define    POSTGRESQL_LWLOCK_ENDWAIT_ENABLED()

#define    POSTGRESQL_LWLOCK_RELEASE(arg0)
#define    POSTGRESQL_LWLOCK_RELEASE_ENABLED()

#define    POSTGRESQL_LWLOCK_STARTWAIT(arg0, arg1)
#define    POSTGRESQL_LWLOCK_STARTWAIT_ENABLED()

#define    DATABASE_TRANSACTION_ABORT(arg0)
#define    DATABASE_TRANSACTION_ABORT_ENABLED()

#define    DATABASE_TRANSACTION_COMMIT(arg0)
#define    DATABASE_TRANSACTION_COMMIT_ENABLED()

#define    DATABASE_TRANSACTION_START(arg0)
#define    DATABASE_TRANSACTION_START_ENABLED()

#endif    /* _PROBES_NULL_H */

Re: Proposed changes to DTrace probe implementation

From
Peter Eisentraut
Date:
Am Freitag, 22. Februar 2008 schrieb Robert Lor:
> Preliminary patch is attached.

Could you please use diff -c or -u for the patch?  It's quite hard to read 
this way.

-- 
Peter Eisentraut
http://developer.postgresql.org/~petere/


Re: Proposed changes to DTrace probe implementation

From
Robert Lor
Date:
Peter,

Peter Eisentraut <peter_e@gmx.net> wrote:
> Could you please use diff -c or -u for the patch?  It's quite hard to 
> read this way.

Attached is the patch with diff -c.

Thanks,
-Robert


Re: Proposed changes to DTrace probe implementation

From
Robert Lor
Date:
Robert Lor <Robert.Lor@Sun.COM> wrote:

> Peter,
>
> Peter Eisentraut <peter_e@gmx.net> wrote:
> > Could you please use diff -c or -u for the patch?  It's quite hard
> to
> > read this way.
>
> Attached is the patch with diff -c.
>

Oops, here's the real attachment!

Regards,
-Robert

Attachment

Re: Proposed changes to DTrace probe implementation

From
Robert Lor
Date:
Robert Lor wrote:
> Proposed changes:
> * Switch from using DTRACE_PROBEn macros to the dynamically generated 
> macros (remove pg_trace.h)
> * Use "dtrace -h" to create a header file that contains the 
> dynamically generated macros to be used in the source code instead of 
> the DTRACE_PROBEn macros.
> * Create a new header file called probes_null.h to map the generated 
> macros to no-ops
> * This is unrelated to making DTrace work on Leopard, but I'd like to 
> propose that we split the probes into generic database and Postgres 
> specific providers, called "database" and "postgresql" respectively. 
> Other databases will be adding DTrace probes as well, and we want to 
> use "database" as the provider for probes that are common to all 
> databases. This way scripts that use the common provider will work on 
> databases that implement the common probes.
>
Since I haven't heard any objections, I assume the proposed changes are 
acceptable. I'll go a head and submit the patch.

Regards,
-Robert


Re: Proposed changes to DTrace probe implementation

From
Tom Lane
Date:
Robert Lor <Robert.Lor@Sun.COM> writes:
>> * This is unrelated to making DTrace work on Leopard, but I'd like to 
>> propose that we split the probes into generic database and Postgres 
>> specific providers, called "database" and "postgresql" respectively. 
>> Other databases will be adding DTrace probes as well, and we want to 
>> use "database" as the provider for probes that are common to all 
>> databases.

I'm unconvinced that there will be any probes that are common to all
databases.  I'd skip this part...
        regards, tom lane


Re: Proposed changes to DTrace probe implementation

From
Robert Lor
Date:
Tom Lane wrote:
> I'm unconvinced that there will be any probes that are common to all
> databases.  I'd skip this part...
>
>   
Any reason why we can't consider probes like transaction-start, 
transaction-commit, or transaction-abort as common probes that can also 
be used in other (maybe no all) databases?  We are only talking about 
the probe definition here as shown below, not how they will be implemented.

probe transaction__start(int);
probe transaction__commit(int);
probe transaction__abort(int);


Regards,
-Robert




Re: Proposed changes to DTrace probe implementation

From
Tom Lane
Date:
Robert Lor <Robert.Lor@Sun.COM> writes:
> Tom Lane wrote:
>> I'm unconvinced that there will be any probes that are common to all
>> databases.  I'd skip this part...
>> 
> Any reason why we can't consider probes like transaction-start, 
> transaction-commit, or transaction-abort as common probes that can also 
> be used in other (maybe no all) databases?

I'm unimpressed; it's not at all clear that you'd be measuring quite the
same thing in, say, mysql as in postgres.

Possibly I have a different view of the uses of dtrace than you do, but
most of the events I'd be interested in probing are probably pretty
Postgres-specific.  I think distinguishing half a dozen of them on the
assumption that there should be (exact) matches to that probe point in
most databases is misleading and a source of useless extra notation.
        regards, tom lane


Re: Proposed changes to DTrace probe implementation

From
Robert Lor
Date:
Tom Lane wrote:
> I'm unimpressed; it's not at all clear that you'd be measuring quite the
> same thing in, say, mysql as in postgres.
>   
I think it depends on the probe, but for transaction rates like start or 
commit, don't you think it's pretty black & white as long as the probes 
are placed in the correct locations.
> Possibly I have a different view of the uses of dtrace than you do, but
> most of the events I'd be interested in probing are probably pretty
> Postgres-specific.
I agree that most probes, especially low level ones, will be Postgres 
specific.
>   I think distinguishing half a dozen of them on the
> assumption that there should be (exact) matches to that probe point in
> most databases is misleading and a source of useless extra notation.
>
>   
This is just forward looking on my part since other OS databases will 
most likely be adding Dtrace probes as well.  I'll put them back 
together in one provider for now!

Regards,
-Robert



Re: Proposed changes to DTrace probe implementation

From
Gregory Stark
Date:
"Tom Lane" <tgl@sss.pgh.pa.us> writes:

> Possibly I have a different view of the uses of dtrace than you do, but
> most of the events I'd be interested in probing are probably pretty
> Postgres-specific.  

I think both types of probes are useful to different people.

One of the really neat things about dtrace, imho, is that it lets you
correlate data from different levels of abstraction. You can find out how many
physical i/o's happen per i/o syscall. And how many i/o syscalls per database
transaction. And how many database transactions per application request. Etc.

Perhaps looking at the standard database SNMP MIB counters would give us a
place to start for upward facing events people want to trace for databases in
general.

--  Gregory Stark EnterpriseDB          http://www.enterprisedb.com Ask me about EnterpriseDB's On-Demand Production
Tuning


Re: Proposed changes to DTrace probe implementation

From
Robert Lor
Date:
Gregory Stark wrote:
> I think both types of probes are useful to different people.
>   
I think certain higher level probes can be really useful to DBAs.
> Perhaps looking at the standard database SNMP MIB counters would give us a
> place to start for upward facing events people want to trace for databases in
> general.
>   
Great idea. I found this link for public RDBMS MIB 
http://www.mnlab.cs.depaul.edu/cgi-bin/sbrowser.cgi?HOST=&OID=RDBMS-MIB!rdbmsMIB

The stats in rdbmsSrvInfoTable is quite useful, and it's one of the 
tables that Oracle implements in their SNMP support. 
http://download-east.oracle.com/docs/cd/B14099_19/manage.1012/b16244/appdx_d_rdbms.htm

Regards,
-Robert



Re: Proposed changes to DTrace probe implementation

From
Tom Lane
Date:
Robert Lor <Robert.Lor@Sun.COM> writes:
> Tom Lane wrote:
>> I'm unimpressed; it's not at all clear that you'd be measuring quite the
>> same thing in, say, mysql as in postgres.

> I think it depends on the probe, but for transaction rates like start or 
> commit, don't you think it's pretty black & white as long as the probes 
> are placed in the correct locations.

I'm not so sure.  For instance, from what I understand about mysql
(admittedly not a lot) their notion of transaction is rather
storage-engine-specific.  It wouldn't be hard to come up with situations
where they show many more or fewer "transactions" than we do.

The concern I've got about this is basically that it would encourage
plastering the same label on subtly different counts, leading to
confusion and perhaps mistaken conclusions.  I would prefer to see any
common probes be reverse-engineered *after the fact*, ie, after you've
already instrumented several DB's you're in a better position to figure
out what's common and what's not.  I distrust preconceived notions about
that.
        regards, tom lane


Re: Proposed changes to DTrace probe implementation

From
Robert Lor
Date:
Tom Lane wrote:
> The concern I've got about this is basically that it would encourage
> plastering the same label on subtly different counts, leading to
> confusion and perhaps mistaken conclusions.  I would prefer to see any
> common probes be reverse-engineered *after the fact*, ie, after you've
> already instrumented several DB's you're in a better position to figure
> out what's common and what's not.  I distrust preconceived notions about
> that.
>
>   
Your point is well taken, and we can revisit this later!

Regards,
-Robert



Re: Proposed changes to DTrace probe implementation

From
Magnus Hagander
Date:
On Tue, Feb 26, 2008 at 03:48:28PM -0600, Robert Lor wrote:
> Gregory Stark wrote:
> >I think both types of probes are useful to different people.
> >  
> I think certain higher level probes can be really useful to DBAs.
> >Perhaps looking at the standard database SNMP MIB counters would give us a
> >place to start for upward facing events people want to trace for databases 
> >in
> >general.
> >  
> Great idea. I found this link for public RDBMS MIB 
> http://www.mnlab.cs.depaul.edu/cgi-bin/sbrowser.cgi?HOST=&OID=RDBMS-MIB!rdbmsMIB
> 
> The stats in rdbmsSrvInfoTable is quite useful, and it's one of the 
> tables that Oracle implements in their SNMP support. 
> http://download-east.oracle.com/docs/cd/B14099_19/manage.1012/b16244/appdx_d_rdbms.htm

Incidentally, most of that's already supported by the pg snmp provider,
through the stats system.

//Magnus


Re: Proposed changes to DTrace probe implementation

From
Paul van den Bogaard
Date:
but putting these and other counters in context is what could be  
missing. Correlating a given (set of) stats with others (possible  
outside of the application domain) is one of the assets offered by  
DTrace.  Besides the generic transaction begin/start/end it could  
also be helpful to see the number of parses,binds,executes per  
transaction, user, connection etc.
And yes, I feel Tom is right in fearing that these things can be used  
in "creative" ways. However is this not true for most benchmarks/ 
results when ones objective is  to "show" how perfect/better/whatever  
product/platform A behaves/is compared to B, C, etc...

One benefit for generalizing a subset of the DTrace probes is the  
possibility of creating generic DTrace scripts that can be used for  
many database installations. DTrace is great, programming DTrace  
scripts is fun (my view, and sure I am biased as a Sun employee :-),  
but it is not likely to be something a dba would like to master. The  
availability of "generic" scripts does add value.


BTW I wonder if we could somehow combine DTrace as a contextual tool  
with the counters provided through the stats interface. Any insight/ 
ideas?

--Paul.






On 27-feb-2008, at 10:28, Magnus Hagander wrote:

> On Tue, Feb 26, 2008 at 03:48:28PM -0600, Robert Lor wrote:
>> Gregory Stark wrote:
>>> I think both types of probes are useful to different people.
>>>
>> I think certain higher level probes can be really useful to DBAs.
>>> Perhaps looking at the standard database SNMP MIB counters would  
>>> give us a
>>> place to start for upward facing events people want to trace for  
>>> databases
>>> in
>>> general.
>>>
>> Great idea. I found this link for public RDBMS MIB
>> http://www.mnlab.cs.depaul.edu/cgi-bin/sbrowser.cgi? 
>> HOST=&OID=RDBMS-MIB!rdbmsMIB
>>
>> The stats in rdbmsSrvInfoTable is quite useful, and it's one of the
>> tables that Oracle implements in their SNMP support.
>> http://download-east.oracle.com/docs/cd/B14099_19/manage.1012/ 
>> b16244/appdx_d_rdbms.htm
>
> Incidentally, most of that's already supported by the pg snmp  
> provider,
> through the stats system.
>
> //Magnus
>
> ---------------------------(end of  
> broadcast)---------------------------
> TIP 3: Have you checked our extensive FAQ?
>
>                http://www.postgresql.org/docs/faq

------------------------------------------------------------------------ 
---------------------
Paul van den Bogaard                                
Paul.vandenBogaard@sun.com
ISV-E  -- ISV Engineering, Opensource Engineering group

Sun Microsystems, Inc                              phone:        +31  
334 515 918
Saturnus 1                                                  
extentsion: x (70)15918
3824 ME Amersfoort                                 mobile:       +31  
651 913 354
The Netherlands                                         
fax:            +31 334 515 001