Re: [PATCH v4 2/4] core.fsync: introduce granular fsync control

On Wed, Mar 09 2022, Patrick Steinhardt wrote:

[[PGP Signed Part:Undecided]]
On Mon, Feb 14, 2022 at 09:17:31AM -0800, Junio C Hamano wrote:

quoted

Patrick Steinhardt [off-list ref] writes:

quoted

To summarize my take: while the degree of durability may be something
that's up for discussions, I think that the current defaults for
atomicity are bad for users because they can and do lead to repository
corruption.

Good summary.

If the user cares about fsynching loose object files in the right
way, we shouldn't leave loose ref files not following the safe
safety level, regardless of how this new core.fsync knobs would look
like.

I think we three are in agreement on that.

Is there anything I can specifically do to help out with this topic? We
have again hit data loss in production because we don't sync loose refs
to disk before renaming them into place, so I'd really love to sort out
this issue somehow so that I can revive my patch series which fixes the
known repository corruption [1].

Alternatively, can we maybe find a way forward with applying a version
of my patch series without first settling the bigger question of how we
want the overall design to look like? In my opinion repository
corruption is a severe bug that needs to be fixed, and it doesn't feel
sensible to block such a fix over a discussion that potentially will
take a long time to settle.

Patrick

[1]: http://public-inbox.org/git/cover.1636544377.git.ps@pks.im/

I share that view. I was wondering how this topic fizzled out the other
day, but then promptly forgot about it.

I think the best thing at this point (hint hint!) would be for someone
in the know to (re-)submit the various patches appropriate to move this
forward. Whether that's just this series, part of it, or some/both of
those + patches from you and Eric and this point I don't know/remember.

But just to be explicitly clear, as probably the person most responsible
for pushing this towards the "bigger question of [...] overall
design".

I just wanted to facilitate a discussion that would result in the
various stakeholders who wanted to add some fsync-related config coming
up with something that's mutually compatible, and I think the design
from Neeraj in this series fits that purpose, is Good Enough etc.

I.e. the actually important and IMO blockers were all resolved, e.g. not
having an fsync configuration that older git versions would needlessly
die on, and not painting ourselves into a corner where
e.g. core.fsync=false or something was squatted on by something other
than a "no fsync, whatsoever" etc.

(But I haven't looked at it again just now, so...)

Anyway, just trying to be explicit that to whatever extent this was held
up by questions/comments of mine I'm very happy to see this go forward.
As you (basically) say we shouldn't lose sight of ongoing data loss in
this area because of some config bikeshedding :)