Re: Review: DTrace probes (merged version) ver_03

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
> sure if that would complicate the use of the probe so much as to be
> counterproductive.
>
Are you referring to this struct?

typedef struct buftag
{
RelFileNode rnode; /* physical relation
identifier */
BlockNumber blockNum; /* blknum relative to begin of
reln */
} BufferTag;

I'm not familiar with this pending patch, but why would it break when
another field is added?

It's certainly possible to pass the buffer tag, but I'd say it's not a
good idea if the other fields will never be used.

> * I find this to be truly bletcherous:
>
>
>> ! /*
>> ! * Due to a bug in Mac OS X 10.5, using defined types (e.g. uintptr_t,
>> ! * uint32_t, etc.) cause compilation error.
>> ! */
>> !
>> ! probe transaction__start(unsigned int transactionId);
>> ! probe transaction__commit(unsigned int transactionId);
>> ! probe transaction__abort(unsigned int transactionId);
>>
>
> especially since some of the manual translations in the file are flat
> out wrong (Oid is unsigned for instance).
Oops!
> Furthermore the comment is
> wrong, at least according to my tests with XCode 3.1. Typedefs seem to
> work fine.

The issue is with Apple's dtrace implementation, not Xcode. For more
info, please see the link below.

http://www.opensolaris.org/jive/thread.jspa?messageID=252503&#252503

> What I suggest might be a reasonable compromise is to copy needed
> typedefs directly into the probes.d file:
>
> typedef unsigned int LocalTransactionId;
>
> provider postgresql {
>
> probe transaction__start(LocalTransactionId);
>
>
I like this solution.

--
Robert Lor Sun Microsystems
Austin, USA http://sun.com/postgresql