Re: [PATCH 0/3] Use ref transactions for fetch

> On 04/22/2014 08:45 PM, Ronnie Sahlberg wrote:
>> This change is based on the previous ref transaction patches.
>> This is sent as a separate patch series since it implements a lot more
>> non-trivial changes to the behaviour than the previous patches and thus can
>> use more detailed review.
>>
>> Update fetch.c to use ref transactions when performing updates. Use a single
>> ref transaction for all updates and only commit the transaction if all other
>> checks and oeprations have been successful. This makes the ref updates during
>> a fetch (mostly) atomic.
>
> Is this always an improvement?  What kind of checks are there that might
> fail?
>
> It would be pretty annoying to spend a lot of time fetching a big pack,
> only to have the fetch fail because one reference out of many couldn't
> be updated.  This would force the user to download the entire pack
> again, whereas if the successful reference updates had been allowed,
> then probably most or all of the second download would have been avoidable.
>
> On the other hand, if a reference was renamed on the remote side,
> allowing a partial reference update could cause history to be discarded
> locally if the old name's delete was accepted but the new name's
> addition was rejected.  This wouldn't be the end of the world, because
> the history is presumably still available remotely to fetch again, but
> it's not ideal either.
>
> I'm not sure myself what I would prefer, but I wanted to point out that
> it is IMO not obvious that atomicity here is an improvement.
>