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
anywhereelse. 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.
Thiswould avoid confusion, and we expect that few people will want to use this option anyway, right?
Yours,
Laurenz Albe