@matheusbsbs / matheus-bagliano / commits / 1df18a1c9a

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>

John Cai committed Jan 23, 2024 at 18:51 UTC 1df18a1c9a50b58afb19ef218f6517f347606800

9 files changed +46 -19

reftable/merged_test.c

+3 -3

@@ -42,7 +42,7 @@ static void write_test_table(struct strbuf *buf,

42 }

43 }

45 - w = reftable_new_writer(&strbuf_add_void, buf, &opts);

45 + w = reftable_new_writer(&strbuf_add_void, &noop_flush, buf, &opts);

46 reftable_writer_set_limits(w, min, max);

48 for (i = 0; i < n; i++) {

@@ -70,7 +70,7 @@ static void write_test_log_table(struct strbuf *buf,

70 .exact_log_message = 1,

71 };

72 struct reftable_writer *w = NULL;

73 - w = reftable_new_writer(&strbuf_add_void, buf, &opts);

73 + w = reftable_new_writer(&strbuf_add_void, &noop_flush, buf, &opts);

74 reftable_writer_set_limits(w, update_index, update_index);

76 for (i = 0; i < n; i++) {

@@ -412,7 +412,7 @@ static void test_default_write_opts(void)

412 struct reftable_write_options opts = { 0 };

413 struct strbuf buf = STRBUF_INIT;

414 struct reftable_writer *w =

415 - reftable_new_writer(&strbuf_add_void, &buf, &opts);

415 + reftable_new_writer(&strbuf_add_void, &noop_flush, &buf, &opts);

416

417 struct reftable_ref_record rec = {

418 .refname = "master",

reftable/readwrite_test.c

+12 -12

@@ -51,7 +51,7 @@ static void write_table(char ***names, struct strbuf *buf, int N,

51 .hash_id = hash_id,

52 };

53 struct reftable_writer *w =

54 - reftable_new_writer(&strbuf_add_void, buf, &opts);

54 + reftable_new_writer(&strbuf_add_void, &noop_flush, buf, &opts);

55 struct reftable_ref_record ref = { NULL };

56 int i = 0, n;

57 struct reftable_log_record log = { NULL };

@@ -130,7 +130,7 @@ static void test_log_buffer_size(void)

130 .message = "commit: 9\n",

131 } } };

132 struct reftable_writer *w =

133 - reftable_new_writer(&strbuf_add_void, &buf, &opts);

133 + reftable_new_writer(&strbuf_add_void, &noop_flush, &buf, &opts);

134

