Hi all,

There are various places in the backend, such as FreeFile(), where the
return value of fclose() is not tested. Whilst we would often notice any
problems with writing to data files due to testing on fsync(), it could
affect things like COPY ... TO, CreateOptsFile() and more.

Are we catching these somewhere else I'm not seeing?

Gavin


Wed, 16 Apr 2003 15:01:25 +1000 (EST)
Gavin Sherry <swm@linuxworld.com.au>
Testing the return value of fclose() in the backend
Gavin Sherry wrote:
> Hi all,
> 
> There are various places in the backend, such as FreeFile(), where the
> return value of fclose() is not tested. Whilst we would often notice any
> problems with writing to data files due to testing on fsync(), it could
> affect things like COPY ... TO, CreateOptsFile() and more.
> 
> Are we catching these somewhere else I'm not seeing?

We are not checking fclose, probably because fclose failures are quite
rare.  Should we be concerned?

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Fri, 30 May 2003 22:54:35 -0400 (EDT)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: Testing the return value of fclose() in the backend
On Fri, 30 May 2003, Bruce Momjian wrote:

> Gavin Sherry wrote:
> > Hi all,
> > 
> > There are various places in the backend, such as FreeFile(), where the
> > return value of fclose() is not tested. Whilst we would often notice any
> > problems with writing to data files due to testing on fsync(), it could
> > affect things like COPY ... TO, CreateOptsFile() and more.
> > 
> > Are we catching these somewhere else I'm not seeing?
> 
> We are not checking fclose, probably because fclose failures are quite
> rare.  Should we be concerned?

fsync() errors are probably just as rare. The problem with long running
daemons not testing for fclose() failure is the problem caused by file
descriptor leakage. I recall that squid (another long running daemon) had
this problem due to the large number of file systems interactions they
undertake. The question is, of course, what to do in postgres if
fclose() returns an error? elog(WARNING)? Another fclose() call? Not sure
what squid did.

Gavin


Sat, 31 May 2003 13:06:15 +1000 (EST)
Gavin Sherry <swm@linuxworld.com.au>
Re: Testing the return value of fclose() in the backend
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> Gavin Sherry wrote:
>> There are various places in the backend, such as FreeFile(), where the
>> return value of fclose() is not tested.

> We are not checking fclose, probably because fclose failures are quite
> rare.  Should we be concerned?

Probably.  Closing a valid file descriptor in itself can't provoke any
error that I can imagine, but fclose() also implies fflush() --- so if
you have written data that hasn't yet been forced out of the stdio
buffers then out-of-disk-space is certainly a foreseeable failure.

fclose failure on an open-for-read-only file seems like Assert()
material; it "can't happen".

			regards, tom lane

Fri, 30 May 2003 23:36:10 -0400
Tom Lane <tgl@sss.pgh.pa.us>
Re: Testing the return value of fclose() in the backend 
On Fri, 30 May 2003, Tom Lane wrote:

> Bruce Momjian <pgman@candle.pha.pa.us> writes:
> > Gavin Sherry wrote:
> >> There are various places in the backend, such as FreeFile(), where the
> >> return value of fclose() is not tested.
> 
> > We are not checking fclose, probably because fclose failures are quite
> > rare.  Should we be concerned?
> 
> Probably.  Closing a valid file descriptor in itself can't provoke any
> error that I can imagine, but fclose() also implies fflush() --- so if
> you have written data that hasn't yet been forced out of the stdio
> buffers then out-of-disk-space is certainly a foreseeable failure.

Yes. I think I brought that up in my original email. Heap access/WAL 
routines 'should not' suffer an fclose() problem because of
fsync() calls. But this isn't necessarily the case for COPY.

> 
> fclose failure on an open-for-read-only file seems like Assert()
> material; it "can't happen".

Right. If this generates an error, there are probably more serious issues.

Gavin


Sat, 31 May 2003 14:55:11 +1000 (EST)
Gavin Sherry <swm@linuxworld.com.au>
Re: Testing the return value of fclose() in the backend 

Added to TODO:

	* Add checks for fclose() failure


---------------------------------------------------------------------------

Gavin Sherry wrote:
> On Fri, 30 May 2003, Tom Lane wrote:
> 
> > Bruce Momjian <pgman@candle.pha.pa.us> writes:
> > > Gavin Sherry wrote:
> > >> There are various places in the backend, such as FreeFile(), where the
> > >> return value of fclose() is not tested.
> > 
> > > We are not checking fclose, probably because fclose failures are quite
> > > rare.  Should we be concerned?
> > 
> > Probably.  Closing a valid file descriptor in itself can't provoke any
> > error that I can imagine, but fclose() also implies fflush() --- so if
> > you have written data that hasn't yet been forced out of the stdio
> > buffers then out-of-disk-space is certainly a foreseeable failure.
> 
> Yes. I think I brought that up in my original email. Heap access/WAL 
> routines 'should not' suffer an fclose() problem because of
> fsync() calls. But this isn't necessarily the case for COPY.
> 
> > 
> > fclose failure on an open-for-read-only file seems like Assert()
> > material; it "can't happen".
> 
> Right. If this generates an error, there are probably more serious issues.
> 
> Gavin
> 
> 
> ---------------------------(end of broadcast)---------------------------
> TIP 5: Have you checked our extensive FAQ?
> 
> http://www.postgresql.org/docs/faqs/FAQ.html
> 

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Fri, 6 Jun 2003 11:55:28 -0400 (EDT)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: Testing the return value of fclose() in the backend