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 2D3076E454; Fri, 11 Feb 2022 01:34:05 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 2D3076E454 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1644532445; bh=hkNEViSJaZ7r51uzNP8Bru4PW1UYrEHpxfy8B9umcC8=; 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=S6HtjNXOmt0eyKAmbS9lf736hSTaSvHfeZERuBtlgSFmuOG8Ap0LxP/johGArQjY2 VVXYpcRKzrexonxRpAiF6H4n83YMHxs119j6UQgnHio3Zt56Vng8gTQ4X9gxG5fyRG s/DZHkzemHlG3ePWTDXZ0hAU6oGbXRngmj4cG4mE= Received: from smtpng1.i.mail.ru (smtpng1.i.mail.ru [94.100.181.251]) (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 2A4676E454 for ; Fri, 11 Feb 2022 01:33:55 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 2A4676E454 Received: by smtpng1.m.smailru.net with esmtpa (envelope-from ) id 1nII0g-0001xE-FN; Fri, 11 Feb 2022 01:33:54 +0300 Message-ID: <785348ec-2dc6-8291-f453-6ccb315a4eb9@tarantool.org> Date: Thu, 10 Feb 2022 23:33:53 +0100 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 Content-Language: en-US To: Oleg Babin , tarantool-patches@dev.tarantool.org References: <91cc3506-003f-cfbe-c02f-f49f9505cf87@tarantool.org> In-Reply-To: <91cc3506-003f-cfbe-c02f-f49f9505cf87@tarantool.org> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-4EC0790: 10 X-7564579A: 646B95376F6C166E X-77F55803: 4F1203BC0FB41BD9B74B50284A7C1A0BFDE482DFFE866438B6B3FD87C0A8FE49182A05F5380850408536EA8344EEF7DECEB6A05F2761F7ACDD2DE1CC88EB874C3B57F2C7670A2991 X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE75263010198C72082EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F79006375CC217B55A7C05578638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D861E09D571595AC8C4C3F77E9093466D9117882F4460429724CE54428C33FAD305F5C1EE8F4F765FCAA867293B0326636D2E47CDBA5A96583BD4B6F7A4D31EC0BC014FD901B82EE079FA2833FD35BB23D27C277FBC8AE2E8BAA867293B0326636D2E47CDBA5A96583BA9C0B312567BB231DD303D21008E29813377AFFFEAFD269A417C69337E82CC2E827F84554CEF50127C277FBC8AE2E8BA83251EDC214901ED5E8D9A59859A8B6B1CFA6D474D4A6A4089D37D7C0E48F6C5571747095F342E88FB05168BE4CE3AF X-8FC586DF: 6EFBBC1D9D64D975 X-C1DE0DAB: 0D63561A33F958A51232CC0C562365B1DC9E8CB4E93824E7ED56023BAAD6187BD59269BC5F550898D99A6476B3ADF6B47008B74DF8BB9EF7333BD3B22AA88B938A852937E12ACA7517A45118377F5F9E8E8E86DC7131B365E7726E8460B7C23C X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D34A503FBFE8BE8FC4931BB2F3F36E2C3228FBD75E1EB90490CB25F52499AF9EEAAB3532D9FD8D5AA1B1D7E09C32AA3244CA7B5680B4A28179FA5503D629840823F69B6CAE0477E908D729B2BEF169E0186 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojtO9NuZTWcnMJi/5oFhQUPA== X-Mailru-Sender: 689FA8AB762F739339CABD9B3CA9A7D668DAAC2D0DB8105ACE2D1C343EED52083841015FED1DE5223CC9A89AB576DD93FB559BB5D741EB963CF37A108A312F5C27E8A8C3839CE0E25FEEDEB644C299C0ED14614B50AE0675 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: Vladislav Shpilevoy via Tarantool-patches Reply-To: Vladislav Shpilevoy Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" 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, } -- 2.24.3 (Apple Git-128)