Generic Monitoring Framework with DTrace patch

Hi,

I've have attached a patch along with two new files.This patch should
reflect what we discussed at the Summit. Please let me know if I miss
anything.

There are a few potential/known issues which I need help from the gurus.

1) The current logic in src/backend/Makefile will only work for Solaris
versions with DTrace, and Peter has offered to help fix this one.

2) Currently an environment variable called DTRACE_DATA_MODEL is used in
src/backend/Makefile to tell the dtrace command whether to generate a 32
or 64 bit binary. This may not be a reliable approach since a user can
forget to set this variable. Perhaps adding a flag like DTRACEFLAGS to
the configure script is a better approach. Thoughts?

3) When using --enable-depend, "gmake clean" removes all *.d files,
including the DTrace probe definition file which also has a .d
extension. I tried to to use a different extension but the dtrace tool
didn't like that. Since we only plan to have one probe definition file
(pgsqlprobes.d) and one provider called "postgresql", I think
src/Makefile.global.in can be changed to not remove this file. Is this
acceptable or is there a better solution?

I have created several DTrace scripts and uploaded them to
http://pgfoundry.org/projects/dtrace/

Many thanks to Gavin and Tom for their help with creating and inserting
the current set of probes, and Peter for his help with the config files.

Regards,
-Robert

Robert Lor wrote:
> I've have attached a patch along with two new files.This patch should
> reflect what we discussed at the Summit. Please let me know if I miss
> anything.

I would prefer to drop the PG_ prefixes on PG_TRACE and pg_trace.h. We
know which software we're dealing with.

> 1) The current logic in src/backend/Makefile will only work for
> Solaris versions with DTrace, and Peter has offered to help fix this
> one.

We should probably move the probes file to a subdirectory. Anyone know
a good place?

Also, again, the pgsql prefix should be dropped.

> 2) Currently an environment variable called DTRACE_DATA_MODEL is used
> in src/backend/Makefile to tell the dtrace command whether to
> generate a 32 or 64 bit binary. This may not be a reliable approach
> since a user can forget to set this variable. Perhaps adding a flag
> like DTRACEFLAGS to the configure script is a better approach.

Certainly doable, but will that be more reliable? Can't we convince
dtrace to create binaries for the host platform by default?

> 3) When using --enable-depend, "gmake clean" removes all *.d files,

I'm working on renaming the dependency files.

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

On Fri, Jul 21, 2006 at 01:42:26PM +0200, Peter Eisentraut wrote:
> Robert Lor wrote:
> > I've have attached a patch along with two new files.This patch should
> > reflect what we discussed at the Summit. Please let me know if I miss
> > anything.
>
> I would prefer to drop the PG_ prefixes on PG_TRACE and pg_trace.h. We
> know which software we're dealing with.

I don't know. "trace" is a fairly generic word, how do you know that
none of the dozen other libraries we include don't already have a
"trace.h" or a TRACE() macro? On any of our supported platforms?

Debian already counts more than a dozen files called "trace.h". While
none are in libraries we're likely to use, this is just one platform.
If it were in a subdirectory (say utils/trace.h) that would be OK
too...

> > 1) The current logic in src/backend/Makefile will only work for
> > Solaris versions with DTrace, and Peter has offered to help fix this
> > one.
>
> We should probably move the probes file to a subdirectory. Anyone know
> a good place?
>
> Also, again, the pgsql prefix should be dropped.

The prefix here is redundant. We know which directory it's in.

Have a nice day,
--
Martijn van Oosterhout <kleptog(at)svana(dot)org> http://svana.org/kleptog/
> From each according to his ability. To each according to his ability to litigate.

Martijn van Oosterhout <kleptog(at)svana(dot)org> writes:
> On Fri, Jul 21, 2006 at 01:42:26PM +0200, Peter Eisentraut wrote:
>> I would prefer to drop the PG_ prefixes on PG_TRACE and pg_trace.h. We
>> know which software we're dealing with.

> I don't know. "trace" is a fairly generic word, how do you know that
> none of the dozen other libraries we include don't already have a
> "trace.h" or a TRACE() macro? On any of our supported platforms?

I concur with Martijn. We've already regretted using ERROR as a macro
name, let's not make the same mistake with TRACE. PG_TRACE is good,
and so is pg_trace.h. (But invoking it as utils/trace.h would be ok.)

regards, tom lane

>> On Fri, Jul 21, 2006 at 01:42:26PM +0200, Peter Eisentraut wrote:
>>
>>> I would prefer to drop the PG_ prefixes on PG_TRACE and pg_trace.h. We
>>> know which software we're dealing with.
>>>
>
>
>> I don't know. "trace" is a fairly generic word, how do you know that
>> none of the dozen other libraries we include don't already have a
>> "trace.h" or a TRACE() macro? On any of our supported platforms?
>>
>
> I concur with Martijn. We've already regretted using ERROR as a macro
> name, let's not make the same mistake with TRACE. PG_TRACE is good,
> and so is pg_trace.h. (But invoking it as utils/trace.h would be ok.)
>
>
How about the obvious DTRACE( .... ) or some similar variant?

