Re: [PATCH v8 21/44] refs.c: ref_transaction_commit should not free the transaction

> On Thu, May 15, 2014 at 5:20 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
>> Ronnie Sahlberg wrote:
>>
>>> Change ref_transaction_commit so that it does not free the transaction.
>>> Instead require that a caller will end a transaction by either calling
>>> ref_transaction_rollback or ref_transaction_free.
>>
>> Can I always use ref_transaction_rollback instead of ref_transaction_free?
>> It would be convenient if my cleanup code could always call _rollback
>> instead of having to do something different for success and errors.
>
> Currently, yes.
>
> But it might make sense to change these so rollback only clears the
> updates that are in flight from the transaction and
> free only frees the transaction itself iff there are no updates in flight.
>
> I.e. the success and error would then differ like this :
> ...
>    if (transaction_commit()) {
>        transaction_rollback()
>        transaction_free()
>        return error("some error")
>    }
>    transaction_free()
>

>
>
>>
>> Another way to ask the question: what is it valid to do with a
>> transaction after commiting?
>
> Right now the only valid thing to do is either rollback or free. But
> we could allow other things too :
>
>
> re-usable transactions.
> ---------------------------------
> I don't know if this is a good reason or not, but one reason we might
> want to keep
> two different names could be if we want to start allowing to re-use
> transactions.
> For example for cases/backends where transaction_begin() might be very
> expensive.
>
> For that case I would imagine we could allow to do things such as
>
> t = transaction_begin()
> ...
> /* first transaction */
> transaction_update(...)
> transaction_commit(...)
>      if transaction failed   transaction_rollback(...)
>
> /* second transaction,  first transaction cleared all updates in
> flight either through commit or through rollback */
> transaction_update()
> transaction_commit()
>      if transaction failed   transaction_rollback(...)
> ...
> transaction_free()
>
>
> (In order to do something like this we would still need to do some
> changes so that rollback will only free the updates that were in
> flight but not free the transaction itself.)
>
>
>>
>> Thanks,
>> Jonathan