Re: [PATCH] Enable core.fsyncObjectFiles by default

On Tue, Jun 23, 2015 at 10:32:08PM -0700, Junio C Hamano wrote:

Regarding loose object files, given that we write to a temporary,
optionally fsync, close and then move to the final name, would we
still see partially written file if we omit the fsync, or would the
corruption be limited to either empty or missing?

*Most* of the time the corruption will be an empty or missing file.
It's possible that the file could be partially written.  This is a
relatively low-probability event, with the probability going up if the
object file is large, and/or if the system is under memory pressure.

The reason I am wondering is because the codepath to create an
object (i.e. "update-index --add", "hash-object -w", or "add") first
checks if a packed or a loose object file _exists_ and if so
bypasses writing the same thing anew, but the existence check for a
loose object is to merely making sure that access(F_OK) (and
optionally utime()) succeeds.  If the potential breakage is limited
to truncation to empty, then we could replace it with stat(2) and
st.st_size check, as no loose object file can be empty.

It would certainly be a good thing to do a st_size check; it can't
possible hurt, and it will catch a large number of failures after a
power failure.  I could also imagine some hueristics that force an
fsync if the object file is larger than a certain size (say, 4k if you
are very paranoid, a few hundred kilobytes if you are less so), but
past a certain point, it might be better just to tell the user to use
fsyncObjectFiles and be done with it.

						- Ted