Thread

1. Re: Nasty resource-leak problem in sort code

   Tom Lane <tgl@sss.pgh.pa.us> — 1999-05-07T15:58:34Z

   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
   
   

2. Re: [HACKERS] Re: Nasty resource-leak problem in sort code

   Brian E Gallew <geek+@cmu.edu> — 1999-05-07T16:18:32Z

   Then <tgl@sss.pgh.pa.us> spoke up and said:
   > After further thought it seems that adding code to release temporary
   > files at transaction abort is the best solution.  I propose the
   > following fix:
   [ explanation snipped ]
   
   Uhm, this all seems unnecessarily complicated.  Shouldn't the process
   look more like this:
   fp = open('tempfile');
   unlink('tempfile');
   
   This way, when the file is closed, the space is freed.  The only
   complication I can see is if backends need to share the file handle,
   or it needs to be re-opened.  This works with all sorts of temp-file
   situations.
   
   Of course, it's not NT safe, since I don't believe that NT provides
   for deleting open files (NT file libs sucks.
   
   -- 
   =====================================================================
   | JAVA must have been developed in the wilds of West Virginia.      |
   | After all, why else would it support only single inheritance??    |
   =====================================================================
   | Finger geek@cmu.edu for my public key.                            |
   =====================================================================
   

3. Re: [HACKERS] Re: Nasty resource-leak problem in sort code

   Bruce Momjian <maillist@candle.pha.pa.us> — 1999-05-07T23:01:45Z

   > 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.
   
   This sounds like a good plan.
   
   -- 
     Bruce Momjian                        |  http://www.op.net/~candle
     maillist@candle.pha.pa.us            |  (610) 853-3000
     +  If your life is a hard drive,     |  830 Blythe Avenue
     +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026
   
   

4. Re: [HACKERS] Re: Nasty resource-leak problem in sort code]

   Bruce Momjian <maillist@candle.pha.pa.us> — 1999-05-07T23:03:35Z

   > Uhm, this all seems unnecessarily complicated.  Shouldn't the process
   > look more like this:
   > fp = open('tempfile');
   > unlink('tempfile');
   > 
   > This way, when the file is closed, the space is freed.  The only
   > complication I can see is if backends need to share the file handle,
   > or it needs to be re-opened.  This works with all sorts of temp-file
   > situations.
   > 
   > Of course, it's not NT safe, since I don't believe that NT provides
   > for deleting open files (NT file libs sucks.
   
   Two problems.  First, we support NT, so we have to behave a little bit
   to keep it happy.  Second, Tom is concerned about leaking file handles,
   not just the files themselves.  He needs to call close() to release them
   on transaction aborts.
   
   -- 
     Bruce Momjian                        |  http://www.op.net/~candle
     maillist@candle.pha.pa.us            |  (610) 853-3000
     +  If your life is a hard drive,     |  830 Blythe Avenue
     +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026