Broken stuff in new dtrace probes - Mailing list pgsql-hackers

From Tom Lane
Subject Broken stuff in new dtrace probes
Date
Msg-id 3307.1236803930@sss.pgh.pa.us
Whole thread Raw
Responses Re: Broken stuff in new dtrace probes  (Tom Lane <tgl@sss.pgh.pa.us>)
Re: Broken stuff in new dtrace probes  (Robert Lor <Robert.Lor@Sun.COM>)
List pgsql-hackers
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?

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.

Seems that we need to have been quality-checking the dtrace patches
a bit more carefully.
        regards, tom lane


pgsql-hackers by date:

Previous
From: "A.M."
Date:
Subject: Re: gcc: why optimize for size flag is not the default
Next
From: Alvaro Herrera
Date:
Subject: Re: gcc: why optimize for size flag is not the default