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

[Michael Haggerty <mhagger@alum.mit.edu>]

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.

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/