Re: [PATCH] Enable core.fsyncObjectFiles by default

From: Theodore Ts'o <tytso@mit.edu>
Date: Jun 24, 2015, 14:30 UTC
Message-ID: <20150624143007.GC14324@thunk.org>
In-Reply-To: <xmqqegl1brjb.fsf@gitster.dls.corp.google.com>

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

Show 5 quoted lines

> 
> 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.

Show 8 quoted lines

> 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