-- Korry

korry <korryd(at)enterprisedb(dot)com> writes:
> How about the obvious DTRACE( .... ) or some similar variant?

Because it's supposed to be generic, ie, not strictly tied to DTrace.
(I'm not sure there is any realistic other alternative at the moment,
but that's the idea...)

regards, tom lane

Peter Eisentraut wrote:

>I would prefer to drop the PG_ prefixes on PG_TRACE and pg_trace.h. We
>know which software we're dealing with.
>
>
>
I also agree with Martin & Tom to keep the PG_ prefixes.

>We should probably move the probes file to a subdirectory. Anyone know
>a good place?
>
>Also, again, the pgsql prefix should be dropped.
>
>
To keep it consistent with the header file, perhaps it can be renamed to
pg_probes.d

>Certainly doable, but will that be more reliable? Can't we convince
>dtrace to create binaries for the host platform by default?
>
>
The user needs to have the flexibility to build a 32 bit PG binary even
when he run the 64 bit kernel. If I understand you correctly, your
suggestion will not allow a 32 bit binary to be built on a 64 bit OS.

>>3) When using --enable-depend, "gmake clean" removes all *.d files,
>>
>>
>
>I'm working on renaming the dependency files.
>
>
>
Excellent!

Thanks Peter for your help!

Regards,
-Robert

korry wrote:

> How about the obvious DTRACE( .... ) or some similar variant?

The idea is to keep the macro name generic since it can be mapped to
other tracing facility on other platforms.

Regards,
-Robert

Robert Lor wrote:
> The user needs to have the flexibility to build a 32 bit PG binary
> even when he run the 64 bit kernel. If I understand you correctly,
> your suggestion will not allow a 32 bit binary to be built on a 64
> bit OS.

I'm not sure about the context. How do you control whether the
PostgreSQL binaries you are about to build end up 32 bit or 64 bit?
Presumably there is some default, and you switch it using CFLAGS or
LDFLAGS. Then it would make sense to let dtrace be controled by
DTRACE_FLAGS or some such. But what does dtrace do if no flag at all
is given?

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

Here is a consolidated patch that contains all the files. I made some
configure and makefile adjustments and put standard comment headers in
all the files. You can use DTRACEFLAGS to pass options to configure,
which should help sorting out the 32/64-bit issue. The problem of the
*.d files is already gone in CVS.

Since I don't have access to a Solaris system, this is untested for the
DTrace-enabled case. The only thing left to do besides actually
testing that case would be moving the probes.d file to a different
location, since we probably don't want to have special-purpose files in
src/backend.

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

Peter Eisentraut wrote:

>Robert Lor wrote:
>
>
>>The user needs to have the flexibility to build a 32 bit PG binary
>>even when he run the 64 bit kernel. If I understand you correctly,
>>your suggestion will not allow a 32 bit binary to be built on a 64
>>bit OS.
>>
>>
>
>I'm not sure about the context. How do you control whether the
>PostgreSQL binaries you are about to build end up 32 bit or 64 bit?
>Presumably there is some default, and you switch it using CFLAGS or
>LDFLAGS.
>
To build 64 bit binary, I use the following flag depending on the
compiler. Without -m64 or -xtarget=native64, it defaults to 32 bit.
CC='gcc -m64'
CC='/<path_to_sun_compiler>/cc -xtarget=native64'

> Then it would make sense to let dtrace be controled by
>DTRACE_FLAGS or some such. But what does dtrace do if no flag at all
>is given?
>
>
We want to be able to set DTRACEFLAGS to 32 or 64 (e.g. DTRACEFLAGS='64').

If DTRACEFLAGS is not set, can we provide a default value to 32?
Otherwise, the compile will fail. It's also possible that the CC and
DTRACEFLAGS are in conflict, and in that case the compile will also
fail, which is probably okay.

Regards,
-Robert

Peter, I'll test the patch on Solaris. Thanks!

Regards,
-Robert

Peter Eisentraut wrote:

