Re: [HACKERS] Review: DTrace probes (merged version) ver_03

2008-08-02 Thread Greg Smith
On Fri, 1 Aug 2008, Alvaro Herrera wrote: Sounds like the thing to do would be to pass CheckpointStats into the DONE probe. I thought it would be more useful to demarcate where the two phases of the checkpoint process were at clearly--the actual times themselves are helpful but dtrace can

Re: [HACKERS] Review: DTrace probes (merged version) ver_03

2008-08-01 Thread Alvaro Herrera
Robert Lor wrote: I made some changes to the sed script so it works with the sed on Solaris OS X. I tested this patch on both Solaris and OS X with DTrace enabled and disabled and also verified that the sed script works with GNU sed. I hope this is the final change for this patch.

Re: [HACKERS] Review: DTrace probes (merged version) ver_03

2008-08-01 Thread Alvaro Herrera
Alvaro Herrera wrote: Applied. I forgot to mention that I renamed the sort_end probe to sort_done, to keep the naming convention. It is a backwards-incompatible change, but there were plenty other probes renamed so my guess is that one more does not matter all that much. Also, it seems I

Re: [HACKERS] Review: DTrace probes (merged version) ver_03

2008-08-01 Thread Robert Lor
Alvaro Herrera wrote: Applied. Thanks! How come Oid works for FLUSH_START but not READ_START and READ_DONE? I'll get the answer for this. Also, I wonder if there's any proof that this works at all on Mac OS X, given that the rule to create probes.o from probes.d is conditionally

Re: [HACKERS] Review: DTrace probes (merged version) ver_03

2008-08-01 Thread Robert Lor
Alvaro Herrera wrote: Alvaro Herrera wrote: Applied. I forgot to mention that I renamed the sort_end probe to sort_done, to keep the naming convention. It is a backwards-incompatible change, but there were plenty other probes renamed so my guess is that one more does not matter all

Re: [HACKERS] Review: DTrace probes (merged version) ver_03

2008-08-01 Thread Greg Smith
One tiny change I'd suggest here: if you look at the code for checkpoint buffer writing there are traces for two points in the process: CheckPointBuffers(int flags) { + TRACE_POSTGRESQL_BUFFER_CHECKPOINT_START(flags); CheckpointStats.ckpt_write_t = GetCurrentTimestamp();

Re: [HACKERS] Review: DTrace probes (merged version) ver_03

2008-08-01 Thread Alvaro Herrera
Greg Smith wrote: One tiny change I'd suggest here: if you look at the code for checkpoint buffer writing there are traces for two points in the process: CheckPointBuffers(int flags) { + TRACE_POSTGRESQL_BUFFER_CHECKPOINT_START(flags); CheckpointStats.ckpt_write_t =

Re: [HACKERS] Review: DTrace probes (merged version) ver_03

2008-08-01 Thread Robert Lor
Alvaro Herrera wrote: Greg Smith wrote: One tiny change I'd suggest here: if you look at the code for checkpoint buffer writing there are traces for two points in the process: CheckPointBuffers(int flags) { + TRACE_POSTGRESQL_BUFFER_CHECKPOINT_START(flags);

Re: [HACKERS] Review: DTrace probes (merged version) ver_03

