Re: Broken stuff in new dtrace probes

Just returned from vacation ...

Tom Lane wrote:
> I notice that we have in md.c
>
>     TRACE_POSTGRESQL_SMGR_MD_READ_DONE(forknum, blocknum, reln->smgr_rnode.spcNode, reln->smgr_rnode.dbNode,
reln->smgr_rnode.relNode,relpath(reln->smgr_rnode, forknum), nbytes, BLCKSZ);
 
>
>     TRACE_POSTGRESQL_SMGR_MD_WRITE_DONE(forknum, blocknum, reln->smgr_rnode.spcNode, reln->smgr_rnode.dbNode,
reln->smgr_rnode.relNode,relpath(reln->smgr_rnode, forknum), nbytes, BLCKSZ);
 
>
> relpath() returns a palloc'd string, which will not get freed, which
> means that any serious use of these probe points will shortly blow out
> the backend's memory.  So far as I can see the path argument is
> redundant with the other probe arguments and we might as well just
> remove it --- objections?
>   
agreed; the other arguments can be used to get the same result.
> There's another problem in tuplesort.c:
>
>     TRACE_POSTGRESQL_SORT_DONE(state->tapeset,
>             (state->tapeset ? LogicalTapeSetBlocks(state->tapeset) :
>             (state->allowedMem - state->availMem + 1023) / 1024));
>
> This is called after state->tapeset has been freed, which means that the
> LogicalTapeSetBlocks call will very likely give a wrong answer,
> especially in assert-enabled builds but maybe even in a regular one.
>
>             
Good catch and thanks for reviewing the probes.

-Robert