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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
