Merge pull request libgit2#4030 from libgit2/ethomson/fsync

fsync all the things

Author: ethomson

include/git2/common.h

@@ -179,6 +179,7 @@ typedef enum {
 	GIT_OPT_SET_SSL_CIPHERS,
 	GIT_OPT_GET_USER_AGENT,
 	GIT_OPT_ENABLE_OFS_DELTA,
+	GIT_OPT_ENABLE_SYNCHRONOUS_OBJECT_CREATION,
 } git_libgit2_opt_t;
 
 /**
@@ -316,6 +317,13 @@ typedef enum {
  *		> Packfiles containing offset deltas can still be read.
  *		> This defaults to enabled.
  *
+ *	* opts(GIT_OPT_ENABLE_SYNCHRONOUS_OBJECT_CREATION, int enabled)
+ *
+ *		> Enable synchronized writes of new objects using `fsync`
+ *		> (or the platform equivalent) to ensure that new object data
+ *		> is written to permanent storage, not simply cached.  This
+ *		> defaults to disabled.
+ *
  * @param option Option key
  * @param ... value to set the option
  * @return 0 on success, <0 on failure

include/git2/odb_backend.h

@@ -39,7 +39,7 @@ GIT_EXTERN(int) git_odb_backend_pack(git_odb_backend **out, const char *objects_
  * @param out location to store the odb backend pointer
  * @param objects_dir the Git repository's objects directory
  * @param compression_level zlib compression level to use
- * @param do_fsync whether to do an fsync() after writing (currently ignored)
+ * @param do_fsync whether to do an fsync() after writing
  * @param dir_mode permissions to use creating a directory or 0 for defaults
  * @param file_mode permissions to use creating a file or 0 for defaults
  *

src/config_cache.c

@@ -78,6 +78,7 @@ static struct map_data _cvar_maps[] = {
 	{"core.logallrefupdates", NULL, 0, GIT_LOGALLREFUPDATES_DEFAULT },
 	{"core.protecthfs", NULL, 0, GIT_PROTECTHFS_DEFAULT },
 	{"core.protectntfs", NULL, 0, GIT_PROTECTNTFS_DEFAULT },
+	{"core.fsyncobjectfiles", NULL, 0, GIT_FSYNCOBJECTFILES_DEFAULT },
 };
 
 int git_config__cvar(int *out, git_config *config, git_cvar_cached cvar)

src/filebuf.c

@@ -291,6 +291,9 @@ int git_filebuf_open_withsize(git_filebuf *file, const char *path, int flags, mo
 	if (flags & GIT_FILEBUF_DO_NOT_BUFFER)
 		file->do_not_buffer = true;
 
+	if (flags & GIT_FILEBUF_FSYNC)
+		file->do_fsync = true;
+
 	file->buf_size = size;
 	file->buf_pos = 0;
 	file->fd = -1;
@@ -425,6 +428,11 @@ int git_filebuf_commit(git_filebuf *file)
 
 	file->fd_is_open = false;
 
+	if (file->do_fsync && p_fsync(file->fd) < 0) {
+		giterr_set(GITERR_OS, "failed to fsync '%s'", file->path_lock);
+		goto on_error;
+	}
+
 	if (p_close(file->fd) < 0) {
 		giterr_set(GITERR_OS, "failed to close file at '%s'", file->path_lock);
 		goto on_error;
@@ -437,6 +445,9 @@ int git_filebuf_commit(git_filebuf *file)
 		goto on_error;
 	}
 
+	if (file->do_fsync && git_futils_fsync_parent(file->path_original) < 0)
+		goto on_error;
+
 	file->did_rename = true;
 
 	git_filebuf_cleanup(file);

src/filebuf.h

@@ -20,7 +20,8 @@
 #define GIT_FILEBUF_FORCE				(1 << 3)
 #define GIT_FILEBUF_TEMPORARY			(1 << 4)
 #define GIT_FILEBUF_DO_NOT_BUFFER		(1 << 5)
-#define GIT_FILEBUF_DEFLATE_SHIFT		(6)
+#define GIT_FILEBUF_FSYNC				(1 << 6)
+#define GIT_FILEBUF_DEFLATE_SHIFT		(7)
 
 #define GIT_FILELOCK_EXTENSION ".lock\0"
 #define GIT_FILELOCK_EXTLENGTH 6
@@ -47,6 +48,7 @@ struct git_filebuf {
 	bool created_lock;
 	bool did_rename;
 	bool do_not_buffer;
+	bool do_fsync;
 	int last_error;
 };
 

src/fileops.c

@@ -236,10 +236,16 @@ int git_futils_readbuffer(git_buf *buf, const char *path)
 int git_futils_writebuffer(
 	const git_buf *buf,	const char *path, int flags, mode_t mode)
 {
-	int fd, error = 0;
+	int fd, do_fsync = 0, error = 0;
 
-	if (flags <= 0)
+	if (!flags)
 		flags = O_CREAT | O_TRUNC | O_WRONLY;
+
+	if ((flags & O_FSYNC) != 0)
+		do_fsync = 1;
+
+	flags &= ~O_FSYNC;
+
 	if (!mode)
 		mode = GIT_FILEMODE_BLOB;
 
@@ -254,8 +260,19 @@ int git_futils_writebuffer(
 		return error;
 	}
 
-	if ((error = p_close(fd)) < 0)
+	if (do_fsync && (error = p_fsync(fd)) < 0) {
+		giterr_set(GITERR_OS, "could not fsync '%s'", path);
+		p_close(fd);
+		return error;
+	}
+
+	if ((error = p_close(fd)) < 0) {
 		giterr_set(GITERR_OS, "error while closing '%s'", path);
+		return error;
+	}
+
+	if (do_fsync && (flags & O_CREAT))
+		error = git_futils_fsync_parent(path);
 
 	return error;
 }
@@ -1108,3 +1125,33 @@ void git_futils_filestamp_set_from_stat(
 		memset(stamp, 0, sizeof(*stamp));
 	}
 }
+
+int git_futils_fsync_dir(const char *path)
+{
+#ifdef GIT_WIN32
+	GIT_UNUSED(path);
+	return 0;
+#else
+	int fd, error = -1;
+
+	if ((fd = p_open(path, O_RDONLY)) < 0) {
+		giterr_set(GITERR_OS, "failed to open directory '%s' for fsync", path);
+		return -1;
+	}
+
+	if ((error = p_fsync(fd)) < 0)
+		giterr_set(GITERR_OS, "failed to fsync directory '%s'", path);
+
+	p_close(fd);
+	return error;
+#endif
+}
+
+int git_futils_fsync_parent(const char *path)
+{
+	char *parent = git_path_dirname(path);
+	int error = git_futils_fsync_dir(parent);
+
+	git__free(parent);
+	return error;
+}

src/fileops.h

@@ -25,6 +25,13 @@ extern int git_futils_readbuffer_updated(
 	git_buf *obj, const char *path, git_oid *checksum, int *updated);
 extern int git_futils_readbuffer_fd(git_buf *obj, git_file fd, size_t len);
 
+/* Additional constants for `git_futils_writebuffer`'s `open_flags`.  We
+ * support these internally and they will be removed before the `open` call.
+ */
+#ifndef O_FSYNC
+# define O_FSYNC (1 << 31)
+#endif
+
 extern int git_futils_writebuffer(
 	const git_buf *buf, const char *path, int open_flags, mode_t mode);
 
