Subject: Re: [PATCH v4 0/3] Make update refs more atomic

From: Michael Haggerty <[email-masked]>
Date: Tue, 15 Apr 2014 22:32:01 +0200

On 04/15/2014 06:33 PM, Ronnie Sahlberg wrote:
> On Mon, Apr 14, 2014 at 11:36 PM, Michael Haggerty <[email-masked]> wrote:
>> [...]
>> I wonder, however, whether your approach of changing callers from
>>
>>     lock = lock_ref_sha1_basic() (or varient of)
>>     write_ref_sha1(lock)
>>
>> to
>>
>>     lock = lock_ref_sha1_basic() (or varient of)
>>     write_ref_sha1(lock)
>>     unlock_ref(lock) | commit_ref_lock(lock)
>>
>> is not doing work that we will soon need to rework.  Would it be jumping
>> the gun to change the callers to
>>
>>     transaction = ref_transaction_begin();
>>     ref_transaction_{update,delete,etc}(transaction, ...);
>>     ref_transaction_{commit,rollback}(transaction, ...);
>>
>> instead?  Then we could bury the details of calling write_ref_sha1() and
>> commit_lock_ref() inside ref_transaction_commit() rather than having to
>> expose them in the public API.
> 
> I think you are right.
> 
> Lets put this patch series on the backburner for now and start by
> making all callers use transactions
> and remove write_ref_sha1() from the public API thar refs.c exports.
> 
> Once everything is switched over to transactions I can rework this
> patchseries for ref_transaction_commit()
> and resubmit to the mailing list.

Sounds good.  Rewriting callers to use transactions would be a great
next step.  Please especially keep track of what new features the
transactions API still needs.  More flexible error handling?  The
ability to have steps in the transaction that are "best-effort" (i.e.,
don't abort the transaction if they fail)?  Different reflog messages
for different updates within the same transaction rather than one reflog
message for all updates?  Etc.

And some callers who currently change multiple references one at a time
might be able to be rewritten to update the references in a single
transaction.

> Lets start preparing patches to change all external callers to use
> transactions instead.
> I am happy to help preparing patches for this. How do we ensure that
> we do not create duplicate work
> and work on the same functions?

I have a few loose ends to take care of on my lockfile patch series, and
there are a few things I would like to tidy up internal to the
transactions implementation, so I think if you are working on the caller
side then we won't step on each other's toes too much in the near future.

I suggest we use IRC (mhagger@freenode) or XMPP ([email-masked]) for
small-scale coordination.  I also have a GitHub repo
(http://github.com/mhagger/git) to which I often push intermediate
results; I will try to push to that more regularly (warning: I often
rebase feature branches even after they are pushed to GitHub).  I think
you are in Pacific Time whereas I am in Berlin, so we will tend to work
in serial rather than in parallel; that should help.  It would be a good
habit to shoot each short status emails at the end of each working day.

Of course we should only use one-on-one communication for early work; as
soon as something is getting ripe we should make sure our technical
discussions take place here on the mailing list.

Sound OK?
Michael

-- 
Michael Haggerty
[email-masked]
http://softwareswirl.blogspot.com/