From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp42.i.mail.ru (smtp42.i.mail.ru [94.100.177.102]) (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 32FC94765E0 for ; Wed, 23 Dec 2020 15:01:08 +0300 (MSK) From: Serge Petrenko References: <7c1516165eee62eb534cc5080971223e1edc3ca8.1607633488.git.sergepetrenko@tarantool.org> <3d0aa43b-bd36-12b3-2abd-754468f2a301@tarantool.org> <99e20e44-9833-5e32-00dc-d6ed91058960@tarantool.org> Message-ID: <52888748-ede0-d409-374a-6fc9eb6f5de8@tarantool.org> Date: Wed, 23 Dec 2020 15:01:07 +0300 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="utf-8"; format="flowed" Content-Transfer-Encoding: 8bit Content-Language: en-GB Subject: Re: [Tarantool-patches] [PATCH 1/4] box: add a single execution guard to clear_synchro_queue List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Vladislav Shpilevoy , Cyrill Gorcunov Cc: tarantool-patches@dev.tarantool.org 21.12.2020 20:11, Vladislav Shpilevoy пишет: >>> But I don't mind having this guard here if you want it. Only my thoughts. >>>> if (try_wait) { /* Wait until pending confirmations/rollbacks reach us. */ diff --git a/src/box/lua/ctl.c b/src/box/lua/ctl.c index bf26465e6..a3447f3e7 100644 --- a/src/box/lua/ctl.c +++ b/src/box/lua/ctl.c @@ -81,8 +81,8 @@ lbox_ctl_on_schema_init(struct lua_State *L) static int lbox_ctl_clear_synchro_queue(struct lua_State *L) { - (void) L; - box_clear_synchro_queue(true); + if (box_clear_synchro_queue(true) != 0) + return luaT_error(L); >>> Maybe better use nil + error object return way? I thought we still use it in the new code. >> Hm, I haven't seen us do that in lua/C. >> As far as I know, every box.* method throws a lua error in case of failure. >> I may miss something. Is there a reason for returning nil + error instead of >> throwing? > The only reason is that it is our new rule for writing Lua code. > http://www.tarantool.io/en/doc/latest/dev_guide/lua_style_guide/#error-handling > > Not every box function, really. > > As some examples I can remember box.session.push(), > box.unprepare, box.execute, box.prepare. > > There are also examples for non-box Lua methods. You > can grep by luaT_push_nil_and_error() to find almost all > usages in Lua C API. To find usages in .lua files you > can try to look for `return nil, err*`, but it won't > show everything. > > For me nil,err looks better, but I understand that there > is a lot of old code still using exceptions. And maybe we > would better stick to them. I suggest to ask in the chat > if in doubt. Thanks for the explanation. I'm still not sure whether we should change it to nil, err for `clear_synchro_queue` or not. Looks like after the chat discussion we should leve it as is (throw an error). -- Serge Petrenko