Re: [PATCH v2 0/6] Implement a batched fsync option for core.fsyncObjectFiles

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Sep 8, 2021, 19:23 UTC
Message-ID: <87k0jqhnji.fsf@evledraar.gmail.com>
In-Reply-To: <CANQDOddQsf4Jj+634mdnJXaPG=2idCbCHd1iXO2qm1EMGcDmXg@mail.gmail.com>

On Wed, Sep 08 2021, Neeraj Singh wrote:

Show 27 quoted lines

> On Tue, Sep 7, 2021 at 11:44 PM Junio C Hamano <gitster@pobox.com> wrote:
>>
>> Neeraj Singh <nksingh85@gmail.com> writes:
>>
>> > BTW, I updated the github PR to enable batch mode everywhere, and all
>> > the tests passed, which is good news to me.
>>
>> I doubt that fsyncObjectFiles is something we can reliably test in
>> CI, either with the new batched thing or with the original "when we
>> close one, make sure the changes hit the disk platter" approach.  So
>> I am not sure what conclusion we should draw from such an experiment,
>> other than "ok, it compiles cleanly."  After all, unless we cause
>> system crashes, what we thought we have written and close(2) would
>> be seen by another process that we spawn after that, with or without
>> sync, no?
>
> The main failure mode I was worried about is that some test or other part
> of Git is relying on a loose object being immediately available after it is
> added to the ODB. With batch mode, the loose objects aren't actually
> available until the bulk checkin is unplugged.
>
> I agree that it is not easy to test whether the data is actually going
> to durable
> storage at the expected time.  FWIW, I did take a disk IO trace on Windows to
> verify that we are issuing disk writes and flushes at the right time.
> But that's a
> one-time test that would be hard to make automated.

I have some semi-related patches I need to dig up and finish sometime which add a "git gc" test mode to the test suite, i.e. any time we call "git gc --auto" it will go ahead and actually run, and some adversarial options to run always, right away, prune with --expire=now. It found some false positives, but also some genuine races and bugs at the time.

Similarly, I think a good longer term goal for better fsync() and data integrity in git is to refactor the various codepaths where we write to disk (grepping for fsync_or_die() is a good start to find those) to all live in one place, we could then easily instrument that code to run in a hostile test mode.

E.g. make anything that expects to write out a "foo" file actually write out "foo.not-synced-yet" as long as fsync() etc. hasn't been called, or with signals/timers/atexit() handlers fake up known FS edge cases such as a write of "foo" only renaming "foo.not-synced-yet" to "foo" 1s after the last close() call not followed by an fsync, etc.

Anyway, I expect given your occupation that you may have better ideas in that area, presumably needing to instrument and test behavior under I/O pressure, deferred syncs etc. is something mature FS's need to deal with as part of their own regression tests...

1. https://lore.kernel.org/git/cover-v2-0.4-0000000000-20210908T003631Z-avarab@gmail.com/