Thread: Proposed changes to DTrace probe implementation
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 */
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/
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
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
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
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
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
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
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
"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
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
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
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
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
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