diff --git a/include/git2/common.h b/include/git2/common.h index f13dfd509..c909f86ca 100644 --- a/include/git2/common.h +++ b/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 diff --git a/include/git2/odb_backend.h b/include/git2/odb_backend.h index b17cfd8ba..9199538ce 100644 --- a/include/git2/odb_backend.h +++ b/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 * diff --git a/src/config_cache.c b/src/config_cache.c index dbea871b9..840722274 100644 --- a/src/config_cache.c +++ b/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) diff --git a/src/filebuf.c b/src/filebuf.c index 825b9c04c..80250ccdf 100644 --- a/src/filebuf.c +++ b/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); diff --git a/src/filebuf.h b/src/filebuf.h index 467708d45..c65aea780 100644 --- a/src/filebuf.h +++ b/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; }; diff --git a/src/fileops.c b/src/fileops.c index acb3b87e3..28f414f23 100644 --- a/src/fileops.c +++ b/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; +} diff --git a/src/fileops.h b/src/fileops.h index 65c96a6f5..46886b0d7 100644 --- a/src/fileops.h +++ b/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__ */ diff --git a/src/indexer.c b/src/indexer.c index 869e2299f..ce67240ce 100644 --- a/src/indexer.c +++ b/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 @@ cleanup: 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); diff --git a/src/indexer.h b/src/indexer.h new file mode 100644 index 000000000..702694bbf --- /dev/null +++ b/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 diff --git a/src/object.c b/src/object.c index 2da36a2ee..bd87c9310 100644 --- a/src/object.c +++ b/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 */ diff --git a/src/object.h b/src/object.h index dd227d16d..13117e4c3 100644 --- a/src/object.h +++ b/src/object.h @@ -10,6 +10,7 @@ #include "repository.h" extern bool git_object__strict_input_validation; +extern bool git_object__synchronous_writing; /** Base git object for inheritance */ struct git_object { diff --git a/src/odb.c b/src/odb.c index 3f2ac02f3..cf321f51e 100644 --- a/src/odb.c +++ b/src/odb.c @@ -496,7 +496,7 @@ int git_odb_get_backend(git_odb_backend **out, git_odb *odb, size_t pos) return GIT_ENOTFOUND; } -static int add_default_backends( +int git_odb__add_default_backends( git_odb *db, const char *objects_dir, bool as_alternates, int alternate_depth) { @@ -531,7 +531,7 @@ static int add_default_backends( #endif /* add the loose object backend */ - if (git_odb_backend_loose(&loose, objects_dir, -1, 0, 0, 0) < 0 || + if (git_odb_backend_loose(&loose, objects_dir, -1, db->do_fsync, 0, 0) < 0 || add_backend_internal(db, loose, GIT_LOOSE_PRIORITY, as_alternates, inode) < 0) return -1; @@ -586,7 +586,7 @@ static int load_alternates(git_odb *odb, const char *objects_dir, int alternate_ alternate = git_buf_cstr(&alternates_path); } - if ((result = add_default_backends(odb, alternate, true, alternate_depth + 1)) < 0) + if ((result = git_odb__add_default_backends(odb, alternate, true, alternate_depth + 1)) < 0) break; } @@ -598,7 +598,7 @@ static int load_alternates(git_odb *odb, const char *objects_dir, int alternate_ int git_odb_add_disk_alternate(git_odb *odb, const char *path) { - return add_default_backends(odb, path, true, 0); + return git_odb__add_default_backends(odb, path, true, 0); } int git_odb_open(git_odb **out, const char *objects_dir) @@ -612,7 +612,7 @@ int git_odb_open(git_odb **out, const char *objects_dir) if (git_odb_new(&db) < 0) return -1; - if (add_default_backends(db, objects_dir, 0, 0) < 0) { + if (git_odb__add_default_backends(db, objects_dir, 0, 0) < 0) { git_odb_free(db); return -1; } @@ -621,6 +621,24 @@ int git_odb_open(git_odb **out, const char *objects_dir) return 0; } +int git_odb__set_caps(git_odb *odb, int caps) +{ + if (caps == GIT_ODB_CAP_FROM_OWNER) { + git_repository *repo = odb->rc.owner; + int val; + + if (!repo) { + giterr_set(GITERR_ODB, "cannot access repository to set odb caps"); + return -1; + } + + if (!git_repository__cvar(&val, repo, GIT_CVAR_FSYNCOBJECTFILES)) + odb->do_fsync = !!val; + } + + return 0; +} + static void odb_free(git_odb *db) { size_t i; diff --git a/src/odb.h b/src/odb.h index 76e82f1dd..4f548bbba 100644 --- a/src/odb.h +++ b/src/odb.h @@ -38,8 +38,25 @@ struct git_odb { git_refcount rc; git_vector backends; git_cache own_cache; + unsigned int do_fsync :1; }; +typedef enum { + GIT_ODB_CAP_FROM_OWNER = -1, +} git_odb_cap_t; + +/* + * Set the capabilities for the object database. + */ +int git_odb__set_caps(git_odb *odb, int caps); + +/* + * Add the default loose and packed backends for a database. + */ +int git_odb__add_default_backends( + git_odb *db, const char *objects_dir, + bool as_alternates, int alternate_depth); + /* * Hash a git_rawobj internally. * The `git_rawobj` is supposed to be previously initialized diff --git a/src/odb_loose.c b/src/odb_loose.c index b2e2f1fb7..e14af4fab 100644 --- a/src/odb_loose.c +++ b/src/odb_loose.c @@ -14,6 +14,7 @@ #include "odb.h" #include "delta.h" #include "filebuf.h" +#include "object.h" #include "git2/odb_backend.h" #include "git2/types.h" @@ -838,6 +839,17 @@ static void loose_backend__stream_free(git_odb_stream *_stream) git__free(stream); } +static int filebuf_flags(loose_backend *backend) +{ + int flags = GIT_FILEBUF_TEMPORARY | + (backend->object_zlib_level << GIT_FILEBUF_DEFLATE_SHIFT); + + if (backend->fsync_object_files || git_object__synchronous_writing) + flags |= GIT_FILEBUF_FSYNC; + + return flags; +} + static int loose_backend__stream(git_odb_stream **stream_out, git_odb_backend *_backend, git_off_t length, git_otype type) { loose_backend *backend; @@ -864,9 +876,7 @@ static int loose_backend__stream(git_odb_stream **stream_out, git_odb_backend *_ stream->stream.mode = GIT_STREAM_WRONLY; if (git_buf_joinpath(&tmp_path, backend->objects_dir, "tmp_object") < 0 || - git_filebuf_open(&stream->fbuf, tmp_path.ptr, - GIT_FILEBUF_TEMPORARY | - (backend->object_zlib_level << GIT_FILEBUF_DEFLATE_SHIFT), + git_filebuf_open(&stream->fbuf, tmp_path.ptr, filebuf_flags(backend), backend->object_file_mode) < 0 || stream->stream.write((git_odb_stream *)stream, hdr, hdrlen) < 0) { @@ -894,9 +904,7 @@ static int loose_backend__write(git_odb_backend *_backend, const git_oid *oid, c header_len = git_odb__format_object_header(header, sizeof(header), len, type); if (git_buf_joinpath(&final_path, backend->objects_dir, "tmp_object") < 0 || - git_filebuf_open(&fbuf, final_path.ptr, - GIT_FILEBUF_TEMPORARY | - (backend->object_zlib_level << GIT_FILEBUF_DEFLATE_SHIFT), + git_filebuf_open(&fbuf, final_path.ptr, filebuf_flags(backend), backend->object_file_mode) < 0) { error = -1; diff --git a/src/pack-objects.c b/src/pack-objects.c index 58b7b94b3..ef272e8f5 100644 --- a/src/pack-objects.c +++ b/src/pack-objects.c @@ -1385,6 +1385,7 @@ int git_packbuilder_write( git_indexer *indexer; git_transfer_progress stats; struct pack_write_context ctx; + int t; PREPARE_PACK; @@ -1392,6 +1393,9 @@ int git_packbuilder_write( &indexer, path, mode, pb->odb, progress_cb, progress_cb_payload) < 0) return -1; + if (!git_repository__cvar(&t, pb->repo, GIT_CVAR_FSYNCOBJECTFILES) && t) + git_indexer__set_fsync(indexer, 1); + ctx.indexer = indexer; ctx.stats = &stats; diff --git a/src/pack-objects.h b/src/pack-objects.h index 5a84f4158..e1e0ee3c8 100644 --- a/src/pack-objects.h +++ b/src/pack-objects.h @@ -16,6 +16,7 @@ #include "netops.h" #include "zstream.h" #include "pool.h" +#include "indexer.h" #include "git2/oid.h" #include "git2/pack.h" diff --git a/src/posix.c b/src/posix.c index e68f324f6..94deb6ab0 100644 --- a/src/posix.c +++ b/src/posix.c @@ -10,6 +10,8 @@ #include #include +size_t p_fsync__cnt = 0; + #ifndef GIT_WIN32 #ifdef NO_ADDRINFO diff --git a/src/posix.h b/src/posix.h index f204751cf..bd5a98e26 100644 --- a/src/posix.h +++ b/src/posix.h @@ -111,6 +111,12 @@ extern int p_rename(const char *from, const char *to); extern int git__page_size(size_t *page_size); extern int git__mmap_alignment(size_t *page_size); +/* The number of times `p_fsync` has been called. Note that this is for + * test code only; it it not necessarily thread-safe and should not be + * relied upon in production. + */ +extern size_t p_fsync__cnt; + /** * Platform-dependent methods */ diff --git a/src/rebase.c b/src/rebase.c index bf1d086ba..f528031b3 100644 --- a/src/rebase.c +++ b/src/rebase.c @@ -447,8 +447,8 @@ static int rebase_setupfiles_merge(git_rebase *rebase) size_t i; int error = 0; - if ((error = rebase_setupfile(rebase, END_FILE, -1, "%" PRIuZ "\n", git_array_size(rebase->operations))) < 0 || - (error = rebase_setupfile(rebase, ONTO_NAME_FILE, -1, "%s\n", rebase->onto_name)) < 0) + if ((error = rebase_setupfile(rebase, END_FILE, 0, "%" PRIuZ "\n", git_array_size(rebase->operations))) < 0 || + (error = rebase_setupfile(rebase, ONTO_NAME_FILE, 0, "%s\n", rebase->onto_name)) < 0) goto done; for (i = 0; i < git_array_size(rebase->operations); i++) { @@ -459,7 +459,7 @@ static int rebase_setupfiles_merge(git_rebase *rebase) git_oid_fmt(id_str, &operation->id); - if ((error = rebase_setupfile(rebase, commit_filename.ptr, -1, + if ((error = rebase_setupfile(rebase, commit_filename.ptr, 0, "%.*s\n", GIT_OID_HEXSZ, id_str)) < 0) goto done; } @@ -486,10 +486,10 @@ static int rebase_setupfiles(git_rebase *rebase) rebase->orig_head_name; if (git_repository__set_orig_head(rebase->repo, &rebase->orig_head_id) < 0 || - rebase_setupfile(rebase, HEAD_NAME_FILE, -1, "%s\n", orig_head_name) < 0 || - rebase_setupfile(rebase, ONTO_FILE, -1, "%.*s\n", GIT_OID_HEXSZ, onto) < 0 || - rebase_setupfile(rebase, ORIG_HEAD_FILE, -1, "%.*s\n", GIT_OID_HEXSZ, orig_head) < 0 || - rebase_setupfile(rebase, QUIET_FILE, -1, rebase->quiet ? "t\n" : "\n") < 0) + rebase_setupfile(rebase, HEAD_NAME_FILE, 0, "%s\n", orig_head_name) < 0 || + rebase_setupfile(rebase, ONTO_FILE, 0, "%.*s\n", GIT_OID_HEXSZ, onto) < 0 || + rebase_setupfile(rebase, ORIG_HEAD_FILE, 0, "%.*s\n", GIT_OID_HEXSZ, orig_head) < 0 || + rebase_setupfile(rebase, QUIET_FILE, 0, rebase->quiet ? "t\n" : "\n") < 0) return -1; return rebase_setupfiles_merge(rebase); @@ -823,8 +823,8 @@ static int rebase_next_merge( normalize_checkout_options_for_apply(&checkout_opts, rebase, current_commit); if ((error = git_indexwriter_init_for_operation(&indexwriter, rebase->repo, &checkout_opts.checkout_strategy)) < 0 || - (error = rebase_setupfile(rebase, MSGNUM_FILE, -1, "%" PRIuZ "\n", rebase->current+1)) < 0 || - (error = rebase_setupfile(rebase, CURRENT_FILE, -1, "%.*s\n", GIT_OID_HEXSZ, current_idstr)) < 0 || + (error = rebase_setupfile(rebase, MSGNUM_FILE, 0, "%" PRIuZ "\n", rebase->current+1)) < 0 || + (error = rebase_setupfile(rebase, CURRENT_FILE, 0, "%.*s\n", GIT_OID_HEXSZ, current_idstr)) < 0 || (error = git_merge_trees(&index, rebase->repo, parent_tree, head_tree, current_tree, &rebase->options.merge_options)) < 0 || (error = git_merge__check_result(rebase->repo, index)) < 0 || (error = git_checkout_index(rebase->repo, index, &checkout_opts)) < 0 || diff --git a/src/refdb_fs.c b/src/refdb_fs.c index ac5a6a6a5..450b3f3ec 100644 --- a/src/refdb_fs.c +++ b/src/refdb_fs.c @@ -62,6 +62,7 @@ typedef struct refdb_fs_backend { int peeling_mode; git_iterator_flag_t iterator_flags; uint32_t direach_flags; + int fsync; } refdb_fs_backend; static int refdb_reflog_fs__delete(git_refdb_backend *_backend, const char *name); @@ -736,7 +737,7 @@ static int reference_path_available( static int loose_lock(git_filebuf *file, refdb_fs_backend *backend, const char *name) { - int error; + int error, filebuf_flags; git_buf ref_path = GIT_BUF_INIT; assert(file && backend && name); @@ -755,7 +756,11 @@ static int loose_lock(git_filebuf *file, refdb_fs_backend *backend, const char * if (git_buf_joinpath(&ref_path, backend->gitpath, name) < 0) return -1; - error = git_filebuf_open(file, ref_path.ptr, GIT_FILEBUF_FORCE, GIT_REFS_FILE_MODE); + filebuf_flags = GIT_FILEBUF_FORCE; + if (backend->fsync) + filebuf_flags |= GIT_FILEBUF_FSYNC; + + error = git_filebuf_open(file, ref_path.ptr, filebuf_flags, GIT_REFS_FILE_MODE); if (error == GIT_EDIRECTORY) giterr_set(GITERR_REFERENCE, "cannot lock ref '%s', there are refs beneath that folder", name); @@ -990,15 +995,18 @@ static int packed_write(refdb_fs_backend *backend) { git_sortedcache *refcache = backend->refcache; git_filebuf pack_file = GIT_FILEBUF_INIT; - int error; + int error, open_flags = 0; size_t i; /* lock the cache to updates while we do this */ if ((error = git_sortedcache_wlock(refcache)) < 0) return error; + if (backend->fsync) + open_flags = GIT_FILEBUF_FSYNC; + /* Open the file! */ - if ((error = git_filebuf_open(&pack_file, git_sortedcache_path(refcache), 0, GIT_PACKEDREFS_FILE_MODE)) < 0) + if ((error = git_filebuf_open(&pack_file, git_sortedcache_path(refcache), open_flags, GIT_PACKEDREFS_FILE_MODE)) < 0) goto fail; /* Packfiles have a header... apparently @@ -1786,7 +1794,7 @@ success: /* Append to the reflog, must be called under reference lock */ static int reflog_append(refdb_fs_backend *backend, const git_reference *ref, const git_oid *old, const git_oid *new, const git_signature *who, const char *message) { - int error, is_symbolic; + int error, is_symbolic, open_flags; git_oid old_id = {{0}}, new_id = {{0}}; git_buf buf = GIT_BUF_INIT, path = GIT_BUF_INIT; git_repository *repo = backend->repo; @@ -1854,7 +1862,12 @@ static int reflog_append(refdb_fs_backend *backend, const git_reference *ref, co goto cleanup; } - error = git_futils_writebuffer(&buf, git_buf_cstr(&path), O_WRONLY|O_CREAT|O_APPEND, GIT_REFLOG_FILE_MODE); + open_flags = O_WRONLY | O_CREAT | O_APPEND; + + if (backend->fsync) + open_flags |= O_FSYNC; + + error = git_futils_writebuffer(&buf, git_buf_cstr(&path), open_flags, GIT_REFLOG_FILE_MODE); cleanup: git_buf_free(&buf); @@ -2011,6 +2024,9 @@ int git_refdb_backend_fs( backend->iterator_flags |= GIT_ITERATOR_PRECOMPOSE_UNICODE; backend->direach_flags |= GIT_PATH_DIR_PRECOMPOSE_UNICODE; } + if ((!git_repository__cvar(&t, backend->repo, GIT_CVAR_FSYNCOBJECTFILES) && t) || + git_object__synchronous_writing) + backend->fsync = 1; backend->parent.exists = &refdb_fs_backend__exists; backend->parent.lookup = &refdb_fs_backend__lookup; diff --git a/src/repository.c b/src/repository.c index 0db481638..425ef796f 100644 --- a/src/repository.c +++ b/src/repository.c @@ -1055,18 +1055,22 @@ int git_repository_odb__weakptr(git_odb **out, git_repository *repo) git_odb *odb; if ((error = git_repository_item_path(&odb_path, repo, - GIT_REPOSITORY_ITEM_OBJECTS)) < 0) + GIT_REPOSITORY_ITEM_OBJECTS)) < 0 || + (error = git_odb_new(&odb)) < 0) return error; - error = git_odb_open(&odb, odb_path.ptr); - if (!error) { - GIT_REFCOUNT_OWN(odb, repo); + GIT_REFCOUNT_OWN(odb, repo); - odb = git__compare_and_swap(&repo->_odb, NULL, odb); - if (odb != NULL) { - GIT_REFCOUNT_OWN(odb, NULL); - git_odb_free(odb); - } + if ((error = git_odb__set_caps(odb, GIT_ODB_CAP_FROM_OWNER)) < 0 || + (error = git_odb__add_default_backends(odb, odb_path.ptr, 0, 0)) < 0) { + git_odb_free(odb); + return error; + } + + odb = git__compare_and_swap(&repo->_odb, NULL, odb); + if (odb != NULL) { + GIT_REFCOUNT_OWN(odb, NULL); + git_odb_free(odb); } git_buf_free(&odb_path); diff --git a/src/repository.h b/src/repository.h index c328ecd21..33adfa60a 100644 --- a/src/repository.h +++ b/src/repository.h @@ -46,6 +46,7 @@ typedef enum { GIT_CVAR_LOGALLREFUPDATES, /* core.logallrefupdates */ GIT_CVAR_PROTECTHFS, /* core.protectHFS */ GIT_CVAR_PROTECTNTFS, /* core.protectNTFS */ + GIT_CVAR_FSYNCOBJECTFILES, /* core.fsyncObjectFiles */ GIT_CVAR_CACHE_MAX } git_cvar_cached; @@ -106,6 +107,8 @@ typedef enum { GIT_PROTECTHFS_DEFAULT = GIT_CVAR_FALSE, /* core.protectNTFS */ GIT_PROTECTNTFS_DEFAULT = GIT_CVAR_FALSE, + /* core.fsyncObjectFiles */ + GIT_FSYNCOBJECTFILES_DEFAULT = GIT_CVAR_FALSE, } git_cvar_value; /* internal repository init flags */ diff --git a/src/settings.c b/src/settings.c index 21585672c..24e549ec1 100644 --- a/src/settings.c +++ b/src/settings.c @@ -227,6 +227,10 @@ int git_libgit2_opts(int key, ...) git_smart__ofs_delta_enabled = (va_arg(ap, int) != 0); break; + case GIT_OPT_ENABLE_SYNCHRONOUS_OBJECT_CREATION: + git_object__synchronous_writing = (va_arg(ap, int) != 0); + break; + default: giterr_set(GITERR_INVALID, "invalid option key"); error = -1; diff --git a/src/unix/posix.h b/src/unix/posix.h index b4786403f..52985fd8a 100644 --- a/src/unix/posix.h +++ b/src/unix/posix.h @@ -40,9 +40,14 @@ typedef int GIT_SOCKET; #define p_link(o,n) link(o, n) #define p_unlink(p) unlink(p) #define p_mkdir(p,m) mkdir(p, m) -#define p_fsync(fd) fsync(fd) extern char *p_realpath(const char *, char *); +GIT_INLINE(int) p_fsync(int fd) +{ + p_fsync__cnt++; + return fsync(fd); +} + #define p_recv(s,b,l,f) recv(s,b,l,f) #define p_send(s,b,l,f) send(s,b,l,f) #define p_inet_pton(a, b, c) inet_pton(a, b, c) diff --git a/src/win32/posix_w32.c b/src/win32/posix_w32.c index fea634b00..5172627b0 100644 --- a/src/win32/posix_w32.c +++ b/src/win32/posix_w32.c @@ -113,6 +113,8 @@ int p_fsync(int fd) { HANDLE fh = (HANDLE)_get_osfhandle(fd); + p_fsync__cnt++; + if (fh == INVALID_HANDLE_VALUE) { errno = EBADF; return -1; diff --git a/tests/odb/loose.c b/tests/odb/loose.c index c91927c4a..dd686aa01 100644 --- a/tests/odb/loose.c +++ b/tests/odb/loose.c @@ -3,6 +3,7 @@ #include "git2/odb_backend.h" #include "posix.h" #include "loose_data.h" +#include "repository.h" #ifdef __ANDROID_API__ # define S_IREAD S_IRUSR @@ -56,11 +57,13 @@ static void test_read_object(object_data *data) void test_odb_loose__initialize(void) { + p_fsync__cnt = 0; cl_must_pass(p_mkdir("test-objects", GIT_OBJECT_DIR_MODE)); } void test_odb_loose__cleanup(void) { + cl_git_pass(git_libgit2_opts(GIT_OPT_ENABLE_SYNCHRONOUS_OBJECT_CREATION, 0)); cl_fixture_cleanup("test-objects"); } @@ -150,3 +153,55 @@ void test_odb_loose__permissions_readwrite(void) { test_write_object_permission(0777, 0666, 0777, 0666); } + +static void write_object_to_loose_odb(int fsync) +{ + git_odb *odb; + git_odb_backend *backend; + git_oid oid; + + cl_git_pass(git_odb_new(&odb)); + cl_git_pass(git_odb_backend_loose(&backend, "test-objects", -1, fsync, 0777, 0666)); + cl_git_pass(git_odb_add_backend(odb, backend, 1)); + cl_git_pass(git_odb_write(&oid, odb, "Test data\n", 10, GIT_OBJ_BLOB)); + git_odb_free(odb); +} + +void test_odb_loose__does_not_fsync_by_default(void) +{ + write_object_to_loose_odb(0); + cl_assert_equal_sz(0, p_fsync__cnt); +} + +void test_odb_loose__fsync_obeys_odb_option(void) +{ + write_object_to_loose_odb(1); + cl_assert(p_fsync__cnt > 0); +} + +void test_odb_loose__fsync_obeys_global_setting(void) +{ + cl_git_pass(git_libgit2_opts(GIT_OPT_ENABLE_SYNCHRONOUS_OBJECT_CREATION, 1)); + write_object_to_loose_odb(0); + cl_assert(p_fsync__cnt > 0); +} + +void test_odb_loose__fsync_obeys_repo_setting(void) +{ + git_repository *repo; + git_odb *odb; + git_oid oid; + + cl_git_pass(git_repository_init(&repo, "test-objects", 1)); + cl_git_pass(git_repository_odb__weakptr(&odb, repo)); + cl_git_pass(git_odb_write(&oid, odb, "No fsync here\n", 14, GIT_OBJ_BLOB)); + cl_assert(p_fsync__cnt == 0); + git_repository_free(repo); + + cl_git_pass(git_repository_open(&repo, "test-objects")); + cl_repo_set_bool(repo, "core.fsyncObjectFiles", true); + cl_git_pass(git_repository_odb__weakptr(&odb, repo)); + cl_git_pass(git_odb_write(&oid, odb, "Now fsync\n", 10, GIT_OBJ_BLOB)); + cl_assert(p_fsync__cnt > 0); + git_repository_free(repo); +} diff --git a/tests/pack/packbuilder.c b/tests/pack/packbuilder.c index 29f3e2d64..1d7becef7 100644 --- a/tests/pack/packbuilder.c +++ b/tests/pack/packbuilder.c @@ -23,6 +23,7 @@ void test_pack_packbuilder__initialize(void) cl_git_pass(git_vector_init(&_commits, 0, NULL)); _commits_is_initialized = 1; memset(&_stats, 0, sizeof(_stats)); + p_fsync__cnt = 0; } void test_pack_packbuilder__cleanup(void) @@ -30,6 +31,8 @@ void test_pack_packbuilder__cleanup(void) git_oid *o; unsigned int i; + cl_git_pass(git_libgit2_opts(GIT_OPT_ENABLE_SYNCHRONOUS_OBJECT_CREATION, 0)); + if (_commits_is_initialized) { _commits_is_initialized = 0; git_vector_foreach(&_commits, i, o) { @@ -188,6 +191,40 @@ void test_pack_packbuilder__permissions_readwrite(void) test_write_pack_permission(0666, 0666); } +void test_pack_packbuilder__does_not_fsync_by_default(void) +{ + seed_packbuilder(); + git_packbuilder_write(_packbuilder, ".", 0666, NULL, NULL); + cl_assert_equal_sz(0, p_fsync__cnt); +} + +/* We fsync the packfile and index. On non-Windows, we also fsync + * the parent directories. + */ +#ifdef GIT_WIN32 +static int expected_fsyncs = 2; +#else +static int expected_fsyncs = 4; +#endif + +void test_pack_packbuilder__fsync_global_setting(void) +{ + cl_git_pass(git_libgit2_opts(GIT_OPT_ENABLE_SYNCHRONOUS_OBJECT_CREATION, 1)); + p_fsync__cnt = 0; + seed_packbuilder(); + git_packbuilder_write(_packbuilder, ".", 0666, NULL, NULL); + cl_assert_equal_sz(expected_fsyncs, p_fsync__cnt); +} + +void test_pack_packbuilder__fsync_repo_setting(void) +{ + cl_repo_set_bool(_repo, "core.fsyncObjectFiles", true); + p_fsync__cnt = 0; + seed_packbuilder(); + git_packbuilder_write(_packbuilder, ".", 0666, NULL, NULL); + cl_assert_equal_sz(expected_fsyncs, p_fsync__cnt); +} + static int foreach_cb(void *buf, size_t len, void *payload) { git_indexer *idx = (git_indexer *) payload; diff --git a/tests/refs/create.c b/tests/refs/create.c index aca808c8c..4ecc60565 100644 --- a/tests/refs/create.c +++ b/tests/refs/create.c @@ -13,6 +13,7 @@ static git_repository *g_repo; void test_refs_create__initialize(void) { g_repo = cl_git_sandbox_init("testrepo"); + p_fsync__cnt = 0; } void test_refs_create__cleanup(void) @@ -21,6 +22,7 @@ void test_refs_create__cleanup(void) cl_git_pass(git_libgit2_opts(GIT_OPT_ENABLE_STRICT_OBJECT_CREATION, 1)); cl_git_pass(git_libgit2_opts(GIT_OPT_ENABLE_STRICT_SYMBOLIC_REF_CREATION, 1)); + cl_git_pass(git_libgit2_opts(GIT_OPT_ENABLE_SYNCHRONOUS_OBJECT_CREATION, 0)); } void test_refs_create__symbolic(void) @@ -297,3 +299,69 @@ void test_refs_create__creating_a_loose_ref_with_invalid_windows_name(void) test_win32_name("refs/heads/com1"); } + +/* Creating a loose ref involves fsync'ing the reference, the + * reflog and (on non-Windows) the containing directories. + * Creating a packed ref involves fsync'ing the packed ref file + * and (on non-Windows) the containing directory. + */ +#ifdef GIT_WIN32 +static int expected_fsyncs_create = 2, expected_fsyncs_compress = 1; +#else +static int expected_fsyncs_create = 4, expected_fsyncs_compress = 2; +#endif + +static void count_fsyncs(size_t *create_count, size_t *compress_count) +{ + git_reference *ref = NULL; + git_refdb *refdb; + git_oid id; + + p_fsync__cnt = 0; + + git_oid_fromstr(&id, current_master_tip); + cl_git_pass(git_reference_create(&ref, g_repo, "refs/heads/fsync_test", &id, 0, "log message")); + git_reference_free(ref); + + *create_count = p_fsync__cnt; + p_fsync__cnt = 0; + + cl_git_pass(git_repository_refdb(&refdb, g_repo)); + cl_git_pass(git_refdb_compress(refdb)); + git_refdb_free(refdb); + + *compress_count = p_fsync__cnt; + p_fsync__cnt = 0; +} + +void test_refs_create__does_not_fsync_by_default(void) +{ + size_t create_count, compress_count; + count_fsyncs(&create_count, &compress_count); + + cl_assert_equal_i(0, create_count); + cl_assert_equal_i(0, compress_count); +} + +void test_refs_create__fsyncs_when_global_opt_set(void) +{ + size_t create_count, compress_count; + + cl_git_pass(git_libgit2_opts(GIT_OPT_ENABLE_SYNCHRONOUS_OBJECT_CREATION, 1)); + count_fsyncs(&create_count, &compress_count); + + cl_assert_equal_i(expected_fsyncs_create, create_count); + cl_assert_equal_i(expected_fsyncs_compress, compress_count); +} + +void test_refs_create__fsyncs_when_repo_config_set(void) +{ + size_t create_count, compress_count; + + cl_repo_set_bool(g_repo, "core.fsyncObjectFiles", true); + + count_fsyncs(&create_count, &compress_count); + + cl_assert_equal_i(expected_fsyncs_create, create_count); + cl_assert_equal_i(expected_fsyncs_compress, compress_count); +}