>Here is a consolidated patch that contains all the files. I made some
>configure and makefile adjustments and put standard comment headers in
>all the files. You can use DTRACEFLAGS to pass options to configure,
>which should help sorting out the 32/64-bit issue. The problem of the
>*.d files is already gone in CVS.
>
>Since I don't have access to a Solaris system, this is untested for the
>DTrace-enabled case. The only thing left to do besides actually
>testing that case would be moving the probes.d file to a different
>location, since we probably don't want to have special-purpose files in
>src/backend.
>
>
>
>------------------------------------------------------------------------
>
>diff -uNr ../cvs-pgsql/configure ./configure
>--- ../cvs-pgsql/configure 2006-07-21 23:35:48.000000000 +0200
>+++ ./configure 2006-07-22 01:21:54.000000000 +0200
>@@ -314,7 +314,7 @@
> # include <unistd.h>
> #endif"
>
>-ac_subst_vars='SHELL PATH_SEPARATOR PACKAGE_NAME PACKAGE_TARNAME PACKAGE_VERSION PACKAGE_STRING PACKAGE_BUGREPORT exec_prefix prefix program_transform_name bindir sbindir libexecdir datadir sysconfdir sharedstatedir localstatedir libdir includedir oldincludedir infodir mandir build_alias host_alias target_alias DEFS ECHO_C ECHO_N ECHO_T LIBS configure_args build build_cpu build_vendor build_os host host_cpu host_vendor host_os PORTNAME docdir enable_nls WANTED_LANGUAGES default_port enable_shared enable_rpath enable_debug CC CFLAGS LDFLAGS CPPFLAGS ac_ct_CC EXEEXT OBJEXT CPP GCC TAS autodepend INCLUDES enable_thread_safety with_tcl with_perl with_python with_krb5 krb_srvtab with_pam with_ldap with_bonjour with_openssl with_zlib EGREP ELF_SYS LDFLAGS_SL AWK FLEX FLEXFLAGS LN_S LD with_gnu_ld ld_R_works RANLIB ac_ct_RANLIB TAR STRIP ac_ct_STRIP STRIP_STATIC_LIB STRIP_SHARED_LIB YACC YFLAGS PERL perl_archlibexp perl_privlibexp perl_useshrplib perl_embed_ldflags PYTHON python_version python_configdir python_includespec python_libdir python_libspec python_additional_libs HAVE_IPV6 LIBOBJS acx_pthread_config PTHREAD_CC PTHREAD_LIBS PTHREAD_CFLAGS HAVE_POSIX_SIGNALS MSGFMT MSGMERGE XGETTEXT localedir TCLSH TCL_CONFIG_SH TCL_INCLUDE_SPEC TCL_LIB_FILE TCL_LIBS TCL_LIB_SPEC TCL_SHARED_BUILD TCL_SHLIB_LD_LIBS NSGMLS JADE have_docbook DOCBOOKSTYLE COLLATEINDEX SGMLSPL vpath_build LTLIBOBJS'
>+ac_subst_vars='SHELL PATH_SEPARATOR PACKAGE_NAME PACKAGE_TARNAME PACKAGE_VERSION PACKAGE_STRING PACKAGE_BUGREPORT exec_prefix prefix program_transform_name bindir sbindir libexecdir datadir sysconfdir sharedstatedir localstatedir libdir includedir oldincludedir infodir mandir build_alias host_alias target_alias DEFS ECHO_C ECHO_N ECHO_T LIBS configure_args build build_cpu build_vendor build_os host host_cpu host_vendor host_os PORTNAME docdir enable_nls WANTED_LANGUAGES default_port enable_shared enable_rpath enable_debug DTRACE DTRACEFLAGS enable_dtrace CC CFLAGS LDFLAGS CPPFLAGS ac_ct_CC EXEEXT OBJEXT CPP GCC TAS autodepend INCLUDES enable_thread_safety with_tcl with_perl with_python with_krb5 krb_srvtab with_pam with_ldap with_bonjour with_openssl with_zlib EGREP ELF_SYS LDFLAGS_SL AWK FLEX FLEXFLAGS LN_S LD with_gnu_ld ld_R_works RANLIB ac_ct_RANLIB TAR STRIP ac_ct_STRIP STRIP_STATIC_LIB STRIP_SHARED_LIB YACC YFLAGS PERL perl_archlibexp perl_privlibexp perl_useshrplib perl_embed_ldflags PYTHON python_version python_configdir python_includespec python_libdir python_libspec python_additional_libs HAVE_IPV6 LIBOBJS acx_pthread_config PTHREAD_CC PTHREAD_LIBS PTHREAD_CFLAGS HAVE_POSIX_SIGNALS MSGFMT MSGMERGE XGETTEXT localedir TCLSH TCL_CONFIG_SH TCL_INCLUDE_SPEC TCL_LIB_FILE TCL_LIBS TCL_LIB_SPEC TCL_SHARED_BUILD TCL_SHLIB_LD_LIBS NSGMLS JADE have_docbook DOCBOOKSTYLE COLLATEINDEX SGMLSPL vpath_build LTLIBOBJS'
> ac_subst_files=''
>
> # Initialize some variables set by options.
>@@ -865,6 +865,7 @@
> --disable-rpath do not embed shared library search path in executables
> --disable-spinlocks do not use spinlocks
> --enable-debug build with debugging symbols (-g)
>+ --enable-dtrace build with DTrace support
> --enable-depend turn on automatic dependency tracking
> --enable-cassert enable assertion checks (for debugging)
> --enable-thread-safety make client libraries thread-safe
>@@ -1947,6 +1948,82 @@
>
>
> #
>+# DTrace
>+#
>+
>+
>+
>+# Check whether --enable-dtrace or --disable-dtrace was given.
>+if test "${enable_dtrace+set}" = set; then
>+ enableval="$enable_dtrace"
>+
>+ case $enableval in
>+ yes)
>+
>+cat >>confdefs.h <<\_ACEOF
>+#define ENABLE_DTRACE 1
>+_ACEOF
>+
>+for ac_prog in dtrace
>+do
>+ # Extract the first word of "$ac_prog", so it can be a program name with args.
>+set dummy $ac_prog; ac_word=$2
>+echo "$as_me:$LINENO: checking for $ac_word" >&5
>+echo $ECHO_N "checking for $ac_word... $ECHO_C" >&6
>+if test "${ac_cv_prog_DTRACE+set}" = set; then
>+ echo $ECHO_N "(cached) $ECHO_C" >&6
>+else
>+ if test -n "$DTRACE"; then
>+ ac_cv_prog_DTRACE="$DTRACE" # Let the user override the test.
>+else
>+as_save_IFS=$IFS; IFS=$PATH_SEPARATOR
>+for as_dir in $PATH
>+do
>+ IFS=$as_save_IFS
>+ test -z "$as_dir" && as_dir=.
>+ for ac_exec_ext in '' $ac_executable_extensions; do
>+ if $as_executable_p "$as_dir/$ac_word$ac_exec_ext"; then
>+ ac_cv_prog_DTRACE="$ac_prog"
>+ echo "$as_me:$LINENO: found $as_dir/$ac_word$ac_exec_ext" >&5
>+ break 2
>+ fi
>+done
>+done
>+
>+fi
>+fi
>+DTRACE=$ac_cv_prog_DTRACE
>+if test -n "$DTRACE"; then
>+ echo "$as_me:$LINENO: result: $DTRACE" >&5
>+echo "${ECHO_T}$DTRACE" >&6
>+else
>+ echo "$as_me:$LINENO: result: no" >&5
>+echo "${ECHO_T}no" >&6
>+fi
>+
>+ test -n "$DTRACE" && break
>+done
>+
>+
>+ ;;
>+ no)
>+ :
>+ ;;
>+ *)
>+ { { echo "$as_me:$LINENO: error: no argument expected for --enable-dtrace option" >&5
>+echo "$as_me: error: no argument expected for --enable-dtrace option" >&2;}
>+ { (exit 1); exit 1; }; }
>+ ;;
>+ esac
>+
>+else
>+ enable_dtrace=no
>+
>+fi;
>+
>+
>+
>+#
> # C compiler
> #
>
>@@ -22759,6 +22836,7 @@
> enable_rpath) ;;
> enable_spinlocks) ;;
> enable_debug) ;;
>+enable_dtrace) ;;
> with_CC) ;;
> enable_depend) ;;
> enable_cassert) ;;
>@@ -23431,6 +23509,9 @@
> s,@enable_shared@,$enable_shared,;t t
> s,@enable_rpath@,$enable_rpath,;t t
> s,@enable_debug@,$enable_debug,;t t
>+s,@DTRACE@,$DTRACE,;t t
>+s,@DTRACEFLAGS@,$DTRACEFLAGS,;t t
>+s,@enable_dtrace@,$enable_dtrace,;t t
> s,@CC@,$CC,;t t
> s,@CFLAGS@,$CFLAGS,;t t
> s,@LDFLAGS@,$LDFLAGS,;t t
>diff -uNr ../cvs-pgsql/configure.in ./configure.in
>--- ../cvs-pgsql/configure.in 2006-07-21 23:35:48.000000000 +0200
>+++ ./configure.in 2006-07-22 01:09:54.000000000 +0200
>@@ -206,6 +206,17 @@
> AC_SUBST(enable_debug)
>
> #
>+# DTrace
>+#
>+PGAC_ARG_BOOL(enable, dtrace, no,
>+ [ --enable-dtrace build with DTrace support],
>+[AC_DEFINE([ENABLE_DTRACE], 1,
>+ [Define to 1 to enable DTrace support. (--enable-dtrace)])
>+AC_CHECK_PROGS(DTRACE, dtrace)
>+AC_SUBST(DTRACEFLAGS)])
>+AC_SUBST(enable_dtrace)
>+
>+#
> # C compiler
> #
>
>diff -uNr ../cvs-pgsql/src/backend/access/transam/xact.c ./src/backend/access/transam/xact.c
>--- ../cvs-pgsql/src/backend/access/transam/xact.c 2006-07-21 23:36:01.000000000 +0200
>+++ ./src/backend/access/transam/xact.c 2006-07-22 00:46:42.000000000 +0200
>@@ -1384,6 +1384,8 @@
>
> XactLockTableInsert(s->transactionId);
>
>+ PG_TRACE1 (transaction__start, s->transactionId);
>+
> /*
> * set transaction_timestamp() (a/k/a now()). We want this to be the
> * same as the first command's statement_timestamp(), so don't do a
>@@ -1535,6 +1537,8 @@
> LWLockRelease(ProcArrayLock);
> }
>
>+ PG_TRACE1 (transaction__commit, s->transactionId);
>+
> /*
> * This is all post-commit cleanup. Note that if an error is raised here,
> * it's too late to abort the transaction. This should be just
>@@ -1931,6 +1935,8 @@
> LWLockRelease(ProcArrayLock);
> }
>
>+ PG_TRACE1 (transaction__abort, s->transactionId);
>+
> /*
> * Post-abort cleanup. See notes in CommitTransaction() concerning
> * ordering.
>diff -uNr ../cvs-pgsql/src/backend/Makefile ./src/backend/Makefile
>--- ../cvs-pgsql/src/backend/Makefile 2006-07-21 23:36:00.000000000 +0200
>+++ ./src/backend/Makefile 2006-07-22 01:21:06.000000000 +0200
>@@ -19,7 +19,11 @@
>
> SUBSYSOBJS := $(DIRS:%=%/SUBSYS.o)
>
>-OBJS := $(SUBSYSOBJS) $(top_builddir)/src/port/libpgport_srv.a
>+OBJS = $(SUBSYSOBJS) $(top_builddir)/src/port/libpgport_srv.a
>+
>+ifeq ($(enable_dtrace), yes)
>+OBJS += probes.o
>+endif
>
> # We put libpgport into OBJS, so remove it from LIBS
> LIBS := $(filter-out -lpgport, $(LIBS))
>@@ -135,6 +139,10 @@
> $(LN_S) ../../../$(subdir)/utils/fmgroids.h .
>
>
>+probes.o: probes.d $(SUBSYSOBJS)
>+ $(DTRACE) $(DTRACEFLAGS) -G -s $^
>+
>+
> ##########################################################################
>
> distprep:
>diff -uNr ../cvs-pgsql/src/backend/probes.d ./src/backend/probes.d
>--- ../cvs-pgsql/src/backend/probes.d 1970-01-01 01:00:00.000000000 +0100
>+++ ./src/backend/probes.d 2006-07-22 01:17:59.000000000 +0200
>@@ -0,0 +1,24 @@
>+/* ----------
>+ * DTrace probes for PostgreSQL backend
>+ *
>+ * Copyright (c) 2006, PostgreSQL Global Development Group
>+ *
>+ * $PostgreSQL$
>+ * ----------
>+ */
>+
>+provider postgresql {
>+
>+probe transaction__start(int);
>+probe transaction__commit(int);
>+probe transaction__abort(int);
>+probe lwlock__acquire(int, int);
>+probe lwlock__release(int);
>+probe lwlock__startwait(int, int);
>+probe lwlock__endwait(int, int);
>+probe lwlock__condacquire(int, int);
>+probe lwlock__condacquire__fail(int, int);
>+probe lock__startwait(int, int);
>+probe lock__endwait(int, int);
>+
>+};
>diff -uNr ../cvs-pgsql/src/backend/storage/lmgr/lock.c ./src/backend/storage/lmgr/lock.c
>--- ../cvs-pgsql/src/backend/storage/lmgr/lock.c 2006-07-21 23:36:08.000000000 +0200
>+++ ./src/backend/storage/lmgr/lock.c 2006-07-22 00:49:47.000000000 +0200
>@@ -752,8 +752,13 @@
> /*
> * Sleep till someone wakes me up.
> */
>+
>+ PG_TRACE2(lock__startwait, locktag->locktag_field2, lockmode);
>+
> WaitOnLock(locallock, owner);
>
>+ PG_TRACE2(lock__endwait, locktag->locktag_field2, lockmode);
>+
> /*
> * NOTE: do not do any material change of state between here and
> * return. All required changes in locktable state must have been
>diff -uNr ../cvs-pgsql/src/backend/storage/lmgr/lwlock.c ./src/backend/storage/lmgr/lwlock.c
>--- ../cvs-pgsql/src/backend/storage/lmgr/lwlock.c 2006-07-21 23:36:08.000000000 +0200
>+++ ./src/backend/storage/lmgr/lwlock.c 2006-07-22 00:50:23.000000000 +0200
>@@ -424,6 +424,8 @@
> block_counts[lockid]++;
> #endif
>
>+ PG_TRACE2(lwlock__startwait, lockid, mode);
>+
> for (;;)
> {
> /* "false" means cannot accept cancel/die interrupt here. */
>@@ -433,6 +435,8 @@
> extraWaits++;
> }
>
>+ PG_TRACE2(lwlock__endwait, lockid, mode);
>+
> LOG_LWDEBUG("LWLockAcquire", lockid, "awakened");
>
> /* Now loop back and try to acquire lock again. */
>@@ -442,6 +446,8 @@
> /* We are done updating shared state of the lock itself. */
> SpinLockRelease(&lock->mutex);
>
>+ PG_TRACE2(lwlock__acquire, lockid, mode);
>+
> /* Add lock to list of locks held by this backend */
> held_lwlocks[num_held_lwlocks++] = lockid;
>
>@@ -511,11 +517,13 @@
> /* Failed to get lock, so release interrupt holdoff */
> RESUME_INTERRUPTS();
> LOG_LWDEBUG("LWLockConditionalAcquire", lockid, "failed");
>+ PG_TRACE2(lwlock__condacquire__fail, lockid, mode);
> }
> else
> {
> /* Add lock to list of locks held by this backend */
> held_lwlocks[num_held_lwlocks++] = lockid;
>+ PG_TRACE2(lwlock__condacquire, lockid, mode);
> }
>
> return !mustwait;
>@@ -600,6 +608,8 @@
> /* We are done updating shared state of the lock itself. */
> SpinLockRelease(&lock->mutex);
>
>+ PG_TRACE1(lwlock__release, lockid);
>+
> /*
> * Awaken any waiters I removed from the queue.
> */
>diff -uNr ../cvs-pgsql/src/include/c.h ./src/include/c.h
>--- ../cvs-pgsql/src/include/c.h 2006-07-21 23:36:15.000000000 +0200
>+++ ./src/include/c.h 2006-07-22 00:46:42.000000000 +0200
>@@ -56,6 +56,7 @@
> #include "pg_config_os.h" /* must be before any system header files */
> #endif
> #include "postgres_ext.h"
>+#include "pg_trace.h"
>
> #if defined(_MSC_VER) || defined(__BORLANDC__)
> #define WIN32_ONLY_COMPILER
>diff -uNr ../cvs-pgsql/src/include/pg_config.h.in ./src/include/pg_config.h.in
>--- ../cvs-pgsql/src/include/pg_config.h.in 2006-07-21 23:36:15.000000000 +0200
>+++ ./src/include/pg_config.h.in 2006-07-22 00:46:42.000000000 +0200
>@@ -36,6 +36,9 @@
> /* Define to the default TCP port number as a string constant. */
> #undef DEF_PGPORT_STR
>
>+/* Define to 1 to enable DTrace support. (--enable-dtrace) */
>+#undef ENABLE_DTRACE
>+
> /* Define to 1 if you want National Language Support. (--enable-nls) */
> #undef ENABLE_NLS
>
>diff -uNr ../cvs-pgsql/src/include/pg_trace.h ./src/include/pg_trace.h
>--- ../cvs-pgsql/src/include/pg_trace.h 1970-01-01 01:00:00.000000000 +0100
>+++ ./src/include/pg_trace.h 2006-07-22 00:54:22.000000000 +0200
>@@ -0,0 +1,56 @@
>+/* ----------
>+ * pg_trace.h
>+ *
>+ * Definitions for the PostgreSQL tracing framework
>+ *
>+ * Copyright (c) 2006, PostgreSQL Global Development Group
>+ *
>+ * $PostgreSQL$
>+ * ----------
>+ */
>+
>+#ifndef PG_TRACE_H
>+#define PG_TRACE_H
>+
>+#ifdef ENABLE_DTRACE
>+
>+#include <sys/sdt.h>
>+
>+/*
>+ * The PG_TRACE macros are mapped to the appropriate macros used by DTrace.
>+ *
>+ * Only one DTrace provider called "postgresql" will be used for PostgreSQL,
>+ * so the name is hard-coded here to avoid having to specify it in the
>+ * source code.
>+ */
>+
>+#define PG_TRACE(name) \
>+ DTRACE_PROBE(postgresql, name)
>+#define PG_TRACE1(name, arg1) \
>+ DTRACE_PROBE1(postgresql, name, arg1)
>+#define PG_TRACE2(name, arg1, arg2) \
>+ DTRACE_PROBE2(postgresql, name, arg1, arg2)
>+#define PG_TRACE3(name, arg1, arg2, arg3) \
>+ DTRACE_PROBE3(postgresql, name, arg1, arg2, arg3)
>+#define PG_TRACE4(name, arg1, arg2, arg3, arg4) \
>+ DTRACE_PROBE4(postgresql, name, arg1, arg2, arg3, arg4)
>+#define PG_TRACE5(name, arg1, arg2, arg3, arg4, arg5) \
>+ DTRACE_PROBE5(postgresql, name, arg1, arg2, arg3, arg4, arg5)
>+
>+#else /* not ENABLE_DTRACE */
>+
>+/*
>+ * Unless DTrace is explicitly enabled with --enable-dtrace, the PG_TRACE
>+ * macros will expand to no-ops.
>+ */
>+
>+#define PG_TRACE(name)
>+#define PG_TRACE1(name, arg1)
>+#define PG_TRACE2(name, arg1, arg2)
>+#define PG_TRACE3(name, arg1, arg2, arg3)
>+#define PG_TRACE4(name, arg1, arg2, arg3, arg4)
>+#define PG_TRACE5(name, arg1, arg2, arg3, arg4, arg5)
>+
>+#endif /* not ENABLE_DTRACE */
>+
>+#endif /* PG_TRACE_H */
>diff -uNr ../cvs-pgsql/src/Makefile.global.in ./src/Makefile.global.in
>--- ../cvs-pgsql/src/Makefile.global.in 2006-07-22 00:37:37.000000000 +0200
>+++ ./src/Makefile.global.in 2006-07-22 01:09:53.000000000 +0200
>@@ -157,6 +157,7 @@
> enable_rpath = @enable_rpath@
> enable_nls = @enable_nls@
> enable_debug = @enable_debug@
>+enable_dtrace = @enable_dtrace@
> enable_thread_safety = @enable_thread_safety@
>
> python_includespec = @python_includespec@
>@@ -212,6 +213,8 @@
> YFLAGS = @YFLAGS@
> FLEX = @FLEX@
> FLEXFLAGS = @FLEXFLAGS@ $(LFLAGS)
>+DTRACE = @DTRACE@
>+DTRACEFLAGS = @DTRACEFLAGS@
>
> # Linking
>
>
>

