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