@@ -356,4 +363,22 @@ extern void git_futils_filestamp_set(
 extern void git_futils_filestamp_set_from_stat(
 	git_futils_filestamp *stamp, struct stat *st);
 
+/**
+ * `fsync` the parent directory of the given path, if `fsync` is
+ * supported for directories on this platform.
+ *
+ * @param path Path of the directory to sync.
+ * @return 0 on success, -1 on error
+ */
+extern int git_futils_fsync_dir(const char *path);
+
+/**
+ * `fsync` the parent directory of the given path, if `fsync` is
+ * supported for directories on this platform.
+ *
+ * @param path Path of the file whose parent directory should be synced.
+ * @return 0 on success, -1 on error
+ */
+extern int git_futils_fsync_parent(const char *path);
+
 #endif /* INCLUDE_fileops_h__ */

src/indexer.c

@@ -17,6 +17,7 @@
 #include "oid.h"
 #include "oidmap.h"
 #include "zstream.h"
+#include "object.h"
 
 extern git_mutex git__mwindow_mutex;
 
@@ -33,7 +34,8 @@ struct git_indexer {
 	unsigned int parsed_header :1,
 		pack_committed :1,
 		have_stream :1,
-		have_delta :1;
+		have_delta :1,
+		do_fsync :1;
 	struct git_pack_header hdr;
 	struct git_pack_file *pack;
 	unsigned int mode;
@@ -123,6 +125,9 @@ int git_indexer_new(
 	git_hash_ctx_init(&idx->hash_ctx);
 	git_hash_ctx_init(&idx->trailer);
 
+	if (git_object__synchronous_writing)
+		idx->do_fsync = 1;
+
 	error = git_buf_joinpath(&path, prefix, suff);
 	if (error < 0)
 		goto cleanup;
@@ -161,6 +166,11 @@ int git_indexer_new(
 	return -1;
 }
 
+void git_indexer__set_fsync(git_indexer *idx, int do_fsync)
+{
+	idx->do_fsync = !!do_fsync;
+}
+
 /* Try to store the delta so we can try to resolve it later */
 static int store_delta(git_indexer *idx)
 {
@@ -989,7 +999,9 @@ int git_indexer_commit(git_indexer *idx, git_transfer_progress *stats)
 		return -1;
 
 	if (git_filebuf_open(&index_file, filename.ptr,
-		GIT_FILEBUF_HASH_CONTENTS, idx->mode) < 0)
+		GIT_FILEBUF_HASH_CONTENTS |
+		(idx->do_fsync ? GIT_FILEBUF_FSYNC : 0),
+		idx->mode) < 0)
 		goto on_error;
 
 	/* Write out the header */
@@ -1066,6 +1078,11 @@ int git_indexer_commit(git_indexer *idx, git_transfer_progress *stats)
 		return -1;
 	}
 
+	if (idx->do_fsync && p_fsync(idx->pack->mwf.fd) < 0) {
+		giterr_set(GITERR_OS, "failed to fsync packfile");
+		goto on_error;
+	}
+
 	/* We need to close the descriptor here so Windows doesn't choke on commit_at */
 	if (p_close(idx->pack->mwf.fd) < 0) {
 		giterr_set(GITERR_OS, "failed to close packfile");
@@ -1078,7 +1095,14 @@ int git_indexer_commit(git_indexer *idx, git_transfer_progress *stats)
 		goto on_error;
 
 	/* And don't forget to rename the packfile to its new place. */
-	p_rename(idx->pack->pack_name, git_buf_cstr(&filename));
+	if (p_rename(idx->pack->pack_name, git_buf_cstr(&filename)) < 0)
+		goto on_error;
+
+	/* And fsync the parent directory if we're asked to. */
+	if (idx->do_fsync &&
+		git_futils_fsync_parent(git_buf_cstr(&filename)) < 0)
+		goto on_error;
+
 	idx->pack_committed = 1;
 
 	git_buf_free(&filename);

src/indexer.h

@@ -0,0 +1,12 @@
+/*
+ * Copyright (C) the libgit2 contributors. All rights reserved.
+ *
+ * This file is part of libgit2, distributed under the GNU GPL v2 with
+ * a Linking Exception. For full terms see the included COPYING file.
+ */
+#ifndef INCLUDE_indexer_h__
+#define INCLUDE_indexer_h__
+
+extern int git_indexer__set_fsync(git_indexer *idx, int do_fsync);
+
+#endif

src/object.c

@@ -16,6 +16,7 @@
 #include "tag.h"
 
 bool git_object__strict_input_validation = true;
+bool git_object__synchronous_writing = false;
 
 typedef struct {
 	const char	*str;	/* type name string */