Re: [PATCH v2 4/4] fetch: implement support for atomic reference updates
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 <ps@pks.im> writes:
Show 30 quoted lines
> > > 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