Peter Eisentraut wrote:

>Here is a consolidated patch that contains all the files. I made some
>configure and makefile adjustments and put standard comment headers in
>all the files. You can use DTRACEFLAGS to pass options to configure,
>which should help sorting out the 32/64-bit issue. The problem of the
>*.d files is already gone in CVS.
>
>Since I don't have access to a Solaris system, this is untested for the
>DTrace-enabled case. The only thing left to do besides actually
>testing that case would be moving the probes.d file to a different
>location, since we probably don't want to have special-purpose files in
>src/backend.
>
>
>
I can't seem to apply the patch on Solaris. I ran the patch command from
the top of the src tree and got the following messages. When I tried to
enter a file name, it goes into infinite loop. What did I do wrong?

-bash-3.00$ patch -p0 -u < /var/tmp/dtrace-patch.diff
Looks like a unified context diff.
The next patch looks like a unified context diff.
The next patch looks like a unified context diff.
The next patch looks like a unified context diff.
The next patch looks like a unified context diff.
File to patch:

Regards,
-Robert

Robert Lor wrote:
> I can't seem to apply the patch on Solaris.

Perhaps the attached patch in -c format will work better.

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

Peter Eisentraut wrote:

>Perhaps the attached patch in -c format will work better.
>
>
>
Now I'm getting a different type of error. After running the patch
command, the configure file is partially patched but not the others.
Attached is configure.rej. I just checked out the source from CVS.

