Re: [PATCH] Enable core.fsyncObjectFiles by default

[Theodore Ts'o <tytso@mit.edu>]

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