From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
To: Mergen Imeev <imeevma@tarantool.org>
Cc: vdavydov.dev@gmail.com, tarantool-patches@freelists.org,
Kirill Yukhin <kyukhin@tarantool.org>
Subject: Re: [tarantool-patches] Re: [PATCH v1 1/1] netbox: follow-ups
Date: Tue, 25 Jun 2019 19:57:22 +0200 [thread overview]
Message-ID: <e8ef1c97-98eb-b02f-1929-c5eeacc8f9bf@tarantool.org> (raw)
In-Reply-To: <20190625161249.GA11738@tarantool.org>
LGTM.
On 25/06/2019 18:12, Mergen Imeev wrote:
> A bit of changes due to errors in release build:
>
> diff --git a/src/box/lua/net_box.c b/src/box/lua/net_box.c
> index 1b0cafb..001af95 100644
> --- a/src/box/lua/net_box.c
> +++ b/src/box/lua/net_box.c
> @@ -618,8 +618,7 @@ static int
> netbox_decode_select(struct lua_State *L)
> {
> uint32_t ctypeid;
> - int top = lua_gettop(L);
> - assert(top == 3);
> + assert(lua_gettop(L) == 3);
> struct tuple_format *format;
> if (lua_type(L, 3) == LUA_TCDATA)
> format = lbox_check_tuple_format(L, 3);
>
>
> On Tue, Jun 25, 2019 at 06:10:27PM +0300, Mergen Imeev wrote:
>> Hi! Thank you for review! I squashed your patch and added
>> some changed. Diff for these changes and new patch below.
>>
>> On Sat, Jun 22, 2019 at 04:21:58PM +0200, Vladislav Shpilevoy wrote:
>>> Hi!
>>>
>>> Thanks for your fixes. I appreciate that you decided
>>> to fix the patch after push. See 2 comments below.
>>>
>>>>> Thanks for the patch!
>>>>>
>>>>>> diff --git a/src/box/lua/net_box.c b/src/box/lua/net_box.c
>>>>>> @@ -618,6 +618,13 @@ static int
>>>>>> netbox_decode_select(struct lua_State *L)
>>>>>> {
>>>>>> uint32_t ctypeid;
>>>>>> + int top = lua_gettop(L);
>>>>>> + assert(top == 1 || top == 2);
>>>>>> + struct tuple_format *format;
>>>>>> + if (top == 2 && lua_type(L, 2) == LUA_TCDATA)
>>>>>> + format = lbox_check_tuple_format(L, 2);
>>>>>
>>>>> How is it possible, that we do not have a format here?
>>>>>
>>>>
>>>> it is possible since decode_call_16 calls select without format.
>>>
>>> 1. Oh, I forgot that call16 uses select decoder.
>>>
>>>> diff --git a/src/box/lua/net_box.lua b/src/box/lua/net_box.lua
>>>> index 251ad40..f7e1688 100644
>>>> --- a/src/box/lua/net_box.lua
>>>> +++ b/src/box/lua/net_box.lua
>>>> @@ -64,12 +64,12 @@ local function decode_data(raw_data)
>>>> local response, raw_end = decode(raw_data)
>>>> return response[IPROTO_DATA_KEY], raw_end
>>>> end
>>>> -local function decode_tuple(raw_data)
>>>> - local response, raw_end = internal.decode_select(raw_data)
>>>> +local function decode_tuple(raw_data, raw_data_end, format)
>>>> + local response, raw_end = internal.decode_select(raw_data, format)
>>>> return response[1], raw_end
>>>> end
>>>> -local function decode_get(raw_data)
>>>> - local body, raw_end = internal.decode_select(raw_data)
>>>> +local function decode_get(raw_data, raw_data_end, format)
>>>> + local body, raw_end = internal.decode_select(raw_data, format)
>>>> if body[2] then
>>>> return nil, raw_end, box.error.MORE_THAN_ONE_TUPLE
>>>> end
>>>> @@ -83,6 +83,12 @@ local function decode_push(raw_data)
>>>> local response, raw_end = decode(raw_data)
>>>> return response[IPROTO_DATA_KEY][1], raw_end
>>>> end
>>>> +local function decode_call_16(raw_data)
>>>> + return internal.decode_select(raw_data)
>>>> +end
>>>
>>> 2. Why do you need that wrapper now? You could keep
>>> direct reference to internal.decode_select in method_decoder
>>> table. Couldn't you?
>>>
>>>> +local function decode_select(raw_data, raw_data_end, format)
>>>> + return internal.decode_select(raw_data, format)
>>>> +end
>>>>
>>>
>>> See my proposals below and on the branch in a separate
>>> commit:
>>>
>>> ===================================================================
>>>
>>> diff --git a/src/box/lua/misc.cc b/src/box/lua/misc.cc
>>> index 3340070b0..a0a72aa28 100644
>>> --- a/src/box/lua/misc.cc
>>> +++ b/src/box/lua/misc.cc
>>> @@ -201,6 +201,7 @@ lbox_tuple_format_new(struct lua_State *L)
>>> if (fields == NULL) {
>>> diag_set(OutOfMemory, size, "region_alloc",
>>> "fields[i].name");
>>> + region_truncate(region, region_svp);
>>> return luaT_error(L);
>>> }
>>> memcpy(fields[i].name, name, len);
>>> diff --git a/src/box/lua/net_box.c b/src/box/lua/net_box.c
>>> index ad7bc6adf..a30105988 100644
>>> --- a/src/box/lua/net_box.c
>>> +++ b/src/box/lua/net_box.c
>>> @@ -619,7 +619,7 @@ netbox_decode_select(struct lua_State *L)
>>> {
>>> uint32_t ctypeid;
>>> int top = lua_gettop(L);
>>> - assert(top == 1 || top == 2);
>>> + assert(top == 1 || top == 2 || top == 3 && lua_isnil(L, 3));
>>> struct tuple_format *format;
>>> if (top == 2 && lua_type(L, 2) == LUA_TCDATA)
>>> format = lbox_check_tuple_format(L, 2);
>>> diff --git a/src/box/lua/net_box.lua b/src/box/lua/net_box.lua
>>> index f7e1688bb..c0e85e1c9 100644
>>> --- a/src/box/lua/net_box.lua
>>> +++ b/src/box/lua/net_box.lua
>>> @@ -83,9 +83,6 @@ local function decode_push(raw_data)
>>> local response, raw_end = decode(raw_data)
>>> return response[IPROTO_DATA_KEY][1], raw_end
>>> end
>>> -local function decode_call_16(raw_data)
>>> - return internal.decode_select(raw_data)
>>> -end
>>> local function decode_select(raw_data, raw_data_end, format)
>>> return internal.decode_select(raw_data, format)
>>> end
>>> @@ -116,7 +113,7 @@ local method_encoder = {
>>>
>>> local method_decoder = {
>>> ping = decode_nil,
>>> - call_16 = decode_call_16,
>>> + call_16 = internal.decode_select,
>>> call_17 = decode_data,
>>> eval = decode_data,
>>> insert = decode_tuple,
>>
>> Thank you.
>>
>> Diff:
>>
>> diff --git a/src/box/lua/net_box.c b/src/box/lua/net_box.c
>> index f232d5d..1b0cafb 100644
>> --- a/src/box/lua/net_box.c
>> +++ b/src/box/lua/net_box.c
>> @@ -619,10 +619,10 @@ netbox_decode_select(struct lua_State *L)
>> {
>> uint32_t ctypeid;
>> int top = lua_gettop(L);
>> - assert(top == 1 || top == 2 || (top == 3 && lua_isnil(L, 3)));
>> + assert(top == 3);
>> struct tuple_format *format;
>> - if (top == 2 && lua_type(L, 2) == LUA_TCDATA)
>> - format = lbox_check_tuple_format(L, 2);
>> + if (lua_type(L, 3) == LUA_TCDATA)
>> + format = lbox_check_tuple_format(L, 3);
>> else
>> format = tuple_format_runtime;
>> const char *data = *(const char **)luaL_checkcdata(L, 1, &ctypeid);
>> diff --git a/src/box/lua/net_box.lua b/src/box/lua/net_box.lua
>> index c0e85e1..2892bb3 100644
>> --- a/src/box/lua/net_box.lua
>> +++ b/src/box/lua/net_box.lua
>> @@ -65,11 +65,11 @@ local function decode_data(raw_data)
>> return response[IPROTO_DATA_KEY], raw_end
>> end
>> local function decode_tuple(raw_data, raw_data_end, format)
>> - local response, raw_end = internal.decode_select(raw_data, format)
>> + local response, raw_end = internal.decode_select(raw_data, nil, format)
>> return response[1], raw_end
>> end
>> local function decode_get(raw_data, raw_data_end, format)
>> - local body, raw_end = internal.decode_select(raw_data, format)
>> + local body, raw_end = internal.decode_select(raw_data, nil, format)
>> if body[2] then
>> return nil, raw_end, box.error.MORE_THAN_ONE_TUPLE
>> end
>> @@ -83,9 +83,6 @@ local function decode_push(raw_data)
>> local response, raw_end = decode(raw_data)
>> return response[IPROTO_DATA_KEY][1], raw_end
>> end
>> -local function decode_select(raw_data, raw_data_end, format)
>> - return internal.decode_select(raw_data, format)
>> -end
>>
>> local method_encoder = {
>> ping = internal.encode_ping,
>> @@ -121,7 +118,7 @@ local method_decoder = {
>> delete = decode_tuple,
>> update = decode_tuple,
>> upsert = decode_nil,
>> - select = decode_select,
>> + select = internal.decode_select,
>> execute = internal.decode_execute,
>> get = decode_get,
>> min = decode_get,
>>
>>
>> New patch:
>>
>> From f0978a19f33ca5879e4a7f419efb74b8db37f79a Mon Sep 17 00:00:00 2001
>> Date: Sat, 22 Jun 2019 12:07:15 +0300
>> Subject: [PATCH] netbox: optimize netbox requests
>>
>> This patch optimizes some netbox requests that were slowed down in
>> commit 665fc3a19c.
>>
>> Follow-up #2978
>>
>> diff --git a/src/box/lua/misc.cc b/src/box/lua/misc.cc
>> index 54f1d7a..7b8b9dc 100644
>> --- a/src/box/lua/misc.cc
>> +++ b/src/box/lua/misc.cc
>> @@ -175,10 +175,10 @@ static int
>> lbox_tuple_format_new(struct lua_State *L)
>> {
>> assert(CTID_STRUCT_TUPLE_FORMAT_PTR != 0);
>> - if (lua_gettop(L) == 0)
>> + int top = lua_gettop(L);
>> + if (top == 0)
>> return lbox_push_tuple_format(L, tuple_format_runtime);
>> - if (lua_gettop(L) > 1 || ! lua_istable(L, 1))
>> - return luaL_error(L, "Usage: box.internal.format_new(format)");
>> + assert(top == 1 && lua_istable(L, 1));
>> uint32_t count = lua_objlen(L, 1);
>> if (count == 0)
>> return lbox_push_tuple_format(L, tuple_format_runtime);
>> @@ -204,25 +204,19 @@ lbox_tuple_format_new(struct lua_State *L)
>> if (! lua_isnil(L, -1)) {
>> const char *type_name = lua_tolstring(L, -1, &len);
>> fields[i].type = field_type_by_name(type_name, len);
>> - if (fields[i].type == field_type_MAX) {
>> - const char *err =
>> - "field %d has unknown field type";
>> - return luaL_error(L, tt_sprintf(err, i + 1));
>> - }
>> + assert(fields[i].type != field_type_MAX);
>> }
>> lua_pop(L, 1);
>>
>> lua_pushstring(L, "name");
>> lua_gettable(L, -2);
>> - if (lua_isnil(L, -1)) {
>> - return luaL_error(L, tt_sprintf("field %d name is not "
>> - "specified", i + 1));
>> - }
>> + assert(! lua_isnil(L, -1));
>> const char *name = lua_tolstring(L, -1, &len);
>> fields[i].name = (char *)region_alloc(region, len + 1);
>> if (fields == NULL) {
>> diag_set(OutOfMemory, size, "region_alloc",
>> "fields[i].name");
>> + region_truncate(region, region_svp);
>> return luaT_error(L);
>> }
>> memcpy(fields[i].name, name, len);
>> diff --git a/src/box/lua/net_box.c b/src/box/lua/net_box.c
>> index ad7bc6a..1b0cafb 100644
>> --- a/src/box/lua/net_box.c
>> +++ b/src/box/lua/net_box.c
>> @@ -619,10 +619,10 @@ netbox_decode_select(struct lua_State *L)
>> {
>> uint32_t ctypeid;
>> int top = lua_gettop(L);
>> - assert(top == 1 || top == 2);
>> + assert(top == 3);
>> struct tuple_format *format;
>> - if (top == 2 && lua_type(L, 2) == LUA_TCDATA)
>> - format = lbox_check_tuple_format(L, 2);
>> + if (lua_type(L, 3) == LUA_TCDATA)
>> + format = lbox_check_tuple_format(L, 3);
>> else
>> format = tuple_format_runtime;
>> const char *data = *(const char **)luaL_checkcdata(L, 1, &ctypeid);
>> diff --git a/src/box/lua/net_box.lua b/src/box/lua/net_box.lua
>> index 8f31712..2892bb3 100644
>> --- a/src/box/lua/net_box.lua
>> +++ b/src/box/lua/net_box.lua
>> @@ -25,6 +25,7 @@ local check_primary_index = box.internal.check_primary_index
>>
>> local communicate = internal.communicate
>> local encode_auth = internal.encode_auth
>> +local encode_select = internal.encode_select
>> local decode_greeting = internal.decode_greeting
>>
>> local TIMEOUT_INFINITY = 500 * 365 * 86400
>> @@ -63,12 +64,12 @@ local function decode_data(raw_data)
>> local response, raw_end = decode(raw_data)
>> return response[IPROTO_DATA_KEY], raw_end
>> end
>> -local function decode_tuple(raw_data, raw_data_end, args)
>> - local response, raw_end = internal.decode_select(raw_data, args.format)
>> +local function decode_tuple(raw_data, raw_data_end, format)
>> + local response, raw_end = internal.decode_select(raw_data, nil, format)
>> return response[1], raw_end
>> end
>> -local function decode_get(raw_data, raw_data_end, args)
>> - local body, raw_end = internal.decode_select(raw_data, args.format)
>> +local function decode_get(raw_data, raw_data_end, format)
>> + local body, raw_end = internal.decode_select(raw_data, nil, format)
>> if body[2] then
>> return nil, raw_end, box.error.MORE_THAN_ONE_TUPLE
>> end
>> @@ -82,70 +83,23 @@ local function decode_push(raw_data)
>> local response, raw_end = decode(raw_data)
>> return response[IPROTO_DATA_KEY][1], raw_end
>> end
>> -local function decode_call_16(raw_data)
>> - return internal.decode_select(raw_data)
>> -end
>> -local function decode_select(raw_data, raw_data_end, args)
>> - return internal.decode_select(raw_data, args.format)
>> -end
>> -
>> -local function encode_call(send_buf, id, args)
>> - return internal.encode_call(send_buf, id, args.func_name, args.args)
>> -end
>> -local function encode_call_16(send_buf, id, args)
>> - return internal.encode_call_16(send_buf, id, args.func_name, args.args)
>> -end
>> -local function encode_ping(send_buf, id, ...)
>> - return internal.encode_ping(send_buf, id, ...)
>> -end
>> -local function encode_eval(send_buf, id, args)
>> - return internal.encode_eval(send_buf, id, args.code, args.args)
>> -end
>> -local function encode_insert(send_buf, id, args)
>> - return internal.encode_insert(send_buf, id, args.space_id, args.tuple)
>> -end
>> -local function encode_replace(send_buf, id, args)
>> - return internal.encode_replace(send_buf, id, args.space_id, args.tuple)
>> -end
>> -local function encode_delete(send_buf, id, args)
>> - return internal.encode_delete(send_buf, id, args.space_id, args.index_id,
>> - args.key)
>> -end
>> -local function encode_update(send_buf, id, args)
>> - return internal.encode_update(send_buf, id, args.space_id, args.index_id,
>> - args.key, args.oplist)
>> -end
>> -local function encode_upsert(send_buf, id, args)
>> - return internal.encode_upsert(send_buf, id, args.space_id, args.key,
>> - args.oplist)
>> -end
>> -local function encode_select(send_buf, id, args)
>> - return internal.encode_select(send_buf, id, args.space_id, args.index_id,
>> - args.iterator, args.offset, args.limit,
>> - args.key)
>> -end
>> -local function encode_execute(send_buf, id, args)
>> - return internal.encode_execute(send_buf, id, args.query, args.parameters,
>> - args.sql_opts)
>> -end
>> -
>>
>> local method_encoder = {
>> - ping = encode_ping,
>> - call_16 = encode_call_16,
>> - call_17 = encode_call,
>> - eval = encode_eval,
>> - insert = encode_insert,
>> - replace = encode_replace,
>> - delete = encode_delete,
>> - update = encode_update,
>> - upsert = encode_upsert,
>> - select = encode_select,
>> - execute = encode_execute,
>> - get = encode_select,
>> - min = encode_select,
>> - max = encode_select,
>> - count = encode_call,
>> + ping = internal.encode_ping,
>> + call_16 = internal.encode_call_16,
>> + call_17 = internal.encode_call,
>> + eval = internal.encode_eval,
>> + insert = internal.encode_insert,
>> + replace = internal.encode_replace,
>> + delete = internal.encode_delete,
>> + update = internal.encode_update,
>> + upsert = internal.encode_upsert,
>> + select = internal.encode_select,
>> + execute = internal.encode_execute,
>> + get = internal.encode_select,
>> + min = internal.encode_select,
>> + max = internal.encode_select,
>> + count = internal.encode_call,
>> -- inject raw data into connection, used by console and tests
>> inject = function(buf, id, bytes)
>> local ptr = buf:reserve(#bytes)
>> @@ -156,7 +110,7 @@ local method_encoder = {
>>
>> local method_decoder = {
>> ping = decode_nil,
>> - call_16 = decode_call_16,
>> + call_16 = internal.decode_select,
>> call_17 = decode_data,
>> eval = decode_data,
>> insert = decode_tuple,
>> @@ -164,7 +118,7 @@ local method_decoder = {
>> delete = decode_tuple,
>> update = decode_tuple,
>> upsert = decode_nil,
>> - select = decode_select,
>> + select = internal.decode_select,
>> execute = internal.decode_execute,
>> get = decode_get,
>> min = decode_get,
>> @@ -532,7 +486,7 @@ local function create_transport(host, port, user, password, callback,
>> -- @retval not nil Future object.
>> --
>> local function perform_async_request(buffer, skip_header, method, on_push,
>> - on_push_ctx, args)
>> + on_push_ctx, request_ctx, ...)
>> if state ~= 'active' and state ~= 'fetch_schema' then
>> return nil, box.error.new({code = last_errno or E_NO_CONNECTION,
>> reason = last_error})
>> @@ -543,11 +497,11 @@ local function create_transport(host, port, user, password, callback,
>> worker_fiber:wakeup()
>> end
>> local id = next_request_id
>> - method_encoder[method](send_buf, id, args)
>> + method_encoder[method](send_buf, id, ...)
>> next_request_id = next_id(id)
>> -- Request in most cases has maximum 10 members:
>> -- method, buffer, skip_header, id, cond, errno, response,
>> - -- on_push, on_push_ctx and args.
>> + -- on_push, on_push_ctx and ctx.
>> local request = setmetatable(table_new(0, 10), request_mt)
>> request.method = method
>> request.buffer = buffer
>> @@ -557,7 +511,7 @@ local function create_transport(host, port, user, password, callback,
>> requests[id] = request
>> request.on_push = on_push
>> request.on_push_ctx = on_push_ctx
>> - request.args = args
>> + request.ctx = request_ctx
>> return request
>> end
>>
>> @@ -567,10 +521,10 @@ local function create_transport(host, port, user, password, callback,
>> -- @retval not nil Response object.
>> --
>> local function perform_request(timeout, buffer, skip_header, method,
>> - on_push, on_push_ctx, args)
>> + on_push, on_push_ctx, request_ctx, ...)
>> local request, err =
>> perform_async_request(buffer, skip_header, method, on_push,
>> - on_push_ctx, args)
>> + on_push_ctx, request_ctx, ...)
>> if not request then
>> return nil, err
>> end
>> @@ -629,8 +583,7 @@ local function create_transport(host, port, user, password, callback,
>> -- Decode xrow.body[DATA] to Lua objects
>> if status == IPROTO_OK_KEY then
>> request.response, real_end, request.errno =
>> - method_decoder[request.method](body_rpos, body_end,
>> - request.args)
>> + method_decoder[request.method](body_rpos, body_end, request.ctx)
>> assert(real_end == body_end, "invalid body length")
>> requests[id] = nil
>> request.id = nil
>> @@ -787,14 +740,13 @@ local function create_transport(host, port, user, password, callback,
>> local select3_id = new_request_id()
>> local response = {}
>> -- fetch everything from space _vspace, 2 = ITER_ALL
>> - internal.encode_select(send_buf, select1_id, VSPACE_ID, 0, 2, 0,
>> - 0xFFFFFFFF, nil)
>> + encode_select(send_buf, select1_id, VSPACE_ID, 0, 2, 0, 0xFFFFFFFF, nil)
>> -- fetch everything from space _vindex, 2 = ITER_ALL
>> - internal.encode_select(send_buf, select2_id, VINDEX_ID, 0, 2, 0,
>> - 0xFFFFFFFF, nil)
>> + encode_select(send_buf, select2_id, VINDEX_ID, 0, 2, 0, 0xFFFFFFFF, nil)
>> -- fetch everything from space _vcollation, 2 = ITER_ALL
>> - internal.encode_select(send_buf, select3_id, VCOLLATION_ID, 0, 2, 0,
>> - 0xFFFFFFFF, nil)
>> + encode_select(send_buf, select3_id, VCOLLATION_ID, 0, 2, 0, 0xFFFFFFFF,
>> + nil)
>> +
>> schema_version = nil -- any schema_version will do provided that
>> -- it is consistent across responses
>> repeat
>> @@ -1113,7 +1065,7 @@ function remote_methods:wait_connected(timeout)
>> return self._transport.wait_state('active', timeout)
>> end
>>
>> -function remote_methods:_request(method, opts, args)
>> +function remote_methods:_request(method, opts, request_ctx, ...)
>> local transport = self._transport
>> local on_push, on_push_ctx, buffer, skip_header, deadline
>> -- Extract options, set defaults, check if the request is
>> @@ -1127,7 +1079,8 @@ function remote_methods:_request(method, opts, args)
>> end
>> local res, err =
>> transport.perform_async_request(buffer, skip_header, method,
>> - table.insert, {}, args)
>> + table.insert, {}, request_ctx,
>> + ...)
>> if err then
>> box.error(err)
>> end
>> @@ -1155,7 +1108,7 @@ function remote_methods:_request(method, opts, args)
>> end
>> local res, err = transport.perform_request(timeout, buffer, skip_header,
>> method, on_push, on_push_ctx,
>> - args)
>> + request_ctx, ...)
>> if err then
>> box.error(err)
>> end
>> @@ -1182,16 +1135,14 @@ end
>> -- @deprecated since 1.7.4
>> function remote_methods:call_16(func_name, ...)
>> check_remote_arg(self, 'call')
>> - local args = {func_name = tostring(func_name), args = {...}}
>> - return (self:_request('call_16', nil, args))
>> + return (self:_request('call_16', nil, nil, tostring(func_name), {...}))
>> end
>>
>> -function remote_methods:call(func_name, call_args, opts)
>> +function remote_methods:call(func_name, args, opts)
>> check_remote_arg(self, 'call')
>> - check_call_args(call_args)
>> - call_args = call_args or {}
>> - local args = {func_name = tostring(func_name), args = call_args}
>> - local res = self:_request('call_17', opts, args)
>> + check_call_args(args)
>> + args = args or {}
>> + local res = self:_request('call_17', opts, nil, tostring(func_name), args)
>> if type(res) ~= 'table' or opts and opts.is_async then
>> return res
>> end
>> @@ -1201,16 +1152,14 @@ end
>> -- @deprecated since 1.7.4
>> function remote_methods:eval_16(code, ...)
>> check_remote_arg(self, 'eval')
>> - local args = {code = code, args = {...}}
>> - return unpack((self:_request('eval', nil, args)))
>> + return unpack((self:_request('eval', nil, nil, code, {...})))
>> end
>>
>> -function remote_methods:eval(code, eval_args, opts)
>> +function remote_methods:eval(code, args, opts)
>> check_remote_arg(self, 'eval')
>> - check_eval_args(eval_args)
>> - eval_args = eval_args or {}
>> - local args = {code = code, args = eval_args}
>> - local res = self:_request('eval', opts, args)
>> + check_eval_args(args)
>> + args = args or {}
>> + local res = self:_request('eval', opts, nil, code, args)
>> if type(res) ~= 'table' or opts and opts.is_async then
>> return res
>> end
>> @@ -1222,9 +1171,8 @@ function remote_methods:execute(query, parameters, sql_opts, netbox_opts)
>> if sql_opts ~= nil then
>> box.error(box.error.UNSUPPORTED, "execute", "options")
>> end
>> - local args = {query =query, parameters = parameters or {},
>> - sql_opts = sql_opts or {}}
>> - return self:_request('execute', netbox_opts, args)
>> + return self:_request('execute', netbox_opts, nil, query, parameters or {},
>> + sql_opts or {})
>> end
>>
>> function remote_methods:wait_state(state, timeout)
>> @@ -1369,11 +1317,12 @@ function console_methods:eval(line, timeout)
>> end
>> if self.protocol == 'Binary' then
>> local loader = 'return require("console").eval(...)'
>> - local args = {code = loader, args = {line}}
>> - res, err = pr(timeout, nil, false, 'eval', nil, nil, args)
>> + res, err = pr(timeout, nil, false, 'eval', nil, nil, nil, loader,
>> + {line})
>> else
>> assert(self.protocol == 'Lua console')
>> - res, err = pr(timeout, nil, false, 'inject', nil, nil, line..'$EOF$\n')
>> + res, err = pr(timeout, nil, false, 'inject', nil, nil, nil,
>> + line..'$EOF$\n')
>> end
>> if err then
>> box.error(err)
>> @@ -1392,16 +1341,12 @@ space_metatable = function(remote)
>>
>> function methods:insert(tuple, opts)
>> check_space_arg(self, 'insert')
>> - local args = {space_id = self.id, tuple = tuple,
>> - format = self._format_cdata}
>> - return remote:_request('insert', opts, args)
>> + return remote:_request('insert', opts, self._format_cdata, self.id, tuple)
>> end
>>
>> function methods:replace(tuple, opts)
>> check_space_arg(self, 'replace')
>> - local args = {space_id = self.id, tuple = tuple,
>> - format = self._format_cdata}
>> - return remote:_request('replace', opts, args)
>> + return remote:_request('replace', opts, self._format_cdata, self.id, tuple)
>> end
>>
>> function methods:select(key, opts)
>> @@ -1421,8 +1366,8 @@ space_metatable = function(remote)
>>
>> function methods:upsert(key, oplist, opts)
>> check_space_arg(self, 'upsert')
>> - local args = {space_id = self.id, key = key, oplist = oplist}
>> - return nothing_or_data(remote:_request('upsert', opts, args))
>> + return nothing_or_data(remote:_request('upsert', opts, nil, self.id, key,
>> + oplist))
>> end
>>
>> function methods:get(key, opts)
>> @@ -1451,10 +1396,9 @@ index_metatable = function(remote)
>> local iterator = check_iterator_type(opts, key_is_nil)
>> local offset = tonumber(opts and opts.offset) or 0
>> local limit = tonumber(opts and opts.limit) or 0xFFFFFFFF
>> - local args = {space_id = self.space.id, index_id = self.id,
>> - iterator = iterator, offset = offset, limit = limit,
>> - key = key, format = self.space._format_cdata}
>> - return (remote:_request('select', opts, args))
>> + return (remote:_request('select', opts, self.space._format_cdata,
>> + self.space.id, self.id, iterator, offset,
>> + limit, key))
>> end
>>
>> function methods:get(key, opts)
>> @@ -1462,10 +1406,10 @@ index_metatable = function(remote)
>> if opts and opts.buffer then
>> error("index:get() doesn't support `buffer` argument")
>> end
>> - local args = {space_id = self.space.id, index_id = self.id,
>> - iterator = box.index.EQ, offset = 0, limit = 2, key = key,
>> - format = self.space._format_cdata}
>> - return nothing_or_data(remote:_request('get', opts, args))
>> + return nothing_or_data(remote:_request('get', opts,
>> + self.space._format_cdata,
>> + self.space.id, self.id,
>> + box.index.EQ, 0, 2, key))
>> end
>>
>> function methods:min(key, opts)
>> @@ -1473,10 +1417,10 @@ index_metatable = function(remote)
>> if opts and opts.buffer then
>> error("index:min() doesn't support `buffer` argument")
>> end
>> - local args = {space_id = self.space.id, index_id = self.id,
>> - iterator = box.index.GE, offset = 0, limit = 1, key = key,
>> - format = self.space._format_cdata}
>> - return nothing_or_data(remote:_request('min', opts, args))
>> + return nothing_or_data(remote:_request('min', opts,
>> + self.space._format_cdata,
>> + self.space.id, self.id,
>> + box.index.GE, 0, 1, key))
>> end
>>
>> function methods:max(key, opts)
>> @@ -1484,10 +1428,10 @@ index_metatable = function(remote)
>> if opts and opts.buffer then
>> error("index:max() doesn't support `buffer` argument")
>> end
>> - local args = {space_id = self.space.id, index_id = self.id,
>> - iterator = box.index.LE, offset = 0, limit = 1, key = key,
>> - format = self.space._format_cdata}
>> - return nothing_or_data(remote:_request('max', opts, args))
>> + return nothing_or_data(remote:_request('max', opts,
>> + self.space._format_cdata,
>> + self.space.id, self.id,
>> + box.index.LE, 0, 1, key))
>> end
>>
>> function methods:count(key, opts)
>> @@ -1497,22 +1441,22 @@ index_metatable = function(remote)
>> end
>> local code = string.format('box.space.%s.index.%s:count',
>> self.space.name, self.name)
>> - local args = {func_name = code, args = { key, opts }}
>> - return remote:_request('count', opts, args)
>> + return remote:_request('count', opts, nil, code, { key, opts })
>> end
>>
>> function methods:delete(key, opts)
>> check_index_arg(self, 'delete')
>> - local args = {space_id = self.space.id, index_id = self.id, key = key,
>> - format = self.space._format_cdata}
>> - return nothing_or_data(remote:_request('delete', opts, args))
>> + return nothing_or_data(remote:_request('delete', opts,
>> + self.space._format_cdata,
>> + self.space.id, self.id, key))
>> end
>>
>> function methods:update(key, oplist, opts)
>> check_index_arg(self, 'update')
>> - local args = {space_id = self.space.id, index_id = self.id, key = key,
>> - oplist = oplist, format = self.space._format_cdata}
>> - return nothing_or_data(remote:_request('update', opts, args))
>> + return nothing_or_data(remote:_request('update', opts,
>> + self.space._format_cdata,
>> + self.space.id, self.id, key,
>> + oplist))
>> end
>>
>> return { __index = methods, __metatable = false }
>> diff --git a/test/box/access.result b/test/box/access.result
>> index e3d4036..ca2531f 100644
>> --- a/test/box/access.result
>> +++ b/test/box/access.result
>> @@ -890,24 +890,15 @@ LISTEN = require('uri').parse(box.cfg.listen)
>> c = (require 'net.box').connect(LISTEN.host, LISTEN.service)
>> ---
>> ...
>> -args = {space_id=1, index_id=0, iterator=box.index.EQ, offset=0, limit=0xFFFFFFFF, key={}}
>> ----
>> -...
>> -c:_request("select", nil, args)
>> +c:_request("select", nil, nil, 1, box.index.EQ, 0, 0, 0xFFFFFFFF, {})
>> ---
>> - error: Space '1' does not exist
>> ...
>> -args.space_id = 65537
>> ----
>> -...
>> -c:_request("select", nil, args)
>> +c:_request("select", nil, nil, 65537, box.index.EQ, 0, 0, 0xFFFFFFFF, {})
>> ---
>> - error: Space '65537' does not exist
>> ...
>> -args.space_id = 4294967295
>> ----
>> -...
>> -c:_request("select", nil, args)
>> +c:_request("select", nil, nil, 4294967295, box.index.EQ, 0, 0, 0xFFFFFFFF, {})
>> ---
>> - error: Space '4294967295' does not exist
>> ...
>> diff --git a/test/box/access.test.lua b/test/box/access.test.lua
>> index c63152b..c1ba002 100644
>> --- a/test/box/access.test.lua
>> +++ b/test/box/access.test.lua
>> @@ -343,12 +343,9 @@ box.schema.func.drop(name)
>> -- very large space id, no crash occurs.
>> LISTEN = require('uri').parse(box.cfg.listen)
>> c = (require 'net.box').connect(LISTEN.host, LISTEN.service)
>> -args = {space_id=1, index_id=0, iterator=box.index.EQ, offset=0, limit=0xFFFFFFFF, key={}}
>> -c:_request("select", nil, args)
>> -args.space_id = 65537
>> -c:_request("select", nil, args)
>> -args.space_id = 4294967295
>> -c:_request("select", nil, args)
>> +c:_request("select", nil, nil, 1, box.index.EQ, 0, 0, 0xFFFFFFFF, {})
>> +c:_request("select", nil, nil, 65537, box.index.EQ, 0, 0, 0xFFFFFFFF, {})
>> +c:_request("select", nil, nil, 4294967295, box.index.EQ, 0, 0, 0xFFFFFFFF, {})
>> c:close()
>>
>> session = box.session
>> diff --git a/test/box/misc.result b/test/box/misc.result
>> index ec2c4fa..c293ccc 100644
>> --- a/test/box/misc.result
>> +++ b/test/box/misc.result
>> @@ -1273,66 +1273,6 @@ s:drop()
>> ...
>> --
>> -- gh-2978: Function to parse space format.
>> ---
>> --- Error: no field name:
>> -format = {}
>> ----
>> -...
>> -format[1] = {}
>> ----
>> -...
>> -format[1].type = 'unsigned'
>> ----
>> -...
>> -box.internal.new_tuple_format(format)
>> ----
>> -- error: field 1 name is not specified
>> -...
>> --- Error: duplicate field name:
>> -format[1].name = 'aaa'
>> ----
>> -...
>> -format[2] = {}
>> ----
>> -...
>> -format[2].name = 'aaa'
>> ----
>> -...
>> -format[2].type = 'any'
>> ----
>> -...
>> -box.internal.new_tuple_format(format)
>> ----
>> -- error: Space field 'aaa' is duplicate
>> -...
>> --- Error: wrong field type:
>> -format[2].name = 'bbb'
>> ----
>> -...
>> -format[3] = {}
>> ----
>> -...
>> -format[3].name = 'ccc'
>> ----
>> -...
>> -format[3].type = 'something'
>> ----
>> -...
>> -box.internal.new_tuple_format(format)
>> ----
>> -- error: field 3 has unknown field type
>> -...
>> --- Error: too many arguments:
>> -box.internal.new_tuple_format(format, format)
>> ----
>> -- error: 'Usage: box.internal.format_new(format)'
>> -...
>> --- Error: wrong argument:
>> -box.internal.new_tuple_format(1)
>> ----
>> -- error: 'Usage: box.internal.format_new(format)'
>> -...
>> ---
>> -- In next tests we should receive cdata("struct tuple_format *").
>> -- We do not have a way to check cdata in Lua, but there should be
>> -- no errors.
>> @@ -1342,7 +1282,13 @@ tuple_format = box.internal.new_tuple_format()
>> ---
>> ...
>> -- If no type that type == "any":
>> -format[3].type = nil
>> +format = {}
>> +---
>> +...
>> +format[1] = {}
>> +---
>> +...
>> +format[1].name = 'aaa'
>> ---
>> ...
>> tuple_format = box.internal.new_tuple_format(format)
>> @@ -1353,7 +1299,7 @@ tuple_format = box.internal.new_tuple_format(box.space._space:format())
>> ---
>> ...
>> -- Check is_nullable option fo field
>> -format[2].is_nullable = true
>> +format[1].is_nullable = true
>> ---
>> ...
>> tuple_format = box.internal.new_tuple_format(format)
>> diff --git a/test/box/misc.test.lua b/test/box/misc.test.lua
>> index 94a6450..a777777 100644
>> --- a/test/box/misc.test.lua
>> +++ b/test/box/misc.test.lua
>> @@ -353,38 +353,8 @@ lsn == expected_lsn
>> box.cfg{too_long_threshold = too_long_threshold}
>> s:drop()
>>
>> -
>> --
>> -- gh-2978: Function to parse space format.
>> ---
>> -
>> --- Error: no field name:
>> -format = {}
>> -format[1] = {}
>> -format[1].type = 'unsigned'
>> -box.internal.new_tuple_format(format)
>> -
>> --- Error: duplicate field name:
>> -format[1].name = 'aaa'
>> -format[2] = {}
>> -format[2].name = 'aaa'
>> -format[2].type = 'any'
>> -box.internal.new_tuple_format(format)
>> -
>> --- Error: wrong field type:
>> -format[2].name = 'bbb'
>> -format[3] = {}
>> -format[3].name = 'ccc'
>> -format[3].type = 'something'
>> -box.internal.new_tuple_format(format)
>> -
>> --- Error: too many arguments:
>> -box.internal.new_tuple_format(format, format)
>> -
>> --- Error: wrong argument:
>> -box.internal.new_tuple_format(1)
>> -
>> ---
>> -- In next tests we should receive cdata("struct tuple_format *").
>> -- We do not have a way to check cdata in Lua, but there should be
>> -- no errors.
>> @@ -394,12 +364,14 @@ box.internal.new_tuple_format(1)
>> tuple_format = box.internal.new_tuple_format()
>>
>> -- If no type that type == "any":
>> -format[3].type = nil
>> +format = {}
>> +format[1] = {}
>> +format[1].name = 'aaa'
>> tuple_format = box.internal.new_tuple_format(format)
>>
>> -- Function space:format() without arguments returns valid format:
>> tuple_format = box.internal.new_tuple_format(box.space._space:format())
>>
>> -- Check is_nullable option fo field
>> -format[2].is_nullable = true
>> +format[1].is_nullable = true
>> tuple_format = box.internal.new_tuple_format(format)
>> diff --git a/test/box/net.box.result b/test/box/net.box.result
>> index 1248c64..efaed11 100644
>> --- a/test/box/net.box.result
>> +++ b/test/box/net.box.result
>> @@ -25,11 +25,11 @@ test_run:cmd("setopt delimiter ';'")
>> - true
>> ...
>> function x_select(cn, space_id, index_id, iterator, offset, limit, key, opts)
>> - local args = {space_id = space_id, index_id = index_id, iterator = iterator,
>> - offset = offset, limit = limit, key = key}
>> - return cn:_request('select', opts, args)
>> + local ret = cn:_request('select', opts, nil, space_id, index_id, iterator,
>> + offset, limit, key)
>> + return ret
>> end
>> -function x_fatal(cn) cn._transport.perform_request(nil, nil, false, 'inject', nil, nil, '\x80') end
>> +function x_fatal(cn) cn._transport.perform_request(nil, nil, false, 'inject', nil, nil, nil, '\x80') end
>> test_run:cmd("setopt delimiter ''");
>> ---
>> ...
>> @@ -2815,7 +2815,7 @@ c.space.test:delete{1}
>> --
>> -- Break a connection to test reconnect_after.
>> --
>> -_ = c._transport.perform_request(nil, nil, false, 'inject', nil, nil, '\x80')
>> +_ = c._transport.perform_request(nil, nil, false, 'inject', nil, nil, nil, '\x80')
>> ---
>> ...
>> c.state
>> @@ -3456,7 +3456,7 @@ c = net:connect(box.cfg.listen, {reconnect_after = 0.01})
>> future = c:call('long_function', {1, 2, 3}, {is_async = true})
>> ---
>> ...
>> -_ = c._transport.perform_request(nil, nil, false, 'inject', nil, nil, '\x80')
>> +_ = c._transport.perform_request(nil, nil, false, 'inject', nil, nil, nil, '\x80')
>> ---
>> ...
>> while not c:is_connected() do fiber.sleep(0.01) end
>> @@ -3689,7 +3689,7 @@ c:ping()
>> -- new attempts to read any data - the connection is closed
>> -- already.
>> --
>> -f = fiber.create(c._transport.perform_request, nil, nil, false, 'call_17', nil, nil, 'long', {}) c._transport.perform_request(nil, nil, false, 'inject', nil, nil, '\x80')
>> +f = fiber.create(c._transport.perform_request, nil, nil, false, 'call_17', nil, nil, nil, 'long', {}) c._transport.perform_request(nil, nil, false, 'inject', nil, nil, nil, '\x80')
>> ---
>> ...
>> while f:status() ~= 'dead' do fiber.sleep(0.01) end
>> @@ -3708,7 +3708,7 @@ c = net:connect(box.cfg.listen)
>> data = msgpack.encode(18400000000000000000)..'aaaaaaa'
>> ---
>> ...
>> -c._transport.perform_request(nil, nil, false, 'inject', nil, nil, data)
>> +c._transport.perform_request(nil, nil, false, 'inject', nil, nil, nil, data)
>> ---
>> - null
>> - Peer closed
>> diff --git a/test/box/net.box.test.lua b/test/box/net.box.test.lua
>> index f222ad9..a140990 100644
>> --- a/test/box/net.box.test.lua
>> +++ b/test/box/net.box.test.lua
>> @@ -8,11 +8,11 @@ test_run:cmd("push filter ".."'\\.lua.*:[0-9]+: ' to '.lua...\"]:<line>: '")
>>
>> test_run:cmd("setopt delimiter ';'")
>> function x_select(cn, space_id, index_id, iterator, offset, limit, key, opts)
>> - local args = {space_id = space_id, index_id = index_id, iterator = iterator,
>> - offset = offset, limit = limit, key = key}
>> - return cn:_request('select', opts, args)
>> + local ret = cn:_request('select', opts, nil, space_id, index_id, iterator,
>> + offset, limit, key)
>> + return ret
>> end
>> -function x_fatal(cn) cn._transport.perform_request(nil, nil, false, 'inject', nil, nil, '\x80') end
>> +function x_fatal(cn) cn._transport.perform_request(nil, nil, false, 'inject', nil, nil, nil, '\x80') end
>> test_run:cmd("setopt delimiter ''");
>>
>> LISTEN = require('uri').parse(box.cfg.listen)
>> @@ -1155,7 +1155,7 @@ c.space.test:delete{1}
>> --
>> -- Break a connection to test reconnect_after.
>> --
>> -_ = c._transport.perform_request(nil, nil, false, 'inject', nil, nil, '\x80')
>> +_ = c._transport.perform_request(nil, nil, false, 'inject', nil, nil, nil, '\x80')
>> c.state
>> while not c:is_connected() do fiber.sleep(0.01) end
>> c:ping()
>> @@ -1388,7 +1388,7 @@ finalize_long()
>> --
>> c = net:connect(box.cfg.listen, {reconnect_after = 0.01})
>> future = c:call('long_function', {1, 2, 3}, {is_async = true})
>> -_ = c._transport.perform_request(nil, nil, false, 'inject', nil, nil, '\x80')
>> +_ = c._transport.perform_request(nil, nil, false, 'inject', nil, nil, nil, '\x80')
>> while not c:is_connected() do fiber.sleep(0.01) end
>> finalize_long()
>> future:wait_result(100)
>> @@ -1473,7 +1473,7 @@ c:ping()
>> -- new attempts to read any data - the connection is closed
>> -- already.
>> --
>> -f = fiber.create(c._transport.perform_request, nil, nil, false, 'call_17', nil, nil, 'long', {}) c._transport.perform_request(nil, nil, false, 'inject', nil, nil, '\x80')
>> +f = fiber.create(c._transport.perform_request, nil, nil, false, 'call_17', nil, nil, nil, 'long', {}) c._transport.perform_request(nil, nil, false, 'inject', nil, nil, nil, '\x80')
>> while f:status() ~= 'dead' do fiber.sleep(0.01) end
>> c:close()
>>
>> @@ -1483,7 +1483,7 @@ c:close()
>> --
>> c = net:connect(box.cfg.listen)
>> data = msgpack.encode(18400000000000000000)..'aaaaaaa'
>> -c._transport.perform_request(nil, nil, false, 'inject', nil, nil, data)
>> +c._transport.perform_request(nil, nil, false, 'inject', nil, nil, nil, data)
>> c:close()
>> test_run:grep_log('default', 'too big packet size in the header') ~= nil
>>
>>
next prev parent reply other threads:[~2019-06-25 17:57 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-06-22 11:13 imeevma
2019-06-22 14:21 ` [tarantool-patches] " Vladislav Shpilevoy
2019-06-25 15:10 ` Mergen Imeev
2019-06-25 16:12 ` Mergen Imeev
2019-06-25 17:57 ` Vladislav Shpilevoy [this message]
2019-06-27 12:31 ` Vladimir Davydov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=e8ef1c97-98eb-b02f-1929-c5eeacc8f9bf@tarantool.org \
--to=v.shpilevoy@tarantool.org \
--cc=imeevma@tarantool.org \
--cc=kyukhin@tarantool.org \
--cc=tarantool-patches@freelists.org \
--cc=vdavydov.dev@gmail.com \
--subject='Re: [tarantool-patches] Re: [PATCH v1 1/1] netbox: follow-ups' \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox