From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp39.i.mail.ru (smtp39.i.mail.ru [94.100.177.99]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 7B407445320 for ; Fri, 17 Jul 2020 01:45:17 +0300 (MSK) References: <20200714144440.551127-1-gorcunov@gmail.com> <20200714144440.551127-5-gorcunov@gmail.com> <20200714145358.GG296695@grain> From: Vladislav Shpilevoy Message-ID: Date: Fri, 17 Jul 2020 00:45:16 +0200 MIME-Version: 1.0 In-Reply-To: <20200714145358.GG296695@grain> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [Tarantool-patches] [PATCH v2 4/4] qsync: don't send negative timeouts into fiber_cond_wait_timeout List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Cyrill Gorcunov Cc: tml 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 > --- > 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) >