Re: [PATCH 12/23] ref_transaction_commit(): break into multiple functions
- Date: Wed, 17 May 2017 10:44:33 -0700
- From: Stefan Beller <sbeller@xxxxxxxxxx>
- Subject: Re: [PATCH 12/23] ref_transaction_commit(): break into multiple functions
On Wed, May 17, 2017 at 5:05 AM, Michael Haggerty <mhagger@xxxxxxxxxxxx> wrote:
> Break the function `ref_transaction_commit()` into two functions,
> `ref_transaction_prepare()` and `ref_transaction_finish()`, with a
> third function, `ref_transaction_abort()`, that can be used to abort
> the transaction. Break up the `ref_store` methods similarly.
> This split will make it easier to implement compound reference stores,
> where a transaction might have to span multiple underlying stores. In
> that case, we would first want to "prepare" the sub-transactions in
> each of the reference stores. If any of the "prepare" steps fails, we
> would "abort" all of the sub-transactions. Only if all of the
> "prepare" steps succeed would we "finish" each of them.
> Signed-off-by: Michael Haggerty <mhagger@xxxxxxxxxxxx>
Code looks good to me.
> * Transaction states.
> - * OPEN: The transaction is in a valid state and can accept new updates.
> - * An OPEN transaction can be committed.
> - * CLOSED: A closed transaction is no longer active and no other operations
> - * than free can be used on it in this state.
> - * A transaction can either become closed by successfully committing
> - * an active transaction or if there is a failure while building
> - * the transaction thus rendering it failed/inactive.
> + *
> + * OPEN: The transaction is in a valid state and new updates can be
> + * added to it. An OPEN transaction can be prepared or
> + * committed.
All of these states are valid, OPEN is not more valid than the others. ;)
OPEN: initial state. new updates can be added to it. An OPEN
transaction can be prepared, committed.
Is it possible to also abort/free an open transaction?
> + *
> + * PREPARED: ref_transaction_prepare(), which locks all of the
> + * references involved in the update and checks that the
> + * update has no errors, has been called successfully for the
> + * transaction.
Maybe add that it cannot be altered any more? Possible ways out are
commit and abort IIUC?
> + *
> + * CLOSED: The transaction is no longer active. No other operations
> + * than free can be used on it in this state. A transaction
> + * becomes closed if there is a failure while building the
> + * transaction, if an active transaction is committed, or if a
The wording here is off as it doesn't use the lingo as introduced before.
(What is an "active" transaction? I think you mean OPEN in this case)
> + * prepared transaction is finished.
Also s/prepared/PREPARED/, add "or aborted"(?)
as that is also a viable way IIUC.