Re: pg_basebackup, pg_receivexlog and data durability (was: silent data loss with ext4 / all current versions)

This is looking pretty good. More comments on this patch set:

0001:

Keep the file order alphabetical in Mkvcbuild.pm.

Needs nls.mk updates for file move (in initdb and pg_basebackup
directories).

0002:

durable_rename needs close(fd) in non-error path (compare backend code).

Changing from fsync() to fsync_name() in some cases means that
inaccessible files are now ignored. I guess this would only happen in
very obscure circumstances, but it's worth considering if that is OK.

You added this comment:

* XXX: This means that we might not restart if a crash occurs
before the
* fsync below. We probably should create the file in a temporary path
* like the backend does...

pg_receivexlog uses the .partial suffix for this reason. Why doesn't
pg_basebackup do that?

In open_walfile, could we move the fsync calls before the fstat or
somewhere around there so we don't have to make the same calls in two
different branches?

0003:

There was a discussion about renaming the --nosync option in initdb to
--no-sync, but until that is done, the option in pg_basebackup should
stay what initdb has right now.

There is a whitespace alignment error in usage().

The -N option should be listed after -n.

Some fsync calls are not covered by a do_sync conditional. I see some
in close_walfile(), HandleCopyStream(), ProcessKeepaliveMsg().

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services