Re: [Tarantool-patches] [PATCH 04/10] box/txn: move setup of txn start to txn_prepare

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

* Cyrill Gorcunov <gorcunov@gmail.com> [20/03/05 15:32]:
> For unification sake, we will handle nop transactions
> via common helper for both sync and async cases.
> 
> Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
> ---
>  src/box/txn.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/src/box/txn.c b/src/box/txn.c
> index 9f61303ab..5dd52e6db 100644
> --- a/src/box/txn.c
> +++ b/src/box/txn.c
> @@ -554,6 +554,15 @@ txn_prepare(struct txn *txn)
>  	trigger_clear(&txn->fiber_on_stop);
>  	if (!txn_has_flag(txn, TXN_CAN_YIELD))
>  		trigger_clear(&txn->fiber_on_yield);
> +
> +	/*
> +	 * It is important to set start transaction
> +	 * time at the last moment, when everything
> +	 * is ready to initiate commit procedure,
> +	 * just to be more precise in timings to
> +	 * detect long WAL writes.
> +	 */

I think this comment is misleading. There are no yields
between start of txn_prepare() and end. There may appear yields
when vinyl lock manager is more smart, but there are no yields
now. So ev_monotonic_now returns exactly the same value regardless 
of where you call it.

Otherwise lgtm.

> +	txn->start_tm = ev_monotonic_now(loop());
>  	return 0;
>  }
>  

-- 
Konstantin Osipov, Moscow, Russia