Tarantool development patches archive
 help / color / mirror / Atom feed
* [PATCH v1 1/1] netbox: follow-ups
@ 2019-06-22 11:13 imeevma
  2019-06-22 14:21 ` [tarantool-patches] " Vladislav Shpilevoy
  0 siblings, 1 reply; 6+ messages in thread
From: imeevma @ 2019-06-22 11:13 UTC (permalink / raw)
  To: v.shpilevoy; +Cc: vdavydov.dev, tarantool-patches

Hi! Thank you for review! I combined all three review-fix patches
into one follow-up patch. Also, I included a diff between the
current lua/net_box.lua and the one that was before the original
original patches of the issue.

My answers, diff for net_box.lua and new patch below.

https://github.com/tarantool/tarantool/issues/2978
https://github.com/tarantool/tarantool/tree/imeevma/gh-2978-follow-ups


On 6/21/19 11:39 PM, Vladislav Shpilevoy wrote:
> Hi! Thanks for the patchset!
>
> First of all I would like to say, that I impressed by its
> simplicity, good work. I have only some minor comments except
> one concerning internal netbox API and perf.
>
> See 3 comments below.
>
> 1. It would be great to see here a comment on why does all
> these methods need request args saved.
>
> Secondly, why do you need the args for call/eval/ping/execute?
> These function does not return tuples from a concrete space.
>
> For DML/DQL you need format ptr only, so why do you store the whole
> set of parameters? You could store just one scalar value. See below
> how.
>
> 2. You could keep these functions as is, couldn't you? The problem with current
> version is that before your patch method_encoder.call would invoke C function
> directly. After your patch each 'call' firstly lookups 'internal' table. This is
> why me and Nick were trying to avoid calling any 'internal' functions via
> accessing 'internal' table.
>
> 3. Problem with wrapping each time the parameters into a table is
> that table wrap is usually orders of magnitude slower than variable
> args pass via stack. It was benched recently. If the patch had not
> been pushed, my suggestion would be to make request() method as
>
>     perform_async_request(buffer, skip_header, method, on_push, on_push_ctx,
>                           *request_ctx*, ...)
>
> This function internally would do
>
>     request.ctx = request_ctx
>
> That 'ctx' would be ignored by all decoders except DML/DQL. The latter
> will store format pointer here. For other requests it would be nil, and
> costs nothing. 'nil' on Lua stack is stored as a special value and does
> not occupy memory except stack cell.
>
> Motivation of my proposal is that netbox *is used* in highload, despite
> we sometimes forget about it. In vshard netbox is used extremely hard. Each
> new request table member costs perf and memory. For example, if we have
> 300k RPS from netbox connections on one instance (it is a real number), and
> args table pack costs us +1us (in fact even more), then it is not hard to
> calculate, that each second we will spend nearly 3ms on packing args.
> Taking into account the fact, that network delays are 'ms' too, it looks
> quite expensive.
>
> https://github.com/tarantool/vshard/issues/186#issuecomment-500380508
>

Thank you for proposal. I mostly reverted original patch and
applied this solution.


On 6/21/19 11:39 PM, Vladislav Shpilevoy wrote:
> Thanks for the patch!
>
>> +
>> +static int
>> +lbox_tuple_format_new(struct lua_State *L)
>> +{
>> +    assert(CTID_STRUCT_TUPLE_FORMAT_PTR != 0);
>> +    if (lua_gettop(L) == 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)");
>> +    uint32_t count = lua_objlen(L, 1);
>> +    if (count == 0)
>> +        return lbox_push_tuple_format(L, tuple_format_runtime);
>> +    size_t size = count * sizeof(struct field_def);
>> +    struct region *region = &fiber()->gc;
>> +    size_t region_svp = region_used(region);
>> +    struct field_def *fields =
>> +        (struct field_def *)region_alloc(region, size);
>> +    if (fields == NULL) {
>> +        diag_set(OutOfMemory, size, "region_alloc", "fields");
>> +        return luaT_error(L);
>> +    }
>> +    for (uint32_t i = 0; i < count; ++i) {
>> +        size_t len;
>> +
>> +        fields[i] = field_def_default;
>> +
>> +        lua_pushinteger(L, i + 1);
>> +        lua_gettable(L, 1);
>> +
>> +        lua_pushstring(L, "type");
>> +        lua_gettable(L, -2);
>> +        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));
>
> Here and in all other 'return on error' the region leaks. But there is
> another point. How can there be errors, if the method is internal? If I
> had implemented that patch, I would panic/assert on any error here, except
> region OOM.

Fixed. Also, I removed tests that checked these errors.


On 6/21/19 11:39 PM, Vladislav Shpilevoy wrote:
> 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.
Also, using _request() method it is possible to not receive format
or receive something other than format. Examples of this can be
found in box/net.box.test.lua.


Diff for lua/net_box.lua:

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
+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,
@@ -110,7 +116,7 @@ local method_encoder = {
 
 local method_decoder = {
     ping    = decode_nil,
-    call_16 = internal.decode_select,
+    call_16 = decode_call_16,
     call_17 = decode_data,
     eval    = decode_data,
     insert  = decode_tuple,
@@ -118,7 +124,7 @@ local method_decoder = {
     delete  = decode_tuple,
     update  = decode_tuple,
     upsert  = decode_nil,
-    select  = internal.decode_select,
+    select  = decode_select,
     execute = internal.decode_execute,
     get     = decode_get,
     min     = decode_get,
@@ -486,7 +492,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, ...)
+                                         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})
@@ -499,10 +505,10 @@ local function create_transport(host, port, user, password, callback,
         local id = next_request_id
         method_encoder[method](send_buf, id, ...)
         next_request_id = next_id(id)
-        -- Request in most cases has maximum 9 members:
+        -- Request in most cases has maximum 10 members:
         -- method, buffer, skip_header, id, cond, errno, response,
-        -- on_push, on_push_ctx.
-        local request = setmetatable(table_new(0, 9), request_mt)
+        -- on_push, on_push_ctx and ctx.
+        local request = setmetatable(table_new(0, 10), request_mt)
         request.method = method
         request.buffer = buffer
         request.skip_header = skip_header
@@ -511,6 +517,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.ctx = request_ctx
         return request
     end
 
@@ -520,10 +527,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, ...)
+                                   on_push, on_push_ctx, request_ctx, ...)
         local request, err =
             perform_async_request(buffer, skip_header, method, on_push,
-                                  on_push_ctx, ...)
+                                  on_push_ctx, request_ctx, ...)
         if not request then
             return nil, err
         end
@@ -582,7 +589,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)
+                method_decoder[request.method](body_rpos, body_end, request.ctx)
             assert(real_end == body_end, "invalid body length")
             requests[id] = nil
             request.id = nil
@@ -1064,7 +1071,7 @@ function remote_methods:wait_connected(timeout)
     return self._transport.wait_state('active', timeout)
 end
 
-function remote_methods:_request(method, opts, ...)
+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
@@ -1078,7 +1085,8 @@ function remote_methods:_request(method, opts, ...)
             end
             local res, err =
                 transport.perform_async_request(buffer, skip_header, method,
-                                                table.insert, {}, ...)
+                                                table.insert, {}, request_ctx,
+                                                ...)
             if err then
                 box.error(err)
             end
@@ -1106,7 +1114,7 @@ function remote_methods:_request(method, opts, ...)
     end
     local res, err = transport.perform_request(timeout, buffer, skip_header,
                                                method, on_push, on_push_ctx,
-                                               ...)
+                                               request_ctx, ...)
     if err then
         box.error(err)
     end
@@ -1133,14 +1141,14 @@ end
 -- @deprecated since 1.7.4
 function remote_methods:call_16(func_name, ...)
     check_remote_arg(self, 'call')
-    return (self:_request('call_16', nil, tostring(func_name), {...}))
+    return (self:_request('call_16', nil, nil, tostring(func_name), {...}))
 end
 
 function remote_methods:call(func_name, args, opts)
     check_remote_arg(self, 'call')
     check_call_args(args)
     args = args or {}
-    local res = self:_request('call_17', opts, tostring(func_name), args)
+    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
@@ -1150,14 +1158,14 @@ end
 -- @deprecated since 1.7.4
 function remote_methods:eval_16(code, ...)
     check_remote_arg(self, 'eval')
-    return unpack((self:_request('eval', nil, code, {...})))
+    return unpack((self:_request('eval', nil, nil, code, {...})))
 end
 
 function remote_methods:eval(code, args, opts)
     check_remote_arg(self, 'eval')
     check_eval_args(args)
     args = args or {}
-    local res = self:_request('eval', opts, code, args)
+    local res = self:_request('eval', opts, nil, code, args)
     if type(res) ~= 'table' or opts and opts.is_async then
         return res
     end
@@ -1169,7 +1177,7 @@ function remote_methods:execute(query, parameters, sql_opts, netbox_opts)
     if sql_opts ~= nil then
         box.error(box.error.UNSUPPORTED, "execute", "options")
     end
-    return self:_request('execute', netbox_opts, query, parameters or {},
+    return self:_request('execute', netbox_opts, nil, query, parameters or {},
                          sql_opts or {})
 end
 
@@ -1220,6 +1228,7 @@ function remote_methods:_install_schema(schema_version, spaces, indices,
         s.index = {}
         s.temporary = false
         s._format = format
+        s._format_cdata = box.internal.new_tuple_format(format)
         s.connection = self
         if #space > 5 then
             local opts = space[6]
@@ -1314,10 +1323,12 @@ function console_methods:eval(line, timeout)
     end
     if self.protocol == 'Binary' then
         local loader = 'return require("console").eval(...)'
-        res, err = pr(timeout, nil, false, 'eval', nil, nil, loader, {line})
+        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)
@@ -1336,12 +1347,12 @@ space_metatable = function(remote)
 
     function methods:insert(tuple, opts)
         check_space_arg(self, 'insert')
-        return remote:_request('insert', opts, self.id, tuple)
+        return remote:_request('insert', opts, self._format_cdata, self.id, tuple)
     end
 
     function methods:replace(tuple, opts)
         check_space_arg(self, 'replace')
-        return remote:_request('replace', opts, self.id, tuple)
+        return remote:_request('replace', opts, self._format_cdata, self.id, tuple)
     end
 
     function methods:select(key, opts)
@@ -1361,7 +1372,7 @@ space_metatable = function(remote)
 
     function methods:upsert(key, oplist, opts)
         check_space_arg(self, 'upsert')
-        return nothing_or_data(remote:_request('upsert', opts, self.id, key,
+        return nothing_or_data(remote:_request('upsert', opts, nil, self.id, key,
                                                oplist))
     end
 
@@ -1391,8 +1402,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
-        return (remote:_request('select', opts, self.space.id, self.id,
-                                iterator, offset, limit, key))
+        return (remote:_request('select', opts, self.space._format_cdata,
+                                self.space.id, self.id, iterator, offset,
+                                limit, key))
     end
 
     function methods:get(key, opts)
@@ -1400,8 +1412,10 @@ index_metatable = function(remote)
         if opts and opts.buffer then
             error("index:get() doesn't support `buffer` argument")
         end
-        return nothing_or_data(remote:_request('get', opts, self.space.id,
-                               self.id, box.index.EQ, 0, 2, key))
+        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)
@@ -1409,9 +1423,10 @@ index_metatable = function(remote)
         if opts and opts.buffer then
             error("index:min() doesn't support `buffer` argument")
         end
-        return nothing_or_data(remote:_request('min', opts, self.space.id,
-                                               self.id, box.index.GE, 0, 1,
-                                               key))
+        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)
@@ -1419,9 +1434,10 @@ index_metatable = function(remote)
         if opts and opts.buffer then
             error("index:max() doesn't support `buffer` argument")
         end
-        return nothing_or_data(remote:_request('max', opts, self.space.id,
-                                               self.id, box.index.LE, 0, 1,
-                                               key))
+        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)
@@ -1431,19 +1447,22 @@ index_metatable = function(remote)
         end
         local code = string.format('box.space.%s.index.%s:count',
                                    self.space.name, self.name)
-        return remote:_request('count', opts, code, { key, opts })
+        return remote:_request('count', opts, nil, code, { key, opts })
     end
 
     function methods:delete(key, opts)
         check_index_arg(self, 'delete')
-        return nothing_or_data(remote:_request('delete', opts, self.space.id,
-                                               self.id, key))
+        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')
-        return nothing_or_data(remote:_request('update', opts, self.space.id,
-                                               self.id, key, oplist))
+        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 }



New patch:

From 167c51753d973cc504595b539582794de96a66d6 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 a835d3d..3340070 100644
--- a/src/box/lua/misc.cc
+++ b/src/box/lua/misc.cc
@@ -160,10 +160,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);
@@ -189,20 +189,13 @@ 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) {
diff --git a/src/box/lua/net_box.lua b/src/box/lua/net_box.lua
index 8f31712..f7e1688 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, 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, format)
     if body[2] then
         return nil, raw_end, box.error.MORE_THAN_ONE_TUPLE
     end
@@ -85,67 +86,26 @@ 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)
+local function decode_select(raw_data, raw_data_end, format)
+    return internal.decode_select(raw_data, 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)
@@ -532,7 +492,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 +503,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 +517,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 +527,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 +589,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 +746,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 +1071,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 +1085,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 +1114,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 +1141,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 +1158,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 +1177,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 +1323,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 +1347,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 +1372,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 +1402,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 +1412,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 +1423,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 +1434,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 +1447,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 5c42e33..d21b86a 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
 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [tarantool-patches] [PATCH v1 1/1] netbox: follow-ups
  2019-06-22 11:13 [PATCH v1 1/1] netbox: follow-ups imeevma
@ 2019-06-22 14:21 ` Vladislav Shpilevoy
  2019-06-25 15:10   ` Mergen Imeev
  0 siblings, 1 reply; 6+ messages in thread
From: Vladislav Shpilevoy @ 2019-06-22 14:21 UTC (permalink / raw)
  To: imeevma; +Cc: vdavydov.dev, tarantool-patches

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,

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [tarantool-patches] [PATCH v1 1/1] netbox: follow-ups
  2019-06-22 14:21 ` [tarantool-patches] " Vladislav Shpilevoy
