@matheusbsbs / matheus-bagliano / commits / 020406eaa5

core.fsync: introduce granular fsync control infrastructure

This commit introduces the infrastructure for the core.fsync configuration knob. The repository components we want to sync are identified by flags so that we can turn on or off syncing for specific components. If core.fsyncObjectFiles is set and the core.fsync configuration also includes FSYNC_COMPONENT_LOOSE_OBJECT, we will fsync any loose objects. This picks the strictest data integrity behavior if core.fsync and core.fsyncObjectFiles are set to conflicting values. This change introduces the currently unused fsync_component helper, which will be used by a later patch that adds fsyncing to the refs backend. Actual configuration and documentation of the fsync components list are in other patches in the series to separate review of the underlying mechanism from the policy of how it's configured. Helped-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Neeraj Singh <neerajsi@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Neeraj Singh committed Mar 10, 2022 at 22:43 UTC 020406eaa52e67440d9b78087ec2ce25532cb219

15 files changed +97 -33

builtin/fast-import.c

+1 -1

@@ -865,7 +865,7 @@ static void end_packfile(void)

865 struct tag *t;

866

867 close_pack_windows(pack_data);

868 - finalize_hashfile(pack_file, cur_pack_oid.hash, 0);

868 + finalize_hashfile(pack_file, cur_pack_oid.hash, FSYNC_COMPONENT_PACK, 0);

869 fixup_pack_header_footer(pack_data->pack_fd, pack_data->hash,

870 pack_data->pack_name, object_count,

871 cur_pack_oid.hash, pack_size);

builtin/index-pack.c

+2 -2

@@ -1290,7 +1290,7 @@ static void conclude_pack(int fix_thin_pack, const char *curr_pack, unsigned cha

1290 nr_objects - nr_objects_initial);

1291 stop_progress_msg(&progress, msg.buf);

1292 strbuf_release(&msg);

1293 - finalize_hashfile(f, tail_hash, 0);

1293 + finalize_hashfile(f, tail_hash, FSYNC_COMPONENT_PACK, 0);

1294 hashcpy(read_hash, pack_hash);