135 /* This tests buffer extension for log compression. Must use a random

136 hash, to ensure that the compressed part is larger than the original.

@@ -171,7 +171,7 @@ static void test_log_overflow(void)

171 .message = msg,

172 } } };

173 struct reftable_writer *w =

174 - reftable_new_writer(&strbuf_add_void, &buf, &opts);

174 + reftable_new_writer(&strbuf_add_void, &noop_flush, &buf, &opts);

175

176 uint8_t hash1[GIT_SHA1_RAWSZ] = {1}, hash2[GIT_SHA1_RAWSZ] = { 2 };

177

@@ -202,7 +202,7 @@ static void test_log_write_read(void)

202 struct reftable_block_source source = { NULL };

203 struct strbuf buf = STRBUF_INIT;

204 struct reftable_writer *w =

205 - reftable_new_writer(&strbuf_add_void, &buf, &opts);

205 + reftable_new_writer(&strbuf_add_void, &noop_flush, &buf, &opts);

206 const struct reftable_stats *stats = NULL;

207 reftable_writer_set_limits(w, 0, N);

208 for (i = 0; i < N; i++) {

@@ -294,7 +294,7 @@ static void test_log_zlib_corruption(void)

294 struct reftable_block_source source = { 0 };

295 struct strbuf buf = STRBUF_INIT;

296 struct reftable_writer *w =

297 - reftable_new_writer(&strbuf_add_void, &buf, &opts);

297 + reftable_new_writer(&strbuf_add_void, &noop_flush, &buf, &opts);

298 const struct reftable_stats *stats = NULL;

299 uint8_t hash1[GIT_SHA1_RAWSZ] = { 1 };

300 uint8_t hash2[GIT_SHA1_RAWSZ] = { 2 };

@@ -535,7 +535,7 @@ static void test_table_refs_for(int indexed)

535

536 struct strbuf buf = STRBUF_INIT;

537 struct reftable_writer *w =

538 - reftable_new_writer(&strbuf_add_void, &buf, &opts);

538 + reftable_new_writer(&strbuf_add_void, &noop_flush, &buf, &opts);

539

540 struct reftable_iterator it = { NULL };

541 int j;

@@ -628,7 +628,7 @@ static void test_write_empty_table(void)

628 struct reftable_write_options opts = { 0 };

629 struct strbuf buf = STRBUF_INIT;

630 struct reftable_writer *w =

631 - reftable_new_writer(&strbuf_add_void, &buf, &opts);

631 + reftable_new_writer(&strbuf_add_void, &noop_flush, &buf, &opts);

632 struct reftable_block_source source = { NULL };

633 struct reftable_reader *rd = NULL;

634 struct reftable_ref_record rec = { NULL };

@@ -666,7 +666,7 @@ static void test_write_object_id_min_length(void)

666 };

667 struct strbuf buf = STRBUF_INIT;

668 struct reftable_writer *w =

669 - reftable_new_writer(&strbuf_add_void, &buf, &opts);

669 + reftable_new_writer(&strbuf_add_void, &noop_flush, &buf, &opts);

670 struct reftable_ref_record ref = {

671 .update_index = 1,

672 .value_type = REFTABLE_REF_VAL1,

@@ -701,7 +701,7 @@ static void test_write_object_id_length(void)

701 };

702 struct strbuf buf = STRBUF_INIT;

703 struct reftable_writer *w =

704 - reftable_new_writer(&strbuf_add_void, &buf, &opts);

704 + reftable_new_writer(&strbuf_add_void, &noop_flush, &buf, &opts);

705 struct reftable_ref_record ref = {

706 .update_index = 1,

707 .value_type = REFTABLE_REF_VAL1,

@@ -735,7 +735,7 @@ static void test_write_empty_key(void)

735 struct reftable_write_options opts = { 0 };

736 struct strbuf buf = STRBUF_INIT;

737 struct reftable_writer *w =

738 - reftable_new_writer(&strbuf_add_void, &buf, &opts);

738 + reftable_new_writer(&strbuf_add_void, &noop_flush, &buf, &opts);

739 struct reftable_ref_record ref = {

740 .refname = "",

741 .update_index = 1,

@@ -758,7 +758,7 @@ static void test_write_key_order(void)

758 struct reftable_write_options opts = { 0 };

759 struct strbuf buf = STRBUF_INIT;

760 struct reftable_writer *w =

761 - reftable_new_writer(&strbuf_add_void, &buf, &opts);

761 + reftable_new_writer(&strbuf_add_void, &noop_flush, &buf, &opts);

762 struct reftable_ref_record refs[2] = {

763 {

764 .refname = "b",

@@ -801,7 +801,7 @@ static void test_write_multiple_indices(void)

801 struct reftable_reader *reader;

802 int err, i;

803

804 - writer = reftable_new_writer(&strbuf_add_void, &writer_buf, &opts);

804 + writer = reftable_new_writer(&strbuf_add_void, &noop_flush, &writer_buf, &opts);

805 reftable_writer_set_limits(writer, 1, 1);

806 for (i = 0; i < 100; i++) {

807 struct reftable_ref_record ref = {

reftable/refname_test.c

+1 -1

@@ -30,7 +30,7 @@ static void test_conflict(void)

30 struct reftable_write_options opts = { 0 };

31 struct strbuf buf = STRBUF_INIT;

32 struct reftable_writer *w =

33 - reftable_new_writer(&strbuf_add_void, &buf, &opts);

33 + reftable_new_writer(&strbuf_add_void, &noop_flush, &buf, &opts);

34 struct reftable_ref_record rec = {

35 .refname = "a/b",

36 .value_type = REFTABLE_REF_SYMREF,

reftable/reftable-writer.h

@@ -88,6 +88,7 @@ struct reftable_stats {

88 /* reftable_new_writer creates a new writer */

89 struct reftable_writer *

90 reftable_new_writer(ssize_t (*writer_func)(void *, const void *, size_t),

91 + int (*flush_func)(void *),

92 void *writer_arg, struct reftable_write_options *opts);

