Thread: Re: [PATCHES] Generic Monitoring Framework with DTrace patch
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@svana.org> http://svana.org/kleptog/ > From each according to his ability. To each according to his ability to litigate.
Martijn van Oosterhout <kleptog@svana.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
<br /><blockquote cite="mid22369.1153492213@sss.pgh.pa.us" type="cite"><blockquote type="cite"><pre wrap="">On Fri, Jul 21,2006 at 01:42:26PM +0200, Peter Eisentraut wrote: </pre><blockquote type="cite"><pre wrap="">I would prefer to dropthe PG_ prefixes on PG_TRACE and pg_trace.h. We know which software we're dealing with. </pre></blockquote></blockquote><pre wrap=""> </pre><blockquote type="cite"><prewrap="">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? </pre></blockquote><pre wrap=""> 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.) </pre></blockquote> How about the obvious DTRACE( .... ) or some similar variant?<br /><br /> -- Korry<br /><br />
korry <korryd@enterprisedb.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/
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