Michael Paquier wrote: > A typo s/pre_fsync_fname/pre_sync_fname, and a mistake from me because > I did not compile with -DPG_FLUSH_DATA_WORKS to check this code. > > v2 is attached, fixing those issues.
The patch applies and compiles fine. I have tested it on Linux and MinGW and could see the fsync(2) and FlushFileBuffers calls I expected. This adds crash safety for a reasonable price, and I think we should have that. The documentation additions are sufficient. Looking through the patch, I had two questions that are more about style and consistency than anything else: - In pg_dumpall.c, the result of fsync_fname() is cast to "void" to show that the return code is ignored, but not anywhere else. Is that by design? - For pg_dumpall, a short option "-N" is added for "--no-sync", but not for pg_dump (because -N is already taken there). I'd opt for either using the same short option for both or (IMO better) only offering a long option for both. This would avoid confusion, and we expect that few people will want to use this option anyway, right? Yours, Laurenz Albe -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