@ 2019-06-25 15:10   ` Mergen Imeev
  2019-06-25 16:12     ` Mergen Imeev
  0 siblings, 1 reply; 6+ messages in thread
From: Mergen Imeev @ 2019-06-25 15:10 UTC (permalink / raw)
  To: Vladislav Shpilevoy; +Cc: vdavydov.dev, tarantool-patches

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
 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [tarantool-patches] [PATCH v1 1/1] netbox: follow-ups
  2019-06-25 15:10   ` Mergen Imeev
@ 2019-06-25 16:12     ` Mergen Imeev
  2019-06-25 17:57       ` [tarantool-patches] " Vladislav Shpilevoy
  2019-06-27 12:31       ` [tarantool-patches] " Vladimir Davydov
  0 siblings, 2 replies; 6+ messages in thread
From: Mergen Imeev @ 2019-06-25 16:12 UTC (permalink / raw)
  To: Vladislav Shpilevoy; +Cc: vdavydov.dev, tarantool-patches

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
>  
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [tarantool-patches] Re: [PATCH v1 1/1] netbox: follow-ups
  2019-06-25 16:12     ` Mergen Imeev
@ 2019-06-25 17:57       ` Vladislav Shpilevoy
  2019-06-27 12:31       ` [tarantool-patches] " Vladimir Davydov
  1 sibling, 0 replies; 6+ messages in thread
From: Vladislav Shpilevoy @ 2019-06-25 17:57 UTC (permalink / raw)
  To: Mergen Imeev; +Cc: vdavydov.dev, tarantool-patches, Kirill Yukhin

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
>>  
>>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [tarantool-patches] [PATCH v1 1/1] netbox: follow-ups
  2019-06-25 16:12     ` Mergen Imeev
  2019-06-25 17:57       ` [tarantool-patches] " Vladislav Shpilevoy
@ 2019-06-27 12:31       ` Vladimir Davydov
  1 sibling, 0 replies; 6+ messages in thread
From: Vladimir Davydov @ 2019-06-27 12:31 UTC (permalink / raw)
  To: Mergen Imeev; +Cc: Vladislav Shpilevoy, tarantool-patches

Pushed to master.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2019-06-27 12:31 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-22 11:13 [PATCH v1 1/1] netbox: follow-ups 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       ` [tarantool-patches] " Vladislav Shpilevoy
2019-06-27 12:31       ` [tarantool-patches] " Vladimir Davydov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox