Re: DTrace probes broken in HEAD on Solaris? - Mailing list pgsql-hackers

Tom Lane wrote:
> Zdenek Kotala <Zdenek.Kotala@Sun.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.
>
>
> 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.)
>   

Yes, the above scenario can occur when compiling with DTrace (passing 
--enable-dtrace to configure) even when the probes are not enabled. It 
won't be an issue if you don't compile with DTrace though as the macros 
are converted to no-ops. Hopefully, any bugs in the probe arguments 
would be caught during testing ;-)  

> 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.
>   
I was contemplating on using the is-enabled test, but when checking the 
arguments to all the probes, they were quite simple (except the 
relpath() call which is now removed).

I think the is-enabled test will address the issues you encountered. I 
see a few alternative to fixing this:

1) Only use if (foo_ENABLED()) test  for probes with expensive function 
call/computation in arguments. This will keep the code clean, and we can 
document this in the "Definine New Probes" section in the online doc.

2) Add the if(foo_ENABLED()) test to all probes manually and make this a 
requirement for all future probes. This makes the check explicit and 
avoid confusion.

3) Post-process probes.h so if(foo_ENABLED()) test is added to every 
probe. We're doing some post-processing now by pre-pending TRACE_ to the 
macros with a sed script. Personally, I don't like doing complex 
post-processing of output from another tool because the script can break 
if for some reason DTrace's output is changed.

I prefer option 1 the most and 3 the least.


-Robert