Re: [Tarantool-patches] [PATCH 1/4] box: add a single execution guard to clear_synchro_queue

[Serge Petrenko <sergepetrenko@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