1295 fixup_pack_header_footer(output_fd, pack_hash,

1296 curr_pack, nr_objects,

@@ -1512,7 +1512,7 @@ static void final(const char *final_pack_name, const char *curr_pack_name,

1512 if (!from_stdin) {

1513 close(input_fd);

1514 } else {

1515 - fsync_or_die(output_fd, curr_pack_name);

1515 + fsync_component_or_die(FSYNC_COMPONENT_PACK, output_fd, curr_pack_name);

1516 err = close(output_fd);

1517 if (err)

1518 die_errno(_("error while closing pack file"));

builtin/pack-objects.c

+17 -7

@@ -1199,16 +1199,26 @@ static void write_pack_file(void)

1199 display_progress(progress_state, written);

1200 }

1201

1202 - /*

1203 - * Did we write the wrong # entries in the header?

1204 - * If so, rewrite it like in fast-import

1205 - */

1202 if (pack_to_stdout) {

1207 - finalize_hashfile(f, hash, CSUM_HASH_IN_STREAM | CSUM_CLOSE);

1203 + /*

1204 + * We never fsync when writing to stdout since we may

1205 + * not be writing to an actual pack file. For instance,

1206 + * the upload-pack code passes a pipe here. Calling

1207 + * fsync on a pipe results in unnecessary

1208 + * synchronization with the reader on some platforms.

1209 + */

1210 + finalize_hashfile(f, hash, FSYNC_COMPONENT_NONE,

1211 + CSUM_HASH_IN_STREAM | CSUM_CLOSE);

1212 } else if (nr_written == nr_remaining) {

1209 - finalize_hashfile(f, hash, CSUM_HASH_IN_STREAM | CSUM_FSYNC | CSUM_CLOSE);

1213 + finalize_hashfile(f, hash, FSYNC_COMPONENT_PACK,

1214 + CSUM_HASH_IN_STREAM | CSUM_FSYNC | CSUM_CLOSE);

1215 } else {

1211 - int fd = finalize_hashfile(f, hash, 0);

1216 + /*

1217 + * If we wrote the wrong number of entries in the

1218 + * header, rewrite it like in fast-import.

1219 + */

1220 +

1221 + int fd = finalize_hashfile(f, hash, FSYNC_COMPONENT_PACK, 0);

1222 fixup_pack_header_footer(fd, hash, pack_tmp_name,

1223 nr_written, hash, offset);

1224 close(fd);

bulk-checkin.c

+3 -2

@@ -53,9 +53,10 @@ static void finish_bulk_checkin(struct bulk_checkin_state *state)

53 unlink(state->pack_tmp_name);

54 goto clear_exit;

55 } else if (state->nr_written == 1) {

56 - finalize_hashfile(state->f, hash, CSUM_HASH_IN_STREAM | CSUM_FSYNC | CSUM_CLOSE);

56 + finalize_hashfile(state->f, hash, FSYNC_COMPONENT_PACK,

57 + CSUM_HASH_IN_STREAM | CSUM_FSYNC | CSUM_CLOSE);

58 } else {

58 - int fd = finalize_hashfile(state->f, hash, 0);

59 + int fd = finalize_hashfile(state->f, hash, FSYNC_COMPONENT_PACK, 0);

60 fixup_pack_header_footer(fd, hash, state->pack_tmp_name,

61 state->nr_written, hash,

62 state->offset);

cache.h

+23

@@ -993,6 +993,27 @@ void reset_shared_repository(void);

993 extern int read_replace_refs;

994 extern char *git_replace_ref_base;

995

996 +/*

997 + * These values are used to help identify parts of a repository to fsync.

998 + * FSYNC_COMPONENT_NONE identifies data that will not be a persistent part of the

999 + * repository and so shouldn't be fsynced.

1000 + */

1001 +enum fsync_component {

1002 + FSYNC_COMPONENT_NONE,

1003 + FSYNC_COMPONENT_LOOSE_OBJECT = 1 << 0,

1004 + FSYNC_COMPONENT_PACK = 1 << 1,

1005 + FSYNC_COMPONENT_PACK_METADATA = 1 << 2,

1006 + FSYNC_COMPONENT_COMMIT_GRAPH = 1 << 3,

1007 +};

1008 +

1009 +#define FSYNC_COMPONENTS_DEFAULT (FSYNC_COMPONENT_PACK | \

1010 + FSYNC_COMPONENT_PACK_METADATA | \

1011 + FSYNC_COMPONENT_COMMIT_GRAPH)

1012 +

1013 +/*

1014 + * A bitmask indicating which components of the repo should be fsynced.

1015 + */

1016 +extern enum fsync_component fsync_components;

1017 extern int fsync_object_files;

1018 extern int use_fsync;

1019

@@ -1707,6 +1728,8 @@ int copy_file_with_time(const char *dst, const char *src, int mode);

1728

1729 void write_or_die(int fd, const void *buf, size_t count);

1730 void fsync_or_die(int fd, const char *);

1731 +int fsync_component(enum fsync_component component, int fd);

1732 +void fsync_component_or_die(enum fsync_component component, int fd, const char *msg);

1733

1734 ssize_t read_in_full(int fd, void *buf, size_t count);

1735 ssize_t write_in_full(int fd, const void *buf, size_t count);

commit-graph.c

+2 -1

@@ -1942,7 +1942,8 @@ static int write_commit_graph_file(struct write_commit_graph_context *ctx)

1942 }

1943

1944 close_commit_graph(ctx->r->objects);

1945 - finalize_hashfile(f, file_hash, CSUM_HASH_IN_STREAM | CSUM_FSYNC);

1945 + finalize_hashfile(f, file_hash, FSYNC_COMPONENT_COMMIT_GRAPH,

1946 + CSUM_HASH_IN_STREAM | CSUM_FSYNC);

1947 free_chunkfile(cf);

1948

1949 if (ctx->split) {

csum-file.c

+3 -2

@@ -58,7 +58,8 @@ static void free_hashfile(struct hashfile *f)

58 free(f);

59 }

61 -int finalize_hashfile(struct hashfile *f, unsigned char *result, unsigned int flags)

61 +int finalize_hashfile(struct hashfile *f, unsigned char *result,

62 + enum fsync_component component, unsigned int flags)

63 {

64 int fd;

@@ -69,7 +70,7 @@ int finalize_hashfile(struct hashfile *f, unsigned char *result, unsigned int fl

70 if (flags & CSUM_HASH_IN_STREAM)

71 flush(f, f->buffer, the_hash_algo->rawsz);

72 if (flags & CSUM_FSYNC)

72 - fsync_or_die(f->fd, f->name);

73 + fsync_component_or_die(component, f->fd, f->name);

74 if (flags & CSUM_CLOSE) {

75 if (close(f->fd))

76 die_errno("%s: sha1 file error on close", f->name);

csum-file.h

+2 -1

@@ -1,6 +1,7 @@

1 #ifndef CSUM_FILE_H

2 #define CSUM_FILE_H

4 +#include "cache.h"

5 #include "hash.h"

7 struct progress;

@@ -38,7 +39,7 @@ int hashfile_truncate(struct hashfile *, struct hashfile_checkpoint *);

39 struct hashfile *hashfd(int fd, const char *name);

40 struct hashfile *hashfd_check(const char *name);

41 struct hashfile *hashfd_throughput(int fd, const char *name, struct progress *tp);

41 -int finalize_hashfile(struct hashfile *, unsigned char *, unsigned int);

42 +int finalize_hashfile(struct hashfile *, unsigned char *, enum fsync_component, unsigned int);

43 void hashwrite(struct hashfile *, const void *, unsigned int);

44 void hashflush(struct hashfile *f);

45 void crc32_begin(struct hashfile *);

environment.c

@@ -45,6 +45,7 @@ int pack_compression_level = Z_DEFAULT_COMPRESSION;

45 int fsync_object_files;

46 int use_fsync = -1;

47 enum fsync_method fsync_method = FSYNC_METHOD_DEFAULT;

48 +enum fsync_component fsync_components = FSYNC_COMPONENTS_DEFAULT;

49 size_t packed_git_window_size = DEFAULT_PACKED_GIT_WINDOW_SIZE;

50 size_t packed_git_limit = DEFAULT_PACKED_GIT_LIMIT;

51 size_t delta_base_cache_limit = 96 * 1024 * 1024;

midx.c

+2 -1

@@ -1438,7 +1438,8 @@ static int write_midx_internal(const char *object_dir,

1438 write_midx_header(f, get_num_chunks(cf), ctx.nr - dropped_packs);

1439 write_chunkfile(cf, &ctx);

1440

1441 - finalize_hashfile(f, midx_hash, CSUM_FSYNC | CSUM_HASH_IN_STREAM);

1441 + finalize_hashfile(f, midx_hash, FSYNC_COMPONENT_PACK_METADATA,

1442 + CSUM_FSYNC | CSUM_HASH_IN_STREAM);

1443 free_chunkfile(cf);

1444

1445 if (flags & MIDX_WRITE_REV_INDEX &&

object-file.c

+9 -4

@@ -1849,11 +1849,16 @@ int hash_object_file(const struct git_hash_algo *algo, const void *buf,

1849 /* Finalize a file on disk, and close it. */

1850 static void close_loose_object(int fd)

1851 {

1852 - if (!the_repository->objects->odb->will_destroy) {

1853 - if (fsync_object_files)

1854 - fsync_or_die(fd, "loose object file");

1855 - }

1852 + if (the_repository->objects->odb->will_destroy)

1853 + goto out;

1854

1855 + if (fsync_object_files > 0)

1856 + fsync_or_die(fd, "loose object file");

1857 + else

1858 + fsync_component_or_die(FSYNC_COMPONENT_LOOSE_OBJECT, fd,

1859 + "loose object file");

1860 +

1861 +out:

1862 if (close(fd) != 0)

1863 die_errno(_("error when closing loose object file"));

1864 }

pack-bitmap-write.c

+2 -1

@@ -719,7 +719,8 @@ void bitmap_writer_finish(struct pack_idx_entry **index,

719 if (options & BITMAP_OPT_HASH_CACHE)

720 write_hash_cache(f, index, index_nr);

721

722 - finalize_hashfile(f, NULL, CSUM_HASH_IN_STREAM | CSUM_FSYNC | CSUM_CLOSE);

722 + finalize_hashfile(f, NULL, FSYNC_COMPONENT_PACK_METADATA,

723 + CSUM_HASH_IN_STREAM | CSUM_FSYNC | CSUM_CLOSE);

724

725 if (adjust_shared_perm(tmp_file.buf))

726 die_errno("unable to make temporary bitmap file readable");

pack-write.c

+7 -6

@@ -159,9 +159,9 @@ const char *write_idx_file(const char *index_name, struct pack_idx_entry **objec

159 }

160

161 hashwrite(f, sha1, the_hash_algo->rawsz);

162 - finalize_hashfile(f, NULL, CSUM_HASH_IN_STREAM | CSUM_CLOSE |

163 - ((opts->flags & WRITE_IDX_VERIFY)

164 - ? 0 : CSUM_FSYNC));

162 + finalize_hashfile(f, NULL, FSYNC_COMPONENT_PACK_METADATA,

163 + CSUM_HASH_IN_STREAM | CSUM_CLOSE |

164 + ((opts->flags & WRITE_IDX_VERIFY) ? 0 : CSUM_FSYNC));

165 return index_name;

166 }

167

@@ -281,8 +281,9 @@ const char *write_rev_file_order(const char *rev_name,

281 if (rev_name && adjust_shared_perm(rev_name) < 0)

282 die(_("failed to make %s readable"), rev_name);

283

284 - finalize_hashfile(f, NULL, CSUM_HASH_IN_STREAM | CSUM_CLOSE |

285 - ((flags & WRITE_IDX_VERIFY) ? 0 : CSUM_FSYNC));

284 + finalize_hashfile(f, NULL, FSYNC_COMPONENT_PACK_METADATA,

285 + CSUM_HASH_IN_STREAM | CSUM_CLOSE |

286 + ((flags & WRITE_IDX_VERIFY) ? 0 : CSUM_FSYNC));

287

288 return rev_name;

289 }

@@ -390,7 +391,7 @@ void fixup_pack_header_footer(int pack_fd,

391 the_hash_algo->final_fn(partial_pack_hash, &old_hash_ctx);

392 the_hash_algo->final_fn(new_pack_hash, &new_hash_ctx);

393 write_or_die(pack_fd, new_pack_hash, the_hash_algo->rawsz);

393 - fsync_or_die(pack_fd, pack_name);

394 + fsync_component_or_die(FSYNC_COMPONENT_PACK, pack_fd, pack_name);

395 }

396

397 char *index_pack_lockfile(int ip_out, int *is_well_formed)

read-cache.c

+1 -1

@@ -3089,7 +3089,7 @@ static int do_write_index(struct index_state *istate, struct tempfile *tempfile,

3089 return -1;

3090 }

3091

3092 - finalize_hashfile(f, istate->oid.hash, CSUM_HASH_IN_STREAM);

3092 + finalize_hashfile(f, istate->oid.hash, FSYNC_COMPONENT_NONE, CSUM_HASH_IN_STREAM);

3093 if (close_tempfile_gently(tempfile)) {

3094 error(_("could not close '%s'"), get_tempfile_path(tempfile));

3095 return -1;

write-or-die.c

+22 -4

@@ -56,21 +56,39 @@ void fprintf_or_die(FILE *f, const char *fmt, ...)

56 }

57 }

59 -void fsync_or_die(int fd, const char *msg)

59 +static int maybe_fsync(int fd)

60 {

61 if (use_fsync < 0)

62 use_fsync = git_env_bool("GIT_TEST_FSYNC", 1);

63 if (!use_fsync)

64 - return;

64 + return 0;

66 if (fsync_method == FSYNC_METHOD_WRITEOUT_ONLY &&

67 git_fsync(fd, FSYNC_WRITEOUT_ONLY) >= 0)

68 - return;

68 + return 0;

69 +

70 + return git_fsync(fd, FSYNC_HARDWARE_FLUSH);

71 +}

70 - if (git_fsync(fd, FSYNC_HARDWARE_FLUSH) < 0)

73 +void fsync_or_die(int fd, const char *msg)

74 +{

75 + if (maybe_fsync(fd) < 0)

76 die_errno("fsync error on '%s'", msg);

77 }

79 +int fsync_component(enum fsync_component component, int fd)

80 +{

81 + if (fsync_components & component)

82 + return maybe_fsync(fd);

83 + return 0;

84 +}

85 +

86 +void fsync_component_or_die(enum fsync_component component, int fd, const char *msg)

87 +{

88 + if (fsync_components & component)

89 + fsync_or_die(fd, msg);

90 +}

91 +

92 void write_or_die(int fd, const void *buf, size_t count)

93 {

94 if (write_in_full(fd, buf, count) < 0) {