reftable: honor core.fsync

While the reffiles backend honors configured fsync settings, the
reftable backend does not. Address this by fsyncing reftable files using
the write-or-die api's fsync_component() in two places: when we
add additional entries into the table, and when we close the reftable
writer.

This commits adds a flush function pointer as a new member of
reftable_writer because we are not sure that the first argument to the
*write function pointer always contains a file descriptor. In the case of
strbuf_add_void, the first argument is a buffer. This way, we can pass
in a corresponding flush function that knows how to flush depending on
which writer is being used.

This patch does not contain tests as they will need to wait for another
patch to start to exercise the reftable backend. At that point, the
tests will be added to observe that fsyncs are happening when the
reftable is in use.

Signed-off-by: John Cai <johncai86@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/reftable/merged_test.c b/reftable/merged_test.c
index 46908f738f770f4be920a65df85b96178ddbc70d..bf090b474ed5c69b8ef973b86914f36fdd206318 100644 (file)
--- a/reftable/merged_test.c
+++ b/reftable/merged_test.c
@@ -42,7 +42,7 @@ static void write_test_table(struct strbuf *buf,
                }
        }
 
-       w = reftable_new_writer(&strbuf_add_void, buf, &opts);
+       w = reftable_new_writer(&strbuf_add_void, &noop_flush, buf, &opts);
        reftable_writer_set_limits(w, min, max);
 
        for (i = 0; i < n; i++) {
@@ -70,7 +70,7 @@ static void write_test_log_table(struct strbuf *buf,
                .exact_log_message = 1,
        };
        struct reftable_writer *w = NULL;
-       w = reftable_new_writer(&strbuf_add_void, buf, &opts);
+       w = reftable_new_writer(&strbuf_add_void, &noop_flush, buf, &opts);
        reftable_writer_set_limits(w, update_index, update_index);
 
        for (i = 0; i < n; i++) {
@@ -412,7 +412,7 @@ static void test_default_write_opts(void)
        struct reftable_write_options opts = { 0 };
        struct strbuf buf = STRBUF_INIT;
        struct reftable_writer *w =
-               reftable_new_writer(&strbuf_add_void, &buf, &opts);
+               reftable_new_writer(&strbuf_add_void, &noop_flush, &buf, &opts);
 
        struct reftable_ref_record rec = {
                .refname = "master",

diff --git a/reftable/readwrite_test.c b/reftable/readwrite_test.c
index b8a32240164d6a5c29789fb1730a630127f085d5..6b99daeaf2a9b912bbf70277413a486c19e06bbc 100644 (file)
--- a/reftable/readwrite_test.c
+++ b/reftable/readwrite_test.c
@@ -51,7 +51,7 @@ static void write_table(char ***names, struct strbuf *buf, int N,
                .hash_id = hash_id,
        };
        struct reftable_writer *w =
-               reftable_new_writer(&strbuf_add_void, buf, &opts);
+               reftable_new_writer(&strbuf_add_void, &noop_flush, buf, &opts);
        struct reftable_ref_record ref = { NULL };
        int i = 0, n;
        struct reftable_log_record log = { NULL };
@@ -130,7 +130,7 @@ static void test_log_buffer_size(void)
                                           .message = "commit: 9\n",
                                   } } };
        struct reftable_writer *w =
