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

Ævar Arnfjörð Bjarmason [off-list ref] writes:

    We fsync() the file descriptors we're writing out for all the
    auxiliary files, but we don't yet fsync() the file descriptor for
    the containing directory. Therefore our data might have been written
    out, but it's anyone's guess what the state of the directory
    containing the file is after we write the *.idx.

    In practice modern OS's are known to be forgiving on that point, so
    this will probably solve races in practice for most users. It will
    almost certainly make them better than they were before when we
    didn't write *.idx files last. We should more generally improve our
    use of fsync() to cover containing directories, but that'll
    hopefully be addressed by some follow-up series.

I'd probably drop the last paragraph, and replace it with a single
sentence "we may want to fsync the containing directory once after
placing *.idx file in place, but it is outside of the scope of this
series", if I were doing this series.

Other than that (and I made a few comments on other patches), these
were pleasant read.

Thanks.

On Thu, Sep 09, 2021 at 12:52:03PM -0700, Junio C Hamano wrote:

Ævar Arnfjörð Bjarmason [off-list ref] writes:

quoted

    We fsync() the file descriptors we're writing out for all the
    auxiliary files, but we don't yet fsync() the file descriptor for
    the containing directory. Therefore our data might have been written
    out, but it's anyone's guess what the state of the directory
    containing the file is after we write the *.idx.

    In practice modern OS's are known to be forgiving on that point, so
    this will probably solve races in practice for most users. It will
    almost certainly make them better than they were before when we
    didn't write *.idx files last. We should more generally improve our
    use of fsync() to cover containing directories, but that'll
    hopefully be addressed by some follow-up series.

I'd probably drop the last paragraph, and replace it with a single
sentence "we may want to fsync the containing directory once after
placing *.idx file in place, but it is outside of the scope of this
series", if I were doing this series.

Yep, I think that this makes things clearer, and I prefer it to Ævar's
suggestion (which conveys more information than I think is necessary
here).

Thanks,
Taylor