Re: do we have too much fsync() configuration in 'next'? (was: [PATCH v7] core.fsync: documentation and user-friendly aggregate options)

From: Neeraj Singh <nksingh85@gmail.com>
Date: Mar 28, 2022, 19:56 UTC
Message-ID: <CANQDOddSnSByqMrU+b11zywwWjOS+A5W0BSXa0rZURWn5zi2Tg@mail.gmail.com>
In-Reply-To: <220328.86pmm6e0i0.gmgdl@evledraar.gmail.com>

On Mon, Mar 28, 2022 at 5:15 AM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:

Show 26 quoted lines

>
> I hope we can work something out :)
>
> Overall: I think you've left one of the the main things I brought up[1]
> unaddressed, i.e. that the core.fsync config schema in its current form
> assumes that we can sync A or B, and configure those separately.
>
> Which AFAIKT is because Neeraj's initial implementation & the discussion
> was focused on finishing A or B with a per-"group" "cookie" to flush the
> files.
>
> But as [2] shows it's more performant for us to simply defer the fsync
> of A until the committing of B.
>
> Which is the main reason I think we should be re-visiting this. Sure, if
> we were just syncing A, B or C having per-[ABC] config options might be
> a bit overdoing it, but would be relatively simple.
>
> But once we start using a more optimized version of the "bulk" mode the
> config schema will be making promises about individual steps in a
> transaction that I think we'll want to leave opaque, and only promise
> that when git returns it will have synced all the relevant assets as
> efficiently as possible.
>
> 1. https://lore.kernel.org/git/220323.86fsn8ohg8.gmgdl@evledraar.gmail.com/
> 2. https://lore.kernel.org/git/RFC-patch-v2-4.7-61f4f3d7ef4-20220323T140753Z-avarab@gmail.com/

I think the current documentation is fine (obviously since I wrote it). Let's reproduce the first part again: --- core.fsync:: A comma-separated list of components of the repository that should be hardened via the core.fsyncMethod when created or modified. You can disable hardening of any component by prefixing it with a '-'. Items that are not hardened may be lost in the event of an unclean system shutdown. Unless you have special requirements, it is recommended that you leave this option empty or pick one of `committed`, `added`, or `all`. + When this configuration is encountered, the set of components starts with the platform default value, disabled components are removed, and additional components are added. `none` resets the state so that the platform default is ignored. + The empty string resets the fsync configuration to the platform default. The default on most platforms is equivalent to `core.fsync=committed,-loose-object`, which has good performance, but risks losing recent work in the event of an unclean system shutdown. + ---

We're only talking about "hardening" parts of the repository, and we say we'll do it using the "fsyncMethod". If you don't harden something, you could lose it if the system dies. All of these statements are true and don't say so much about the implementation of how the hardening happens. It's perfectly valid to not force any component out of the disk cache, and a straightforward implementation of repo transactions can put the sync point anywhere. We also explicitly point the user at a "porcelain" setting in the first paragraph.

So I think you're alone in thinking that anything needs to change here.

Thanks, Neeraj