-               reftable_new_writer(&strbuf_add_void, &buf, &opts);
+               reftable_new_writer(&strbuf_add_void, &noop_flush, &buf, &opts);
 
        /* This tests buffer extension for log compression. Must use a random
           hash, to ensure that the compressed part is larger than the original.
@@ -171,7 +171,7 @@ static void test_log_overflow(void)
                                           .message = msg,
                                   } } };
        struct reftable_writer *w =
-               reftable_new_writer(&strbuf_add_void, &buf, &opts);
+               reftable_new_writer(&strbuf_add_void, &noop_flush, &buf, &opts);
 
        uint8_t hash1[GIT_SHA1_RAWSZ]  = {1}, hash2[GIT_SHA1_RAWSZ] = { 2 };
 
@@ -202,7 +202,7 @@ static void test_log_write_read(void)
        struct reftable_block_source source = { NULL };
        struct strbuf buf = STRBUF_INIT;
        struct reftable_writer *w =
-               reftable_new_writer(&strbuf_add_void, &buf, &opts);
+               reftable_new_writer(&strbuf_add_void, &noop_flush, &buf, &opts);
        const struct reftable_stats *stats = NULL;
        reftable_writer_set_limits(w, 0, N);
        for (i = 0; i < N; i++) {
@@ -294,7 +294,7 @@ static void test_log_zlib_corruption(void)
        struct reftable_block_source source = { 0 };
        struct strbuf buf = STRBUF_INIT;
        struct reftable_writer *w =
-               reftable_new_writer(&strbuf_add_void, &buf, &opts);
+               reftable_new_writer(&strbuf_add_void, &noop_flush, &buf, &opts);
        const struct reftable_stats *stats = NULL;
        uint8_t hash1[GIT_SHA1_RAWSZ] = { 1 };
        uint8_t hash2[GIT_SHA1_RAWSZ] = { 2 };
@@ -535,7 +535,7 @@ static void test_table_refs_for(int indexed)
 
        struct strbuf buf = STRBUF_INIT;
        struct reftable_writer *w =
-               reftable_new_writer(&strbuf_add_void, &buf, &opts);
+               reftable_new_writer(&strbuf_add_void, &noop_flush, &buf, &opts);
 
        struct reftable_iterator it = { NULL };
        int j;
@@ -628,7 +628,7 @@ static void test_write_empty_table(void)
        struct reftable_write_options opts = { 0 };
        struct strbuf buf = STRBUF_INIT;
        struct reftable_writer *w =
-               reftable_new_writer(&strbuf_add_void, &buf, &opts);
+               reftable_new_writer(&strbuf_add_void, &noop_flush, &buf, &opts);
        struct reftable_block_source source = { NULL };
        struct reftable_reader *rd = NULL;
        struct reftable_ref_record rec = { NULL };
@@ -666,7 +666,7 @@ static void test_write_object_id_min_length(void)
        };
        struct strbuf buf = STRBUF_INIT;
        struct reftable_writer *w =
-               reftable_new_writer(&strbuf_add_void, &buf, &opts);
+               reftable_new_writer(&strbuf_add_void, &noop_flush, &buf, &opts);
        struct reftable_ref_record ref = {
                .update_index = 1,
                .value_type = REFTABLE_REF_VAL1,
@@ -701,7 +701,7 @@ static void test_write_object_id_length(void)
        };
        struct strbuf buf = STRBUF_INIT;
        struct reftable_writer *w =
-               reftable_new_writer(&strbuf_add_void, &buf, &opts);
+               reftable_new_writer(&strbuf_add_void, &noop_flush, &buf, &opts);
        struct reftable_ref_record ref = {
                .update_index = 1,
                .value_type = REFTABLE_REF_VAL1,
@@ -735,7 +735,7 @@ static void test_write_empty_key(void)
        struct reftable_write_options opts = { 0 };
        struct strbuf buf = STRBUF_INIT;
        struct reftable_writer *w =
-               reftable_new_writer(&strbuf_add_void, &buf, &opts);
+               reftable_new_writer(&strbuf_add_void, &noop_flush, &buf, &opts);
        struct reftable_ref_record ref = {
                .refname = "",
                .update_index = 1,
@@ -758,7 +758,7 @@ static void test_write_key_order(void)
        struct reftable_write_options opts = { 0 };
        struct strbuf buf = STRBUF_INIT;
        struct reftable_writer *w =
-               reftable_new_writer(&strbuf_add_void, &buf, &opts);
+               reftable_new_writer(&strbuf_add_void, &noop_flush, &buf, &opts);
        struct reftable_ref_record refs[2] = {
                {
                        .refname = "b",
@@ -801,7 +801,7 @@ static void test_write_multiple_indices(void)
        struct reftable_reader *reader;
        int err, i;
 
-       writer = reftable_new_writer(&strbuf_add_void, &writer_buf, &opts);
+       writer = reftable_new_writer(&strbuf_add_void, &noop_flush, &writer_buf, &opts);
        reftable_writer_set_limits(writer, 1, 1);
        for (i = 0; i < 100; i++) {
                struct reftable_ref_record ref = {

diff --git a/reftable/refname_test.c b/reftable/refname_test.c
index 699e1aea4122d66a8a00451521f7d902858474c2..b9cc62554ea1569a66e8fc08be40e04452e3032c 100644 (file)
--- a/reftable/refname_test.c
+++ b/reftable/refname_test.c
@@ -30,7 +30,7 @@ static void test_conflict(void)
        struct reftable_write_options opts = { 0 };
        struct strbuf buf = STRBUF_INIT;
        struct reftable_writer *w =
-               reftable_new_writer(&strbuf_add_void, &buf, &opts);
+               reftable_new_writer(&strbuf_add_void, &noop_flush, &buf, &opts);
        struct reftable_ref_record rec = {
                .refname = "a/b",
                .value_type = REFTABLE_REF_SYMREF,

diff --git a/reftable/reftable-writer.h b/reftable/reftable-writer.h
index db8de197f6c42a0de203fc1fbca8174f7f4d2938..7c7cae5f99b7cd4c2be74707bb550366e1088a35 100644 (file)
--- a/reftable/reftable-writer.h
+++ b/reftable/reftable-writer.h
@@ -88,6 +88,7 @@ struct reftable_stats {
 /* reftable_new_writer creates a new writer */
 struct reftable_writer *
 reftable_new_writer(ssize_t (*writer_func)(void *, const void *, size_t),
+                   int (*flush_func)(void *),
                    void *writer_arg, struct reftable_write_options *opts);
 
 /* Set the range of update indices for the records we will add. When writing a

diff --git a/reftable/stack.c b/reftable/stack.c
index 7ffeb3ee1074bc59aa094fabbbce24d1f3cfaf9a..ab295341cc4de4357c6e4944f833c6b501323752 100644 (file)
--- a/reftable/stack.c
+++ b/reftable/stack.c
@@ -8,6 +8,7 @@ https://developers.google.com/open-source/licenses/bsd
 
 #include "stack.h"
 
+#include "../write-or-die.h"
 #include "system.h"
 #include "merged.h"
 #include "reader.h"
@@ -16,7 +17,6 @@ https://developers.google.com/open-source/licenses/bsd
 #include "reftable-record.h"
 #include "reftable-merged.h"
 #include "writer.h"
-
 #include "tempfile.h"
 
 static int stack_try_add(struct reftable_stack *st,
@@ -47,6 +47,13 @@ static ssize_t reftable_fd_write(void *arg, const void *data, size_t sz)
        return write_in_full(*fdp, data, sz);
 }
 
+static int reftable_fd_flush(void *arg)
+{
+       int *fdp = (int *)arg;
+
+       return fsync_component(FSYNC_COMPONENT_REFERENCE, *fdp);
+}
+
 int reftable_new_stack(struct reftable_stack **dest, const char *dir,
                       struct reftable_write_options config)
 {
@@ -545,6 +552,9 @@ int reftable_addition_commit(struct reftable_addition *add)
                goto done;
        }
 
+       fsync_component_or_die(FSYNC_COMPONENT_REFERENCE, lock_file_fd,
+                              get_tempfile_path(add->lock_file));
+
        err = rename_tempfile(&add->lock_file, add->stack->list_file);
        if (err < 0) {
                err = REFTABLE_IO_ERROR;
@@ -639,7 +649,7 @@ int reftable_addition_add(struct reftable_addition *add,
                        goto done;
                }
        }
-       wr = reftable_new_writer(reftable_fd_write, &tab_fd,
+       wr = reftable_new_writer(reftable_fd_write, reftable_fd_flush, &tab_fd,
                                 &add->stack->config);
        err = write_table(wr, arg);
        if (err < 0)
@@ -731,7 +741,7 @@ static int stack_compact_locked(struct reftable_stack *st, int first, int last,
        strbuf_addstr(temp_tab, ".temp.XXXXXX");
 
        tab_fd = mkstemp(temp_tab->buf);
-       wr = reftable_new_writer(reftable_fd_write, &tab_fd, &st->config);
+       wr = reftable_new_writer(reftable_fd_write, reftable_fd_flush, &tab_fd, &st->config);
 
        err = stack_write_compact(st, wr, first, last, config);
        if (err < 0)

diff --git a/reftable/test_framework.c b/reftable/test_framework.c
index 04044fc1a0fb5ffa143264527b882afb752fc2c6..4066924eee45dc6b4fe5b353f7fe54c9dc9f7d7e 100644 (file)
--- a/reftable/test_framework.c
+++ b/reftable/test_framework.c
@@ -20,3 +20,8 @@ ssize_t strbuf_add_void(void *b, const void *data, size_t sz)
        strbuf_add(b, data, sz);
        return sz;
 }
+
+int noop_flush(void *arg)
+{
+       return 0;
+}

diff --git a/reftable/test_framework.h b/reftable/test_framework.h
index ee44f735aea51d03fc3400a9340ccbd7af881f9e..687390f9c239a397652a9a4c9a0bbb7a3510d232 100644 (file)
--- a/reftable/test_framework.h
+++ b/reftable/test_framework.h
@@ -56,4 +56,6 @@ void set_test_hash(uint8_t *p, int i);
  */
 ssize_t strbuf_add_void(void *b, const void *data, size_t sz);
 
