Commit 1229e1c4 by Edward Thomson

fsync parent directories when fsyncing

When fsync'ing files, fsync the parent directory in the case where we
rename a file into place, or create a new file, to ensure that the
directory entry is flushed correctly.
parent 5a747e0c
...@@ -445,6 +445,9 @@ int git_filebuf_commit(git_filebuf *file) ...@@ -445,6 +445,9 @@ int git_filebuf_commit(git_filebuf *file)
goto on_error; goto on_error;
} }
if (file->do_fsync && git_futils_fsync_parent(file->path_original) < 0)
goto on_error;
file->did_rename = true; file->did_rename = true;
git_filebuf_cleanup(file); git_filebuf_cleanup(file);
......
...@@ -265,8 +265,13 @@ int git_futils_writebuffer( ...@@ -265,8 +265,13 @@ int git_futils_writebuffer(
return error; return error;
} }
if ((error = p_close(fd)) < 0) if ((error = p_close(fd)) < 0) {
giterr_set(GITERR_OS, "error while closing '%s'", path); 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; return error;
} }
...@@ -1119,3 +1124,28 @@ void git_futils_filestamp_set_from_stat( ...@@ -1119,3 +1124,28 @@ void git_futils_filestamp_set_from_stat(
memset(stamp, 0, sizeof(*stamp)); memset(stamp, 0, sizeof(*stamp));
} }
} }
int git_futils_fsync_dir(const char *path)
{
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;
}
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;
}
...@@ -363,4 +363,22 @@ extern void git_futils_filestamp_set( ...@@ -363,4 +363,22 @@ extern void git_futils_filestamp_set(
extern void git_futils_filestamp_set_from_stat( extern void git_futils_filestamp_set_from_stat(
git_futils_filestamp *stamp, struct stat *st); 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__ */ #endif /* INCLUDE_fileops_h__ */
...@@ -1086,7 +1086,14 @@ int git_indexer_commit(git_indexer *idx, git_transfer_progress *stats) ...@@ -1086,7 +1086,14 @@ int git_indexer_commit(git_indexer *idx, git_transfer_progress *stats)
goto on_error; goto on_error;
/* And don't forget to rename the packfile to its new place. */ /* 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 (git_object__synchronized_writing &&
git_futils_fsync_parent(git_buf_cstr(&filename)) < 0)
goto on_error;
idx->pack_committed = 1; idx->pack_committed = 1;
git_buf_free(&filename); git_buf_free(&filename);
......
...@@ -204,7 +204,7 @@ void test_pack_packbuilder__fsync_when_asked(void) ...@@ -204,7 +204,7 @@ void test_pack_packbuilder__fsync_when_asked(void)
p_fsync__cnt = 0; p_fsync__cnt = 0;
seed_packbuilder(); seed_packbuilder();
git_packbuilder_write(_packbuilder, ".", 0666, NULL, NULL); git_packbuilder_write(_packbuilder, ".", 0666, NULL, NULL);
cl_assert_equal_sz(2, p_fsync__cnt); cl_assert_equal_sz(4, p_fsync__cnt);
} }
static int foreach_cb(void *buf, size_t len, void *payload) static int foreach_cb(void *buf, size_t len, void *payload)
......
...@@ -329,7 +329,7 @@ void test_refs_create__fsyncs_when_requested(void) ...@@ -329,7 +329,7 @@ void test_refs_create__fsyncs_when_requested(void)
git_oid_fromstr(&id, current_master_tip); git_oid_fromstr(&id, current_master_tip);
cl_git_pass(git_reference_create(&ref, g_repo, "refs/heads/fsync_test", &id, 0, "log message")); cl_git_pass(git_reference_create(&ref, g_repo, "refs/heads/fsync_test", &id, 0, "log message"));
git_reference_free(ref); git_reference_free(ref);
cl_assert_equal_i(2, p_fsync__cnt); cl_assert_equal_i(4, p_fsync__cnt);
p_fsync__cnt = 0; p_fsync__cnt = 0;
...@@ -337,5 +337,5 @@ void test_refs_create__fsyncs_when_requested(void) ...@@ -337,5 +337,5 @@ void test_refs_create__fsyncs_when_requested(void)
cl_git_pass(git_refdb_compress(refdb)); cl_git_pass(git_refdb_compress(refdb));
git_refdb_free(refdb); git_refdb_free(refdb);
cl_assert_equal_i(1, p_fsync__cnt); cl_assert_equal_i(2, p_fsync__cnt);
} }
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment