Subject: RE: [PATCH v3 0/4] A design for future-proofing fsync() configuration

From: <[email-masked]>
Date: Sat, 8 Jan 2022 19:55:40 -0500

On January 7, 2022 8:14 PM, Neeraj Singh wrote:
> Hello Everyone,
> I wanted to revive this thread in the new year.
> 
> To summarize the current state of affairs:
> * The current fsync patch series implements two new configuration options:
>        core.fsync = <comma-separate list> -- select which repo components will
> be fsynced
>        core.fsyncMethod = fsync|writeout-only  -- select what form of fsyncing
> will be done
> 
> * This patch series now ignores core.fsyncObjectFiles with a deprecation
> warning pointing the user at core.fsync.
> 
> * There is a follow-on series that will extend the core.fsyncMethod to also
> include a `batch` mode that speeds up bulk operations by avoiding repeated
> disk cache flushes.
> 
> * I developed the current mechanism after Ævar pointed out that the original
> `core.fsyncObjectFiles=batch` change would cause older versions of Git to
> die() when exposed to a new configuration. There were also several fsync
> changes floating around, including Patrick Steinhardts `core.fsyncRefFiles`
> change [1] and Eric Wong's `core.fsync = false` change [2].
> 
> * The biggest sticking points are in [3].  The fundamental disagreement is
> about whether core.fsync should look like:
>       A) core.fsync = objects,commit-graph   [current patch implementation]
>       or
>       B) core.fsync = objects
>           core.fsync = commit-graph    [Ævar's multivalued proposal].
> I prefer sticking with (A) for reasons spelled out in the thread. I'm happy to re-
> litigate this discussion though.
> 
> * There's also a sticking point about whether we should fsync when invoking
> pack-objects against stdout.  I think that mostly reflects a missing comment in
> the code rather than a real disagreement.
> 
> * Now that ew/test-wo-fsync has been integrated, there's some redundancy
> between core.fsync=none and Eric's patch.
> 
> Open questions:
> 1) What format should we use for the core.fsync configuration to select
> individual repo components to sync?
> 2) Are we okay with deprecating core.fsyncObjectFiles in a single release with
> a warning?
> 3) Is it reasonable to expect people adding new persistent files to add and
> document new values of the core.fsync settings?
> 
> Thanks,
> Neeraj
> 
> [1]  https://lore.kernel.org/git/20211030103950.M489266@dcvr/
> [2] https://lore.kernel.org/git/[email-masked]/
> [3]
> https://lore.kernel.org/git/[email-masked]/

Neeraj,

Please remember that fsync() is operating system and version specific. You cannot make any assumptions about what is supported and what is not. I have recently had issues with git built on a recent operating system not running on a version from 2020. The proposed patches do not work, as I recall, in a portable manner, so caution is required making this change. You can expect this not to work on some platforms and some versions. Please account for that. Requiring users who are not aware of OS details to configure git to function at all is a bad move, in my view - which has not changed since last time.

Thanks,
Randall