[Tarantool-patches] [PATCH v2 4/4] qsync: don't send negative timeouts into fiber_cond_wait_timeout

[Vladislav Shpilevoy]

Thanks for the patch!

On 14.07.2020 16:53, Cyrill Gorcunov wrote:
> Basically our timeout is calculated via (a - b), where
> @a is a constant positive value fetched once, in turn
> the @b is rather a dynamic value thus the result may
> be negative. libev uses assert() call to catch such
> values when passed to timers setup. Thus lets intercept
> potential assert() trigger and exit early if timeout
> is already expired.
> 
> Signed-off-by: Cyrill Gorcunov <gorcunov at gmail.com>
> ---
> There were a typo, so I force-updated the branch
> 
>  src/box/txn_limbo.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/src/box/txn_limbo.c b/src/box/txn_limbo.c
> index d5b887d36..0924952b7 100644
> --- a/src/box/txn_limbo.c
> +++ b/src/box/txn_limbo.c
> @@ -174,8 +174,10 @@ txn_limbo_wait_complete(struct txn_limbo *limbo, struct txn_limbo_entry *entry)
>  	double start_time = fiber_clock();
>  	while (true) {
>  		double deadline = start_time + replication_synchro_timeout;
> -		bool cancellable = fiber_set_cancellable(false);
>  		double timeout = deadline - fiber_clock();
> +		if (timeout < 0)
> +			goto do_rollback;
> +		bool cancellable = fiber_set_cancellable(false);

I added timeout = -1; here and tried to commit a sync transaction. I
got timed out error, no assertions. Please, tell me, how to reproduce
the assertion you mention in the commit message. Otherwise I don't see
why would we need the < 0 check. If it is done somewhere inside
fiber_cond_wait_timeout anyway.

>  		int rc = fiber_cond_wait_timeout(&limbo->wait_cond, timeout);
>  		fiber_set_cancellable(cancellable);
>  		if (txn_limbo_entry_is_complete(entry))
> @@ -509,8 +511,10 @@ txn_limbo_wait_confirm(struct txn_limbo *limbo)
>  	double start_time = fiber_clock();
>  	while (true) {
>  		double deadline = start_time + replication_synchro_timeout;
> -		bool cancellable = fiber_set_cancellable(false);
>  		double timeout = deadline - fiber_clock();
> +		if (timeout < 0)
> +			goto timed_out;
> +		bool cancellable = fiber_set_cancellable(false);
>  		int rc = fiber_cond_wait_timeout(&limbo->wait_cond, timeout);
>  		fiber_set_cancellable(cancellable);
>  		if (cwp.is_confirm || cwp.is_rollback)
>