[PATCH 9/9] pack-objects: rename .idx files into place after .bitmap files

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



From: Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx>

In preceding commits the race of renaming .idx files in place before
.rev files and other auxiliary files was fixed in pack-write.c's
finish_tmp_packfile(), builtin/repack.c's "struct exts", and
builtin/index-pack.c's final(). As noted in the change to pack-write.c
we left in place the issue of writing *.bitmap files after the *.idx,
let's fix that issue.

See 7cc8f971085 (pack-objects: implement bitmap writing, 2013-12-21)
for commentary at the time when *.bitmap was implemented about how
those files are written out, nothing in that commit contradicts what's
being done here.

Note that this commit and preceding ones only close any race condition
with *.idx files being written before their auxiliary files if we're
optimistic about our lack of fsync()-ing in this are not tripping us
over. See the thread at [1] for a rabbit hole of various discussions
about filesystem races in the face of doing and not doing fsync() (and
if doing fsync(), not doing it properly).

In particular, in this case of writing to ".git/objects/pack" we only
write and fsync() the individual files, but if we wanted to guarantee
that the metadata update was seen in that way by concurrent processes
we'd need to fsync() on the "fd" of the containing directory. That
concern is probably more theoretical than not, modern OS's tend to be
more on the forgiving side than the overly pedantic side of
implementing POSIX FS semantics.

1. https://lore.kernel.org/git/8735qgkvv1.fsf@xxxxxxxxxxxxxxxxxxx/

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx>
Signed-off-by: Taylor Blau <me@xxxxxxxxxxxx>
---
 builtin/pack-objects.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 944134b6f2..a01767a384 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -1250,7 +1250,6 @@ static void write_pack_file(void)
 			stage_tmp_packfiles(&tmpname, pack_tmp_name,
 					    written_list, nr_written,
 					    &pack_idx_opts, hash, &idx_tmp_name);
-			rename_tmp_packfile_idx(&tmpname, &idx_tmp_name);
 
 			if (write_bitmap_index) {
 				size_t tmpname_len = tmpname.len;
@@ -1267,6 +1266,8 @@ static void write_pack_file(void)
 				strbuf_setlen(&tmpname, tmpname_len);
 			}
 
+			rename_tmp_packfile_idx(&tmpname, &idx_tmp_name);
+
 			free(idx_tmp_name);
 			strbuf_release(&tmpname);
 			free(pack_tmp_name);
-- 
2.33.0.96.g73915697e6



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux