Subject: Re: [PATCH v2 4/4] fetch: implement support for atomic reference updates

From: Patrick Steinhardt <[email-masked]>
Date: Tue, 12 Jan 2021 13:29:02 +0100

On Tue, Jan 12, 2021 at 01:22:40PM +0100, Patrick Steinhardt wrote:
> On Mon, Jan 11, 2021 at 11:47:08AM -0800, Junio C Hamano wrote:
> > Patrick Steinhardt <[email-masked]> writes:
[snip]
> > > If we change to
> > > always accumulate first and flush once we're done, then we essentially
> > > have a change in behaviour as FETCH_HEADs aren't written in an
> > > interleaving fashion anymore, but only at the end.
> > 
> > I view it a good thing, actually, for another reason [*2*], but that
> > would take a follow-up fix that is much more involved, so let's not
> > go there (yet).  At least buffering a single line's worth of output
> > in a buffer and writing it out in a single write() would get us much
> > closer to solving the "mixed up output" from multiple processes
> > problem the current code seems to already have.
> 
> The buffering of a single line is exactly what we're doing now in the
> non-atomic case. Previously there had been multiple writes, now we only
> call `strbuf_write()` once on the buffer for each reference.
> 
> > > Also, if there is any
> > > unexpected error in between updating references which causes us to die,
> > > then we wouldn't have written the already updated references to
> > > FETCH_HEAD now.
> > 
> > That one may be a valid concern.
> > 
> > Thanks.
> 
> Just to be explicit: are you fine with changes in this patch as-is? They
> don't solve concurrent writes to FETCH_HEAD, but they should make it
> easier to solve that if we ever need to.
> 
> Patrick

Never mind, I forgot that I'm still replying on v2 of this patch series.

Patrick