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



--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to