-bash-3.00$ patch -p0 < /var/tmp/dtrace-patch-c.diff
Looks like a context diff to me...
Hunk #1 failed at line 314.
Hunk #13 failed at line 19.
2 out of 33 hunks failed: saving rejects to ./configure.rej
done

Regards,
-Robert

Robert Lor wrote:
> Now I'm getting a different type of error. After running the patch
> command, the configure file is partially patched but not the others.
> Attached is configure.rej. I just checked out the source from CVS.

Sorry, there must be something wrong with your source code or patch
tools, or the patch is mangled by email. I'm not sure.

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

Peter Eisentraut wrote:

>Robert Lor wrote:
>
>
>>Now I'm getting a different type of error. After running the patch
>>command, the configure file is partially patched but not the others.
>>Attached is configure.rej. I just checked out the source from CVS.
>>
>>
>
>Sorry, there must be something wrong with your source code or patch
>tools, or the patch is mangled by email. I'm not sure.
>
>
>
I was able to apply the patch after splitting it into individual files,
ran dos2unix on each, and removed the diff line. I believe the problem
is with the patch tool.

Anyways, I tested the patch with --enable-dtrace and/or DTRACEFLAGS, and
it works like a charm! If DTRACEFLAGS is not use, dtrace will default to
32 bit which is what we want. When building a 64 bit binary with
--enable-dtrace, DTRACEFLAGS needs to be set to '-64'. I will submit a
doc patch to explain all of the above.