+int noop_flush(void *);
+
 #endif

diff --git a/reftable/writer.c b/reftable/writer.c
index ee4590e20f84dd442fde4f7506e27e568f9ad509..92935baa7036924de241a2adc8eca3640b0fc376 100644 (file)
--- a/reftable/writer.c
+++ b/reftable/writer.c
@@ -121,6 +121,7 @@ static struct strbuf reftable_empty_strbuf = STRBUF_INIT;
 
 struct reftable_writer *
 reftable_new_writer(ssize_t (*writer_func)(void *, const void *, size_t),
+                   int (*flush_func)(void *),
                    void *writer_arg, struct reftable_write_options *opts)
 {
        struct reftable_writer *wp =
@@ -136,6 +137,7 @@ reftable_new_writer(ssize_t (*writer_func)(void *, const void *, size_t),
        wp->write = writer_func;
        wp->write_arg = writer_arg;
        wp->opts = *opts;
+       wp->flush = flush_func;
        writer_reinit_block_writer(wp, BLOCK_TYPE_REF);
 
        return wp;
@@ -603,6 +605,12 @@ int reftable_writer_close(struct reftable_writer *w)
        put_be32(p, crc32(0, footer, p - footer));
        p += 4;
 
+       err = w->flush(w->write_arg);
+       if (err < 0) {
+               err = REFTABLE_IO_ERROR;
+               goto done;
+       }
+
        err = padded_write(w, footer, footer_size(writer_version(w)), 0);
        if (err < 0)
                goto done;

diff --git a/reftable/writer.h b/reftable/writer.h
index 09b88673d9757536c3c0f5122f991703eb94a06e..8d0df9cc528dba79677716b6b6760b93c7dad057 100644 (file)
--- a/reftable/writer.h
+++ b/reftable/writer.h
@@ -16,6 +16,7 @@ https://developers.google.com/open-source/licenses/bsd
 
 struct reftable_writer {
        ssize_t (*write)(void *, const void *, size_t);
+       int (*flush)(void *);
        void *write_arg;
        int pending_padding;
        struct strbuf last_key;