Re: Nasty resource-leak problem in sort code - Mailing list pgsql-hackers

From Tom Lane
Subject Re: Nasty resource-leak problem in sort code
Date
Msg-id 13342.926092714@sss.pgh.pa.us
Whole thread Raw
In response to Nasty resource-leak problem in sort code  (Tom Lane <tgl@sss.pgh.pa.us>)
Responses Re: [HACKERS] Re: Nasty resource-leak problem in sort code  (geek+@cmu.edu)
Re: [HACKERS] Re: Nasty resource-leak problem in sort code  (Bruce Momjian <maillist@candle.pha.pa.us>)
List pgsql-hackers
I wrote:
> If a CREATE INDEX fails, the backend returns to the main loop without
> having closed the temporary files that are created for sorting.
> ...
> If you then try to create another index, you will crash for
> lack of free file descriptors (unless your kernel has a
> higher-than-usual open-files-per-process limit).  In any case, the
> sort temp files will never get deleted from your database directory.
>
> Offhand I'm not sure how to fix this.  The system knows about releasing
> memory after an elog(ERROR), but does it have any provisions for
> releasing other kinds of resources?

After further thought it seems that adding code to release temporary
files at transaction abort is the best solution.  I propose the
following fix:

1. In fd.c, invent the notion of a "temporary file", ie one that fd.c
knows (a) should be discarded when it is closed and (b) should not
outlive the current transaction.  Add a function TemporaryOpenFile()
that selects an appropriate temp file name, does the open, and marks
the resulting VFD as a temp file.  FileClose will implicitly act like
FileUnlink on this file.

2. Add a hook to xact.c that calls fd.c to close and delete all
remaining temporary files at transaction commit or abort.

3. Change psort.c and nodeHash.c (anyplace else?) to use this facility
instead of their existing ad-hoc temp file code.

This will fix several problems, including (a) failure to release file
descriptors after an error; (b) failure to delete temp files after
an error; (c) risk of running out of FDs if multiple sorts are invoked
concurrently (not sure if this can actually happen in the current state
of the code).  Centralizing the generation of temp file names in one
place seems like a good idea too.

Unless I hear objections, I'll work on this this weekend.
        regards, tom lane


pgsql-hackers by date:

Previous
From: geek+@cmu.edu
Date:
Subject: Re: [HACKERS] pg_dump problem?
Next
From: geek+@cmu.edu
Date:
Subject: Re: [HACKERS] Re: Nasty resource-leak problem in sort code