Re: [Tarantool-patches] [PATCH 06/10] box/txn: unweave txn_commit from txn_commit_async

[Konstantin Osipov <kostja.osipov@gmail.com>]

* Cyrill Gorcunov <gorcunov@gmail.com> [20/03/05 15:32]:
> We gonna divergence sync and async code flow thus

divergence->diverge. Or rather, split.

> lets make txn_commit to not use txn_commit_async.
> 
> +++ b/src/box/txn.c
> @@ -604,10 +604,24 @@ txn_commit_async(struct txn *txn)
>  int
>  txn_commit(struct txn *txn)
>  {
> +	int res = -1;
> +
>  	txn->fiber = fiber();
>  
> -	if (txn_commit_async(txn) != 0)
> +	if (txn_prepare(txn) != 0) {
> +		txn_rollback(txn);
> +		goto out;
> +	}
> +
> +	if (txn_commit_nop(txn)) {
> +		res = 0;
> +		goto out;
> +	}
> +
> +	fiber_set_txn(fiber(), NULL);
> +	if (txn_write_to_wal(txn) != 0)
>  		return -1;

I know this function will keep changing, but 
this specific place when you don't use res 
make the code temporarily ugly.

I suggest not having out: branch, and duplicate 
txn_free() in the above two branches.  No big deal.

But it's also minor.

So lgtm.


> +
>  	/*
>  	 * In case of non-yielding journal the transaction could already
>  	 * be done and there is nothing to wait in such cases.
> @@ -617,10 +631,11 @@ txn_commit(struct txn *txn)
>  		fiber_yield();
>  		fiber_set_cancellable(cancellable);
>  	}
> -	int res = txn->signature >= 0 ? 0 : -1;
> +	res = txn->signature >= 0 ? 0 : -1;
>  	if (res != 0)
>  		diag_set(ClientError, ER_WAL_IO);
>  
> +out:
>  	/* Synchronous transactions are freed by the calling fiber. */
>  	txn_free(txn);
>  	return res;
> -- 
> 2.20.1
> 

-- 
Konstantin Osipov, Moscow, Russia