Subject: Re: [PATCH 0/8] Making reflog modifications part of the transactions API

From: Michael Haggerty <[email-masked]>
Date: Fri, 12 Dec 2014 17:17:16 +0100

On 12/06/2014 03:46 AM, Stefan Beller wrote:
> This goes on top of Michaels series. The idea of this series is make the
> reflogs being part of the transaction API, so it will be part of the contract
> of transaction_commit to either commit all the changes or none at all.
> 
> Currently when using the transaction API to change refs, also reflogs are changed.
> But the changes to the reflogs just happen as a side effect and not as part of
> the atomic part of changes we want to commit altogether.

Would you please explain why this patch series is still needed if my
"reflog_expire()" series is accepted?

reflog_expire() already has its own little transaction. Reflog
expiration never needs to be combined with other reference changes, so
there is no need for reflog expiration to be done via a ref_transaction.

ref_transaction_commit() already updates the reflog if necessary when a
reference is updated. That update is part of the containing
ref_transaction, because it is locked by the lock on the reference
itself at the beginning of the transaction and unlocked at the end of
the transaction [1]. It can't fail in normal circumstances because the
preconditions for the transaction have already been checked.

As far as I can tell, the only thing left to do is provide a substitute
for log_ref_setup() a.k.a. create_reflog() that can be triggered within
a transaction. In my opinion the easiest way to do that is to give
ref_transaction_update()'s flag parameter an additional option,
REF_CREATE_REFLOG, which forces the reference's reflog to be created if
it does not already exist.

What am I missing?

Michael

[1] The reflog updates are not atomic in the face of disk errors and
power outages and stuff, but neither are reference updates. In other
words, I think reflog updates in ref_transaction_commit() are done as
safely as reference updates, which is surely good enough for this
reference backend. Other reference backends can do a better job with
both while staying within the existing API.

-- 
Michael Haggerty
[email-masked]