On Thu, Feb 13, 2020 at 10:02:31AM +0100, Peter Eisentraut wrote: > On 2020-02-12 06:28, Michael Paquier wrote: >> Now, don't we need to care about durable_rename() and make the >> panic-like failure an optional choice? From what I can see, this >> routine is used now in the backend for pg_basebackup to rename >> temporary history files or partial WAL segments. > > durable_rename() calls fsync_fname(), so it would be covered by this change. > The other file access calls in there can be handled by normal error > handling, I think. Is there any specific scenario you have in mind?
The old file flush is handled by your patch, but not the new one if it exists, and it seems to me that we should handle failures consistently to reason easier about it, actually as the top of the function says :) Another point that we could consider is if fsync_fname() should have an option to not trigger an immediate exit when facing a failure. The backend has that option thanks to fsync_fname_ext() with its elevel argument. Your choice to default to a failure is fine for most cases because that's what we want. However, I am questioning if this change would be surprising for some client applications or not, and if we should have the option to choose one behavior or the other. -- Michael
signature.asc
Description: PGP signature
