Re: [HACKERS] [PATCHES] Patch to log usage of temporary files

On Tue, 2007-01-09 at 17:16 -0500, Bruce Momjian wrote:
> Tom Lane wrote:

> > > /* reset flag so that die() interrupt won't cause problems */
> > > vfdP->fdstate &= ~FD_TEMPORARY;
> > > + PG_TRACE1(temp__file__cleanup, vfdP->fileName);
> > > + if (log_temp_files >= 0)
> > > + {
> > > + if (stat(vfdP->fileName, &filestats) == 0)
> >
> > The TRACE is in the wrong place no? I thought it was going to be after
> > the stat() operation so it could pass the file size.

We had that discussion already. If you only pass it after the stat()
then you cannot use DTrace, except when you already get a message in the
log and therefore don't need DTrace. DTrace can get the filesize if it
likes, but thats up to the script author.

> > Also, I dunno much about DTrace, but I had the idea that you can't
> > simply throw a PG_TRACE macro into the source and think you are done
> > --- isn't there a file of probe declarations to add to? Not to mention
> > the documentation of what probes exist.
>
> I didn't like the macro in that area anyway. It seems too adhock to
> just throw it in when we have so few places monitored now. Removed.

err... why are we removing it? The patch should have included an
addition to the probes.d file also, but that should be fixed, not
removed. Don't we normally reject incomplete patches?

You can't say we don't have many probes so we won't add one. There never
will be many if we do that - its a circular argument.

--
Simon Riggs
EnterpriseDB http://www.enterprisedb.com