There is one minor issue - "gmake clean" doesn't remove probe.o.

Regarding where to put probe.d, will src/probe.d work since probes for
all subsystems will go into this one file.

Thanks a bunch Peter!

Regards,
-Robert

Robert Lor wrote:
> Regarding where to put probe.d, will src/probe.d work since probes
> for all subsystems will go into this one file.

As I understand this, the probe file is compiled into some sort of
object file which is linked into the binary. So if we ever have probes
in other components, we'd probably want to have separate probe source
and object files for them. That would seem better than one big probe
file that is linked in everywhere.

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

Peter Eisentraut wrote:

>As I understand this, the probe file is compiled into some sort of
>object file which is linked into the binary.
>
Correct.

>So if we ever have probes
>in other components, we'd probably want to have separate probe source
>and object files for them. That would seem better than one big probe
>file that is linked in everywhere.
>
>
>
We agreed that there would only be one provider called postgresql, and I
believe (need to double check) all the probes have to be defined in the
same provider in the same file. What you suggest sounds like a good way
to separate probes from different components.

Regards,
-Robert

I've committed the dtrace patch. Some documentation would be nice now ...

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

Excellent! I'll submit a doc patch shortly.

Regards,
-Robert

Peter Eisentraut wrote:

>I've committed the dtrace patch. Some documentation would be nice now ...
>
>
>

