Broken stuff in new dtrace probes

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: pgsql-hackers(at)postgreSQL(dot)org
Subject: Broken stuff in new dtrace probes
Date: 2009-03-11 20:38:50
Message-ID: 3307.1236803930@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: 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

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Josh Berkus 2009-03-11 21:27:51 Should SET ROLE inherit config params?
Previous Message Alvaro Herrera 2009-03-11 20:09:06 Re: gcc: why optimize for size flag is not the default