[PATCH] Enable core.fsyncObjectFiles by default

Linus Torvalds started a discussion[1] if we want to play rather safe
than use defaults which make sense only for the most power users of Git:

So git is "safe" in the sense that you won't really lose any data,
but you may well be inconvenienced.  The "fsync each object" config
option is there in case you don't want that inconvenience, but it
should be noted that it can make for a hell of a performance impact.

Of course, it might well be the case that the actual default
might be worth turning around. Most git users probably don't
care about that kind of "apply two hundred patches from Andrew
Morton" kind of workload, although "rebase a big patch-series"
does end up doing basically the same thing, and might be more
common.

This patch enables fsync_object_files by default.

[1] https://plus.google.com/u/1/+JonathanCorbet/posts/JBxiKPe3VXa

Signed-off-by: Stefan Beller <redacted>
---
 Documentation/config.txt | 8 ++++----
 environment.c            | 2 +-
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 43bb53c..dce2640 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt

@@ -693,10 +693,10 @@ core.whitespace::
 core.fsyncObjectFiles::
 	This boolean will enable 'fsync()' when writing object files.
 +
-This is a total waste of time and effort on a filesystem that orders
-data writes properly, but can be useful for filesystems that do not use
-journalling (traditional UNIX filesystems) or that only journal metadata
-and not file contents (OS X's HFS+, or Linux ext3 with "data=writeback").
+This ensures objects are written to disk instead of relying on the
+operating systems cache and eventual write. Disabling this option will
+yield performance with a trade off in safety for repository corruption
+during power loss.
 
 core.preloadIndex::
 	Enable parallel index preload for operations like 'git diff'

diff --git a/environment.c b/environment.c
index 61c685b..b406f5e 100644
--- a/environment.c
+++ b/environment.c

@@ -35,7 +35,7 @@ const char *git_attributes_file;
 int zlib_compression_level = Z_BEST_SPEED;
 int core_compression_level;
 int core_compression_seen;
-int fsync_object_files;
+int fsync_object_files = 1;
 size_t packed_git_window_size = DEFAULT_PACKED_GIT_WINDOW_SIZE;
 size_t packed_git_limit = DEFAULT_PACKED_GIT_LIMIT;
 size_t delta_base_cache_limit = 96 * 1024 * 1024;

-- 
2.4.1.345.gab207b6.dirty

On Wed, Jun 24, 2015 at 4:57 AM, Stefan Beller [off-list ref] wrote:

Linus Torvalds started a discussion[1] if we want to play rather safe
than use defaults which make sense only for the most power users of Git:

quoted

So git is "safe" in the sense that you won't really lose any data,
but you may well be inconvenienced.  The "fsync each object" config
option is there in case you don't want that inconvenience, but it
should be noted that it can make for a hell of a performance impact.

quoted

Of course, it might well be the case that the actual default
might be worth turning around. Most git users probably don't
care about that kind of "apply two hundred patches from Andrew
Morton" kind of workload, although "rebase a big patch-series"
does end up doing basically the same thing, and might be more
common.

This patch enables fsync_object_files by default.

Will this make nfs performance a lot worse or still within acceptable range?
-- 
Duy

On Tue, Jun 23, 2015 at 02:57:23PM -0700, Stefan Beller wrote:

Linus Torvalds started a discussion[1] if we want to play rather safe
than use defaults which make sense only for the most power users of Git:

quoted

So git is "safe" in the sense that you won't really lose any data,
but you may well be inconvenienced.  The "fsync each object" config
option is there in case you don't want that inconvenience, but it
should be noted that it can make for a hell of a performance impact.

quoted

Of course, it might well be the case that the actual default
might be worth turning around. Most git users probably don't
care about that kind of "apply two hundred patches from Andrew
Morton" kind of workload, although "rebase a big patch-series"
does end up doing basically the same thing, and might be more
common.

This patch enables fsync_object_files by default.

If you are looking for safety out of the box, I think this falls far
short, as we do not fsync all of the other files. For instance, we do
not fsync refs before they are written (nor anything else that uses the
commit_lock_file() interface to rename, such as the index).  We do
always fsync packfiles and their indices.

I had always assumed this was fine on ext4 with data=ordered (i.e.,
either the rename and its pointed-to content will go through, or not; so
you either get your update or the old state, but not a garbage or empty
file). But it sounds from what Ted wrote in:

  http://article.gmane.org/gmane.linux.file-systems/97255

that this may not be the case. If it's not, I think we should consider
fsyncing ref writes.

-Peff