2008-07-31 Thread Alvaro Herrera
Robert Lor wrote: Hi, What I suggest might be a reasonable compromise is to copy needed typedefs directly into the probes.d file: Implemented this suggestion. There are some weirdness with the OS X compiler causing some of the probe declarations not to compile (see comments in

Re: [HACKERS] Review: DTrace probes (merged version) ver_03

2008-07-31 Thread Robert Lor
Alvaro Herrera wrote: But I don't see a reason to define the rest: +typedef unsigned int locktag_field2; +typedef const char * query_string; +typedef int sortType; +typedef int trueFalse; +typedef int nkeys; +typedef int workMem; +typedef int randomAccess; +typedef unsigned long

Re: [HACKERS] Review: DTrace probes (merged version) ver_03

2008-07-31 Thread Alvaro Herrera
Robert Lor wrote: That Mac OS X problem merits some extra investigation, I think. I'm investigating this one and will find the root cause, but I don't think it should hold back this patch. Other than this, I think this patch can be committed. I'd appreciate if it can be committed

Re: [HACKERS] Review: DTrace probes (merged version) ver_03

2008-07-31 Thread Alvaro Herrera
Robert Lor wrote: Tom Lane wrote: * The probes that pass buffer tag elements are already broken by the pending relation forks patch: there is soon going to be another field in buffer tags. Perhaps it'd be feasible to pass the buffer tag as a single probe argument to make that a bit more

Re: [HACKERS] Review: DTrace probes (merged version) ver_03

2008-07-31 Thread Alvaro Herrera
Here's what I have. Please confirm that this compiles for you. -- Alvaro Herrerahttp://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support Index: src/backend/access/transam/clog.c

Re: [HACKERS] Review: DTrace probes (merged version) ver_03

2008-07-31 Thread Robert Lor
Alvaro Herrera wrote: Here's what I have. Please confirm that this compiles for you. I made some changes to the sed script so it works with the sed on Solaris OS X. I tested this patch on both Solaris and OS X with DTrace enabled and disabled and also verified that the sed script works

Re: [HACKERS] Review: DTrace probes (merged version) ver_03

2008-07-31 Thread Robert Lor
Alvaro Herrera wrote: I was checking the DTrace docs for other reasons and I came across this, which maybe can be useful here: http://docs.sun.com/app/docs/doc/817-6223/chp-xlate?a=view Yes, I think using the translator is the best approach to expose internal structures in a stable

Re: [HACKERS] Review: DTrace probes (merged version) ver_03

2008-07-29 Thread Robert Lor
Updated patch attached. This patch addresses all of Alvaro's feedback except the new probes for v3 protocol which will be submitted later along with the rest of the probes. It also incorporates Tom's feedback as explained inline below. I hope this patch is good to go for this commit fest.

Re: [HACKERS] Review: DTrace probes (merged version) ver_03

2008-07-28 Thread Zdenek Kotala
Alvaro Herrera napsal(a): Zdenek Kotala wrote: I performed review and I prepared own patch which contains only probes without any issue. I suggest commit this patch because the rest of patch is independent and it can be committed next commit fest after rework. I found following issues: I

Re: [HACKERS] Review: DTrace probes (merged version) ver_03

2008-07-28 Thread Zdenek Kotala
Tom Lane napsal(a): Zdenek Kotala [EMAIL PROTECTED] writes: I performed review and I prepared own patch which contains only probes without any issue. I suggest commit this patch because the rest of patch is independent and it can be committed next commit fest after rework. I looked at this

Re: [HACKERS] Review: DTrace probes (merged version) ver_03

2008-07-28 Thread Zdenek Kotala
Zdenek Kotala napsal(a): Alvaro Herrera napsal(a): Zdenek Kotala wrote: I performed review and I prepared own patch which contains only probes without any issue. I suggest commit this patch because the rest of patch is independent and it can be committed next commit fest after rework. I

Re: [HACKERS] Review: DTrace probes (merged version) ver_03

2008-07-28 Thread Tom Lane
Zdenek Kotala [EMAIL PROTECTED] writes: Whats about #include c.h? Does it work or does it have same issue? Same problem --- postgres.h isn't actually including any of the problematic files for itself. What I suggest might be a reasonable compromise is to copy needed typedefs directly into the

Re: [HACKERS] Review: DTrace probes (merged version) ver_03

2008-07-28 Thread Robert Lor
Zdenek Kotala wrote: However what I suggested is commit probes without issue now and the rest will be processed on the next commit fest after rework/discussion. Agreed. I'll fix up the remaining issues with the patch you sent. -- Robert Lor Sun Microsystems Austin, USA

Re: [HACKERS] Review: DTrace probes (merged version) ver_03

2008-07-28 Thread Robert Lor
Zdenek Kotala wrote: Alvaro Herrera napsal(a): Zdenek Kotala wrote: I performed review and I prepared own patch which contains only probes without any issue. I suggest commit this patch because the rest of patch is independent and it can be committed next commit fest after rework. I found

Re: [HACKERS] Review: DTrace probes (merged version) ver_03

2008-07-28 Thread Robert Lor
Tom Lane wrote: * The probes that pass buffer tag elements are already broken by the pending relation forks patch: there is soon going to be another field in buffer tags. Perhaps it'd be feasible to pass the buffer tag as a single probe argument to make that a bit more future-proof? I'm not

Re: [HACKERS] Review: DTrace probes (merged version) ver_03

2008-07-28 Thread Tom Lane
Robert Lor [EMAIL PROTECTED] writes: Tom Lane wrote: * The probes that pass buffer tag elements are already broken by the pending relation forks patch: there is soon going to be another field in buffer tags. I'm not familiar with this pending patch, but why would it break when another

Re: [HACKERS] Review: DTrace probes (merged version) ver_03

2008-07-28 Thread Robert Lor
Tom Lane wrote: By break I meant fail to function usefully. Yes, it would still compile, but if you don't have the fork number available then you won't be able to tell what's really happening in the buffer pool. You might as well not pass any of the buffer tag as pass only part of it. Got

Re: [HACKERS] Review: DTrace probes (merged version) ver_03

2008-07-26 Thread Tom Lane
Zdenek Kotala [EMAIL PROTECTED] writes: I performed review and I prepared own patch which contains only probes without any issue. I suggest commit this patch because the rest of patch is independent and it can be committed next commit fest after rework. I looked at this patch a little bit.

Re: [HACKERS] Review: DTrace probes (merged version) ver_03

2008-07-25 Thread Zdenek Kotala
Theo Schlossnagle napsal(a): On Jul 24, 2008, at 11:11 AM, Zdenek Kotala wrote: I performed review and I prepared own patch which contains only probes without any issue. I suggest commit this patch because the rest of patch is independent and it can be committed next commit fest after

Re: [HACKERS] Review: DTrace probes (merged version) ver_03

2008-07-25 Thread Alvaro Herrera
Zdenek Kotala wrote: I performed review and I prepared own patch which contains only probes without any issue. I suggest commit this patch because the rest of patch is independent and it can be committed next commit fest after rework. I found following issues: I noticed that CLOG, Subtrans

Re: [HACKERS] Review: DTrace probes (merged version) ver_03

2008-07-24 Thread Zdenek Kotala
I performed review and I prepared own patch which contains only probes without any issue. I suggest commit this patch because the rest of patch is independent and it can be committed next commit fest after rework. I found following issues: 1) SLRU probes. I think it is good to have probes

Re: [HACKERS] Review: DTrace probes (merged version) ver_03

2008-07-24 Thread Theo Schlossnagle
On Jul 24, 2008, at 11:11 AM, Zdenek Kotala wrote: I performed review and I prepared own patch which contains only probes without any issue. I suggest commit this patch because the rest of patch is independent and it can be committed next commit fest after rework. I found following