Thread: Testing the return value of fclose() in the backend
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
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, Pennsylvania19073
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
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
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
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, Pennsylvania19073