Proposed changes to DTrace probe implementation - Mailing list pgsql-hackers

From Robert Lor
Subject Proposed changes to DTrace probe implementation
Date
Msg-id 47BF02B3.1070701@sun.com
Whole thread Raw
Responses Re: Proposed changes to DTrace probe implementation
Re: Proposed changes to DTrace probe implementation
List pgsql-hackers
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 */

pgsql-hackers by date:

Previous
From: "Greg Sabino Mullane"
Date:
Subject: Re: Including PL/PgSQL by default
Next
From: "Joshua D. Drake"
Date:
Subject: Re: 8.3 / 8.2.6 restore comparison