On Tue, Sep 28, 2021 at 11:32:43PM +0000, Neeraj Singh via GitGitGadget wrote: > If a temporary ODB is active, as determined by GIT_QUARANTINE_PATH > being set, create object files with their final names. This avoids > an extra rename beyond what is needed to merge the temporary ODB in > tmp_objdir_migrate. What's our goal here? Is it the performance of avoiding the extra rename()? Or do we benefit from the simplicity of avoiding it? If the former, do we have measurements on how much this matters? If the latter, what does the simplicity buy us? I thought maybe it would make reasoning about fsync() easier, because we don't have to worry about fsyncing the rename. But we'd eventually have to rename() into the real object directory anyway. The reason I want to push back is... > Creating an object file with the expected final name should be okay > since the git process writing to the temporary object store is the > only writer, and it only invokes write_loose_object/create_object_file > after checking that the object doesn't exist. ...this seems like a kind-of dangerous assumption. Most of the time, yeah, I'd expect just a single process to be writing. But one of the things that happens during the receive-pack quarantine is that we run hooks, which can run any set of arbitrary Git commands, including simultaneous readers and writers. It seems like we might be introducing subtle races there. -Peff
