Re: [PATCH] enable core.fsyncObjectFiles by default

From: Christoph Hellwig <hch@lst.de>
Date: Jan 17, 2018, 21:10 UTC
Message-ID: <20180117211011.GA355@lst.de>
In-Reply-To: <20180117205509.GA14828@sigill.intra.peff.net>

On Wed, Jan 17, 2018 at 03:55:09PM -0500, Jeff King wrote:

Show 8 quoted lines

> I'm definitely sympathetic, and I've contemplated a patch like this a
> few times. But I'm not sure we're "safe by default" here after this
> patch. In particular:
> 
>   1. This covers only loose objects. We generally sync pack writes
>      already, so we're covered there. But we do not sync ref updates at
>      all, which we'd probably want to in a default-safe setup (a common
>      post-crash symptom I've seen is zero-length ref files).

I've not seen them myself yet, but yes, they need an fsync.

>   2. Is it sufficient to fsync() the individual file's descriptors?
>      We often do other filesystem operations (like hardlinking or
>      renaming) that also need to be committed to disk before an
>      operation can be considered saved.

No, for metadata operations we need to fsync the directory as well.

Show 14 quoted lines

>   3. Related to (2), we often care about the order of metadata commits.
>      E.g., a common sequence is:
> 
>        a. Write object contents to tempfile.
> 
>        b. rename() or hardlink tempfile to final name.
> 
>        c. Write object name into ref.lock file.
> 
>        d. rename() ref.lock to ref
> 
>      If we see (d) but not (b), then the result is a corrupted
>      repository. Is this guaranteed by ext4 journaling with
>      data=ordered?

It is not generally guranteed by Linux file system semantics. Various file system will actually start writeback of file data before rename, but not actually wait on it.