On Monday 24 July 2006 17:00, Robert Lor wrote:
> Excellent! I'll submit a doc patch shortly.
>
> Regards,
> -Robert
>
> Peter Eisentraut wrote:
> >I've committed the dtrace patch. Some documentation would be nice now ...
>

Looking through -patches I don't see the doc patch, and outside of
installation.sgml there doesn't seem to be anything either. Robert, are you
still on the hook for these? Also should installation.sgml mention the
issueswith building 32 vs 64 bit binaries and/or the issue with static
functions?

--
Robert Treat
Build A Brighter LAMP :: Linux Apache {middleware} PostgreSQL

Robert,

I'm working on the new version of the Solaris FAQ and I mentioned this
in this document. I'm going to sent patch asap.

Zdenek

Robert Treat napsal(a):
> On Monday 24 July 2006 17:00, Robert Lor wrote:
>> Excellent! I'll submit a doc patch shortly.
>>
>> Regards,
>> -Robert
>>
>> Peter Eisentraut wrote:
>>> I've committed the dtrace patch. Some documentation would be nice now ...
>
> Looking through -patches I don't see the doc patch, and outside of
> installation.sgml there doesn't seem to be anything either. Robert, are you
> still on the hook for these? Also should installation.sgml mention the
> issueswith building 32 vs 64 bit binaries and/or the issue with static
> functions?
>

Robert Treat wrote:
> Also should installation.sgml
> mention the issueswith building 32 vs 64 bit binaries

I'm not convinced there is an issue. dtrace will build the right
binaries by default. If you're messing with mixed environments *and*
delve into dtrace, you should probably be able to figure this out
yourself.

> and/or the issue with static functions?

The issue with that is simply that there is no released operating system
version for which the dtrace support works. I'm not sure what to do
about that.

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

Peter Eisentraut wrote:
> Robert Treat wrote:
>> Also should installation.sgml
>> mention the issueswith building 32 vs 64 bit binaries
>
> I'm not convinced there is an issue. dtrace will build the right
> binaries by default. If you're messing with mixed environments *and*
> delve into dtrace, you should probably be able to figure this out
> yourself.
>
>> and/or the issue with static functions?
>
> The issue with that is simply that there is no released operating system
> version for which the dtrace support works. I'm not sure what to do
> about that.
>

Peter, dtrace is part of Solaris 10 as well. Latest version of
opensolaris does not have this problem, but Solaris 10 will solve this
in the update 3.

Zdenek

All,

I'll be fixing this documentation issue now that I have full information.

--
--Josh

Josh Berkus
PostgreSQL @ Sun
San Francisco

Sorry for the delayed response.

Robert Treat wrote:

>Looking through -patches I don't see the doc patch, and outside of
>installation.sgml there doesn't seem to be anything either. Robert, are you
>still on the hook for these?
>
Josh will help submit the doc patch. I have documented the usage
instructions in a couple of places but just have been too busy to get
the patch submitted. My bad.

http://pgfoundry.org/docman/?group_id=1000163
http://blogs.sun.com/robertlor

>Also should installation.sgml mention the
>issues with building 32 vs 64 bit binaries and/or the issue with static
>functions?
>
>
There are no issues with building 32 and 64 bit binaries. The above doc
explains the issue with static function.

Regards,
-Robert

Peter Eisentraut wrote:

>Robert Treat wrote:
>
>
>>Also should installation.sgml
>>mention the issueswith building 32 vs 64 bit binaries
>>
>>
>
>I'm not convinced there is an issue. dtrace will build the right
>binaries by default. If you're messing with mixed environments *and*
>delve into dtrace, you should probably be able to figure this out
>yourself.
>
>
None that I'm aware of.

>
>
>>and/or the issue with static functions?
>>
>>
>
>The issue with that is simply that there is no released operating system
>version for which the dtrace support works. I'm not sure what to do
>about that.
>
>
>
This is a very temporary issue, and it will just require PostgreSQL to
be built on the lastest version of Solaris (e.g Solaris Express), but
the binary can will run just fine on the FCS version (e.g. Solaris 10).
This will be clarified in the doc patch.

-Robert