Web lists-archives.com

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. ;)
Maybe:

    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.

Thanks,
Stefan