From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from [87.239.111.99] (localhost [127.0.0.1]) by dev.tarantool.org (Postfix) with ESMTP id 32F3B6E21E; Fri, 11 Feb 2022 19:39:44 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 32F3B6E21E DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1644597584; bh=OdG00v5X9fE0E2F8SZkRHRQ5oJPYhY2DUDrNaYScEAo=; h=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=jqw6ZMDKxtsE7wgsaV0rSr1lvfWzbo8fx77cXKplKAd+St+1Qz3YeMFqVavX45vGi Msc8JlnxLtD/Jj6UgQtsYE/sAE0myRBSHyL08wKoyoa8FYQ3JbWNY+XqG7IikcxLCO qa2GlA2V+pBmO6i3OaRbvkutyzsVnb1CrBDkkQ4g= Received: from smtp63.i.mail.ru (smtp63.i.mail.ru [217.69.128.43]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id D839D6E224 for ; Fri, 11 Feb 2022 19:38:52 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org D839D6E224 Received: by smtp63.i.mail.ru with esmtpa (envelope-from ) id 1nIYwe-0001yh-77; Fri, 11 Feb 2022 19:38:52 +0300 Message-ID: Date: Fri, 11 Feb 2022 19:38:51 +0300 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0) Gecko/20100101 Thunderbird/91.5.1 To: Vladislav Shpilevoy , tarantool-patches@dev.tarantool.org References: <91cc3506-003f-cfbe-c02f-f49f9505cf87@tarantool.org> <785348ec-2dc6-8291-f453-6ccb315a4eb9@tarantool.org> In-Reply-To: <785348ec-2dc6-8291-f453-6ccb315a4eb9@tarantool.org> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-4EC0790: 10 X-7564579A: EEAE043A70213CC8 X-77F55803: 4F1203BC0FB41BD94A599DE53EADEE5775F76360B65F5D0755C036E01EF35A04182A05F538085040223236A5AFA0205D40A60A8130B5D875CF02E29C01EFF534A1DB401815932F2A X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE749E89BD568380EECC2099A533E45F2D0395957E7521B51C2CFCAF695D4D8E9FCEA1F7E6F0F101C6778DA827A17800CE7E628FE8A185FCFBEEA1F7E6F0F101C6723150C8DA25C47586E58E00D9D99D84E1BDDB23E98D2D38BEBC5CAB6D411FFA6AB5AA0244D397B14A11877603F79680DCC7F00164DA146DAFE8445B8C89999728AA50765F7900637F6B57BC7E64490618DEB871D839B7333395957E7521B51C2DFABB839C843B9C08941B15DA834481F8AA50765F7900637F6B57BC7E6449061A352F6E88A58FB86F5D81C698A659EA73AA81AA40904B5D9A18204E546F3947C68E4D7E803FA7AD59735652A29929C6C4AD6D5ED66289B52698AB9A7B718F8C46E0066C2D8992A16725E5C173C3A84C318264FC03A346C19BA3038C0950A5D36B5C8C57E37DE458B0BC6067A898B09E46D1867E19FE14079C09775C1D3CA48CF3D321E7403792E342EB15956EA79C166A417C69337E82CC275ECD9A6C639B01B78DA827A17800CE798228CBAD4AC77F6731C566533BA786AA5CC5B56E945C8DA X-8FC586DF: 6EFBBC1D9D64D975 X-C1DE0DAB: 0D63561A33F958A5330F363F957902B59F11BF8FD825B16F993EDF07E19C09B5D59269BC5F550898D99A6476B3ADF6B47008B74DF8BB9EF7333BD3B22AA88B938A852937E12ACA7506FE1F977233B9BB410CA545F18667F91A7EA1CDA0B5A7A0 X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D340E23B77EA1807817C81A8BBFAEAEE041BACD7932ADED01B77D0BBA30D6B55AC915AD78F7B3D2A23D1D7E09C32AA3244C6EE7FEE9C000BB5C0DFA4F4D8C6E2BCDF94338140B71B8EE729B2BEF169E0186 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojtO9NuZTWcnPdSpxHAJOE4g== X-Mailru-Sender: 583F1D7ACE8F49BD508BD8DBBD09B14DE953011063C6E35E99C2A395D190D01F57482077F63C535380EE221D05932256AD9BA6614E257C8ED9E51C16F2486AFBE342CF4F05FB7E8CB0DAF586E7D11B3E67EA787935ED9F1B X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH vshard 3/4] router: support msgpack object args X-BeenThere: tarantool-patches@dev.tarantool.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Oleg Babin via Tarantool-patches Reply-To: Oleg Babin Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" Thanks for your changes. LGTM. On 11.02.2022 01:33, Vladislav Shpilevoy wrote: > Thanks for the review! > >>> diff --git a/test/router-luatest/router_test.lua b/test/router-luatest/router_test.lua >>> index 621794a..13ab74d 100644 >>> --- a/test/router-luatest/router_test.lua >>> +++ b/test/router-luatest/router_test.lua >>> @@ -52,3 +53,49 @@ g.test_basic = function(g) >>>       t.assert(not err, 'no error') >>>       t.assert_equals(res, 1, 'good result') >>>   end >>> + >>> +if vutil.feature.msgpack_object then >> You could use g.skip_if(vutil.feature.msgpack_object) instead of conditional test declaration. > Looks better indeed: > > ==================== > @@ -54,13 +54,12 @@ g.test_basic = function(g) > t.assert_equals(res, 1, 'good result') > end > > -if vutil.feature.msgpack_object then > - > g.test_msgpack_args = function(g) > - local router = g.router > + t.run_only_if(vutil.feature.msgpack_object) > -- > -- Normal call ro. > -- > + local router = g.router > local res, err = router:exec(function(timeout) > local args = msgpack.object({100}) > return vshard.router.callrw(1, 'echo', args, {timeout = timeout}) > @@ -97,5 +96,3 @@ g.test_msgpack_args = function(g) > t.assert(err == nil, 'no error') > t.assert_equals(res, 100, 'good result') > end > - > -end -- vutil.feature.msgpack_object > ==================== > >>> diff --git a/vshard/util.lua b/vshard/util.lua >>> index 9e0212a..72176f7 100644 >>> --- a/vshard/util.lua >>> +++ b/vshard/util.lua >>> @@ -230,6 +230,14 @@ local function fiber_is_self_canceled() >>>       return not pcall(fiber.testcancel) >>>   end >>>   +-- >>> +-- Dictionary of supported core features on the given instance. Try to use it >>> +-- in all the other code rather than direct version check. >>> +-- >>> +local feature = { >>> +    msgpack_object = version_is_at_least(2, 10, 0, 'beta', 2, 86), >>> +} >> It could be simplified to `require('msgpack').object ~= nil` > Thanks, looks good: > > ==================== > @@ -3,6 +3,7 @@ local log = require('log') > local fiber = require('fiber') > local lerror = require('vshard.error') > local lversion = require('vshard.version') > +local lmsgpack = require('msgpack') > > local MODULE_INTERNALS = '__module_vshard_util' > local M = rawget(_G, MODULE_INTERNALS) > @@ -235,7 +236,7 @@ end > -- in all the other code rather than direct version check. > -- > local feature = { > - msgpack_object = version_is_at_least(2, 10, 0, 'beta', 2, 86), > + msgpack_object = lmsgpack.object ~= nil, > > ==================== > > > New patch: > > ==================== > router: support msgpack object args > > Msgpack object allows to represent scalar values and tables with > them as a raw MessagePack buffer. It will be introduced after > 2.10.0-beta2. > > It can be used in most of the places which used to take arguments > for further encoding into MessagePack anyway. Including netbox > API. Usage of msgpack object allows not to decode the buffer if it > was received from a remote instance for further proxying. It is > simply memcpy-ed into a next call. > > VShard.router almost supported this feature. Only needed to change > the arguments type check. But even better - simply drop it and > trust netbox to do the type check. > > Part of #312 > --- > test/instances/router.lua | 1 + > test/router-luatest/router_test.lua | 44 +++++++++++++++++++++++++++++ > vshard/replicaset.lua | 4 --- > vshard/util.lua | 10 +++++++ > 4 files changed, 55 insertions(+), 4 deletions(-) > > diff --git a/test/instances/router.lua b/test/instances/router.lua > index 587a473..288a052 100755 > --- a/test/instances/router.lua > +++ b/test/instances/router.lua > @@ -1,5 +1,6 @@ > #!/usr/bin/env tarantool > local helpers = require('test.luatest_helpers') > +_G.msgpack = require('msgpack') > -- Do not load entire vshard into the global namespace to catch errors when code > -- relies on that. > _G.vshard = { > diff --git a/test/router-luatest/router_test.lua b/test/router-luatest/router_test.lua > index 621794a..b7aeea9 100644 > --- a/test/router-luatest/router_test.lua > +++ b/test/router-luatest/router_test.lua > @@ -1,5 +1,6 @@ > local t = require('luatest') > local vtest = require('test.luatest_helpers.vtest') > +local vutil = require('vshard.util') > local wait_timeout = 120 > > local g = t.group('router') > @@ -52,3 +53,46 @@ g.test_basic = function(g) > t.assert(not err, 'no error') > t.assert_equals(res, 1, 'good result') > end > + > +g.test_msgpack_args = function(g) > + t.run_only_if(vutil.feature.msgpack_object) > + -- > + -- Normal call ro. > + -- > + local router = g.router > + local res, err = router:exec(function(timeout) > + local args = msgpack.object({100}) > + return vshard.router.callrw(1, 'echo', args, {timeout = timeout}) > + end, {wait_timeout}) > + t.assert(not err, 'no error') > + t.assert_equals(res, 100, 'good result') > + -- > + -- Normal call rw. > + -- > + res, err = router:exec(function(timeout) > + local args = msgpack.object({100}) > + return vshard.router.callro(1, 'echo', args, {timeout = timeout}) > + end, {wait_timeout}) > + t.assert(not err, 'no error') > + t.assert_equals(res, 100, 'good result') > + -- > + -- Direct call ro. > + -- > + res, err = router:exec(function(timeout) > + local args = msgpack.object({100}) > + local route = vshard.router.route(1) > + return route:callro('echo', args, {timeout = timeout}) > + end, {wait_timeout}) > + t.assert(err == nil, 'no error') > + t.assert_equals(res, 100, 'good result') > + -- > + -- Direct call rw. > + -- > + res, err = router:exec(function(timeout) > + local args = msgpack.object({100}) > + local route = vshard.router.route(1) > + return route:callrw('echo', args, {timeout = timeout}) > + end, {wait_timeout}) > + t.assert(err == nil, 'no error') > + t.assert_equals(res, 100, 'good result') > +end > diff --git a/vshard/replicaset.lua b/vshard/replicaset.lua > index 8e5526a..16b89aa 100644 > --- a/vshard/replicaset.lua > +++ b/vshard/replicaset.lua > @@ -408,8 +408,6 @@ end > -- > local function replicaset_master_call(replicaset, func, args, opts) > assert(opts == nil or type(opts) == 'table') > - assert(type(func) == 'string', 'function name') > - assert(args == nil or type(args) == 'table', 'function arguments') > local master = replicaset.master > if not master then > opts = opts and table.copy(opts) or {} > @@ -552,8 +550,6 @@ local function replicaset_template_multicallro(prefer_replica, balance) > > return function(replicaset, func, args, opts) > assert(opts == nil or type(opts) == 'table') > - assert(type(func) == 'string', 'function name') > - assert(args == nil or type(args) == 'table', 'function arguments') > opts = opts and table.copy(opts) or {} > local timeout = opts.timeout or consts.CALL_TIMEOUT_MAX > local net_status, storage_status, retval, err, replica > diff --git a/vshard/util.lua b/vshard/util.lua > index 9e0212a..354727d 100644 > --- a/vshard/util.lua > +++ b/vshard/util.lua > @@ -3,6 +3,7 @@ local log = require('log') > local fiber = require('fiber') > local lerror = require('vshard.error') > local lversion = require('vshard.version') > +local lmsgpack = require('msgpack') > > local MODULE_INTERNALS = '__module_vshard_util' > local M = rawget(_G, MODULE_INTERNALS) > @@ -230,6 +231,14 @@ local function fiber_is_self_canceled() > return not pcall(fiber.testcancel) > end > > +-- > +-- Dictionary of supported core features on the given instance. Try to use it > +-- in all the other code rather than direct version check. > +-- > +local feature = { > + msgpack_object = lmsgpack.object ~= nil, > +} > + > return { > tuple_extract_key = tuple_extract_key, > reloadable_fiber_create = reloadable_fiber_create, > @@ -241,4 +250,5 @@ return { > table_minus_yield = table_minus_yield, > fiber_cond_wait = fiber_cond_wait, > fiber_is_self_canceled = fiber_is_self_canceled, > + feature = feature, > }