94 /* Set the range of update indices for the records we will add. When writing a

reftable/stack.c

+13 -3

@@ -8,6 +8,7 @@ https://developers.google.com/open-source/licenses/bsd

9 #include "stack.h"

11 +#include "../write-or-die.h"

12 #include "system.h"

13 #include "merged.h"

14 #include "reader.h"

@@ -16,7 +17,6 @@ https://developers.google.com/open-source/licenses/bsd

17 #include "reftable-record.h"

18 #include "reftable-merged.h"

19 #include "writer.h"

19 -

20 #include "tempfile.h"

22 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)

47 return write_in_full(*fdp, data, sz);

48 }

50 +static int reftable_fd_flush(void *arg)

51 +{

52 + int *fdp = (int *)arg;

53 +

54 + return fsync_component(FSYNC_COMPONENT_REFERENCE, *fdp);

55 +}

56 +

57 int reftable_new_stack(struct reftable_stack **dest, const char *dir,

58 struct reftable_write_options config)

59 {

@@ -545,6 +552,9 @@ int reftable_addition_commit(struct reftable_addition *add)

552 goto done;

553 }

554

555 + fsync_component_or_die(FSYNC_COMPONENT_REFERENCE, lock_file_fd,

556 + get_tempfile_path(add->lock_file));

557 +

558 err = rename_tempfile(&add->lock_file, add->stack->list_file);

559 if (err < 0) {

560 err = REFTABLE_IO_ERROR;

@@ -639,7 +649,7 @@ int reftable_addition_add(struct reftable_addition *add,

649 goto done;

650 }

651 }

642 - wr = reftable_new_writer(reftable_fd_write, &tab_fd,

652 + wr = reftable_new_writer(reftable_fd_write, reftable_fd_flush, &tab_fd,

653 &add->stack->config);

654 err = write_table(wr, arg);

655 if (err < 0)

@@ -731,7 +741,7 @@ static int stack_compact_locked(struct reftable_stack *st, int first, int last,

741 strbuf_addstr(temp_tab, ".temp.XXXXXX");

742

743 tab_fd = mkstemp(temp_tab->buf);

734 - wr = reftable_new_writer(reftable_fd_write, &tab_fd, &st->config);

744 + wr = reftable_new_writer(reftable_fd_write, reftable_fd_flush, &tab_fd, &st->config);

745

746 err = stack_write_compact(st, wr, first, last, config);

747 if (err < 0)

reftable/test_framework.c

@@ -20,3 +20,8 @@ ssize_t strbuf_add_void(void *b, const void *data, size_t sz)

20 strbuf_add(b, data, sz);

21 return sz;

22 }

23 +

24 +int noop_flush(void *arg)

25 +{

26 + return 0;

27 +}

reftable/test_framework.h

@@ -56,4 +56,6 @@ void set_test_hash(uint8_t *p, int i);

56 */

57 ssize_t strbuf_add_void(void *b, const void *data, size_t sz);

59 +int noop_flush(void *);

60 +

61 #endif

reftable/writer.c

@@ -121,6 +121,7 @@ static struct strbuf reftable_empty_strbuf = STRBUF_INIT;

121

122 struct reftable_writer *

123 reftable_new_writer(ssize_t (*writer_func)(void *, const void *, size_t),

124 + int (*flush_func)(void *),

125 void *writer_arg, struct reftable_write_options *opts)

126 {

127 struct reftable_writer *wp =

@@ -136,6 +137,7 @@ reftable_new_writer(ssize_t (*writer_func)(void *, const void *, size_t),

137 wp->write = writer_func;

138 wp->write_arg = writer_arg;

139 wp->opts = *opts;

140 + wp->flush = flush_func;

141 writer_reinit_block_writer(wp, BLOCK_TYPE_REF);

142

143 return wp;

@@ -603,6 +605,12 @@ int reftable_writer_close(struct reftable_writer *w)

605 put_be32(p, crc32(0, footer, p - footer));

606 p += 4;

607

608 + err = w->flush(w->write_arg);

609 + if (err < 0) {

610 + err = REFTABLE_IO_ERROR;

611 + goto done;

612 + }

613 +

614 err = padded_write(w, footer, footer_size(writer_version(w)), 0);

615 if (err < 0)

616 goto done;

reftable/writer.h

@@ -16,6 +16,7 @@ https://developers.google.com/open-source/licenses/bsd

17 struct reftable_writer {

18 ssize_t (*write)(void *, const void *, size_t);

19 + int (*flush)(void *);

20 void *write_arg;

21 int pending_padding;

22 struct strbuf last_key;