reftable/stack: provide fsync(3p) via system header

Users of the reftable library are expected to provide their own function
callback in cases they want to sync(3p) data to disk via the reftable
write options. But if no such function was provided we end up calling
fsync(3p) directly, which may not even be available on some systems.

While dropping the explicit call to fsync(3p) would work, it would lead
to an unsafe default behaviour where a project may have forgotten to set
up the callback function, and that could lead to potential data loss. So
this is not a great solution.

Instead, drop the callback function and make it mandatory for the
project to define fsync(3p). In the case of Git, we can then easily
inject our custom implementation via the "reftable-system.h" header so
that we continue to use `fsync_component()`.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c
index b124404663edf67931b191e7ad74cd04347586bf..daea30a5b4cad91314b40b51d56b49a37720fc63 100644 (file)
--- a/refs/reftable-backend.c
+++ b/refs/reftable-backend.c
@@ -366,11 +366,6 @@ static int reftable_be_config(const char *var, const char *value,
        return 0;
 }
 
-static int reftable_be_fsync(int fd)
-{
-       return fsync_component(FSYNC_COMPONENT_REFERENCE, fd);
-}
-
 static struct ref_store *reftable_be_init(struct repository *repo,
                                          const char *payload,
                                          const char *gitdir,
@@ -408,7 +403,6 @@ static struct ref_store *reftable_be_init(struct repository *repo,
        refs->write_options.disable_auto_compact =
                !git_env_bool("GIT_TEST_REFTABLE_AUTOCOMPACTION", 1);
        refs->write_options.lock_timeout_ms = 100;
-       refs->write_options.fsync = reftable_be_fsync;
 
        repo_config(the_repository, reftable_be_config, &refs->write_options);
 

diff --git a/reftable/reftable-system.h b/reftable/reftable-system.h
index 4a18a6a790d737a3c5fa8e2617fa889acc9c408b..76f3e33e90139777b6db885b150182454f0215dc 100644 (file)
--- a/reftable/reftable-system.h
+++ b/reftable/reftable-system.h
@@ -12,4 +12,7 @@
 #include "compat/posix.h"
 #include "compat/zlib-compat.h"
 
+int reftable_fsync(int fd);
+#define fsync(fd) reftable_fsync(fd)
+
 #endif

diff --git a/reftable/reftable-writer.h b/reftable/reftable-writer.h
index 065dd93dc6bc613dcd109f0e9b4d3302923248d0..a66db415c87637c3bf25004c919835f76074f93d 100644 (file)
--- a/reftable/reftable-writer.h
+++ b/reftable/reftable-writer.h
@@ -61,12 +61,6 @@ struct reftable_write_options {
         */
        long lock_timeout_ms;
 
-       /*
-        * Optional callback used to fsync files to disk. Falls back to using
-        * fsync(3P) when unset.
-        */
-       int (*fsync)(int fd);
-
        /*
         * Callback function to execute whenever the stack is being reloaded.
         * This can be used e.g. to discard cached information that relies on

diff --git a/reftable/stack.c b/reftable/stack.c
index 1c9f21dfe1eb45021836e1bb94d090a96b5fb5a6..fa87b46c377af2b3b8d484309ef7b682e050b862 100644 (file)
--- a/reftable/stack.c
+++ b/reftable/stack.c
@@ -29,13 +29,6 @@ static int stack_filename(struct reftable_buf *dest, struct reftable_stack *st,
        return 0;
 }
 
-static int stack_fsync(const struct reftable_write_options *opts, int fd)
-{
-       if (opts->fsync)
-               return opts->fsync(fd);
-       return fsync(fd);
-}
-
 static ssize_t reftable_write_data(int fd, const void *data, size_t size)
 {
        size_t total_written = 0;
@@ -69,7 +62,7 @@ static ssize_t fd_writer_write(void *arg, const void *data, size_t sz)
 static int fd_writer_flush(void *arg)
 {
        struct fd_writer *writer = arg;
-       return stack_fsync(writer->opts, writer->fd);
+       return fsync(writer->fd);
 }
 
 static int fd_read_lines(int fd, char ***namesp)
@@ -812,7 +805,7 @@ int reftable_addition_commit(struct reftable_addition *add)
                goto done;
        }
 
-       err = stack_fsync(&add->stack->opts, add->tables_list_lock.fd);
+       err = fsync(add->tables_list_lock.fd);
        if (err < 0) {
                err = REFTABLE_IO_ERROR;
                goto done;
@@ -1480,7 +1473,7 @@ static int stack_compact_range(struct reftable_stack *st,
                goto done;
        }
 
-       err = stack_fsync(&st->opts, tables_list_lock.fd);
+       err = fsync(tables_list_lock.fd);
        if (err < 0) {
                err = REFTABLE_IO_ERROR;
                unlink(new_table_path.buf);

diff --git a/reftable/system.c b/reftable/system.c
index 725a25844ea1797995d2f5f787ef32248dcbb0a9..4d7e366b55b6d6b5551732a74a4ac4189522dea1 100644 (file)
--- a/reftable/system.c
+++ b/reftable/system.c
@@ -5,6 +5,7 @@
 #include "reftable-error.h"
 #include "../lockfile.h"
 #include "../tempfile.h"
+#include "../write-or-die.h"
 
 uint32_t reftable_rand(void)
 {
@@ -131,3 +132,8 @@ int flock_commit(struct reftable_flock *l)
 
        return 0;
 }
+
+int reftable_fsync(int fd)
+{
+       return fsync_component(FSYNC_COMPONENT_REFERENCE, fd);
+}