Re: DTrace probes broken in HEAD on Solaris?

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Zdenek Kotala <Zdenek(dot)Kotala(at)Sun(dot)COM>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: DTrace probes broken in HEAD on Solaris?
Date: 2009-03-17 18:49:16
Message-ID: 24639.1237315756@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Zdenek Kotala <Zdenek(dot)Kotala(at)Sun(dot)COM> writes:
> Answer why it happens when probes are disabled is, that for user
> application there are piece of code which prepare DTrace probes
> arguments which will be passed into kernel DTrace modul. This code has
> performance penalty which depends on number of arguments.

More specifically, it seems that DTrace is designed so that it evaluates
all the arguments to a probe macro before it decides whether to actually
take the trap or not.

This seems to me to be a pretty bad/surprising behavior; it's not the
way that our Assert macros work, for example. There's a performance
issue, which would probably only be brutally obvious if you had an
expensive function in the arguments. (Before you say no one would do
that, note the relpath() calls I was complaining about last week.)
But we've been muttering about dropping probes into some extremely
hot hot-spots, like spinlock acquisition, and even a few more
instructions to copy local variables could make a difference there.

The other thing I don't like is that this implementation exposes people
to any bugs that may exist in the probe arguments, *even when they don't
have any tracing turned on*. (Again, we had two different instances of
that last week, so don't bother arguing that it doesn't happen.)

Both of these considerations are strong arguments for not building
production installations with --enable-dtrace, just as we don't
encourage people to build for production with --enable-cassert.

But of course part of the argument for dtrace support is that people
would like to have such probing available in production installations.

What I've found out about this is that for each probe macro, DTrace
also defines a foo_ENABLED() macro that can be used like this:

if (foo_ENABLED())
foo(...);

I think what we should do about these considerations is fix our macro
definitions so that the if(ENABLED()) test is built into the macros.
I'm not sure what this will require ... probably some post-processing
of the probes.h file ... but if we don't do it we're going to keep
getting bit.

Comments?

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Bruce Momjian 2009-03-17 19:24:57 Re: Problem with zero year
Previous Message Chuck McDevitt 2009-03-17 18:48:40 Re: Solaris getopt_long and PostgreSQL