Tarantool development patches archive
 help / color / mirror / Atom feed
* [Tarantool-patches] [PATCH vshard 0/4] Router msgpack object and netbox return_raw
@ 2022-02-09  0:32 Vladislav Shpilevoy via Tarantool-patches
  2022-02-09  0:32 ` [Tarantool-patches] [PATCH vshard 1/4] test: support luatest Vladislav Shpilevoy via Tarantool-patches
                   ` (4 more replies)
  0 siblings, 5 replies; 20+ messages in thread
From: Vladislav Shpilevoy via Tarantool-patches @ 2022-02-09  0:32 UTC (permalink / raw)
  To: tarantool-patches, olegrok

The patchset makes router support 'return_raw' option for all router calls, and
user's function arguments as 'msgpack' object.

It is not benchmarked whether it helps much. But the solution with the current
msgpack object API can't be any better anyway. The feature must be supported
regardless of what benches will show on the next step.

Branch: http://github.com/tarantool/vshard/tree/gerold103/gh-312-raw-call
Issue: https://github.com/tarantool/vshard/issues/312

Vladislav Shpilevoy (4):
  test: support luatest
  util: introduce Tarantool's semver parser
  router: support msgpack object args
  router: support netbox return_raw

 test-run                             |   2 +-
 test/instances/router.lua            |  16 ++
 test/instances/storage.lua           |  26 +++
 test/luatest_helpers.lua             |  72 ++++++++
 test/luatest_helpers/asserts.lua     |  43 +++++
 test/luatest_helpers/cluster.lua     | 132 +++++++++++++
 test/luatest_helpers/server.lua      | 266 +++++++++++++++++++++++++++
 test/luatest_helpers/vtest.lua       | 135 ++++++++++++++
 test/reload_evolution/storage.result |   4 -
 test/router-luatest/router_test.lua  | 187 +++++++++++++++++++
 test/router-luatest/suite.ini        |   5 +
 test/storage/storage.result          |   2 -
 test/unit-luatest/suite.ini          |   5 +
 test/unit-luatest/version_test.lua   | 179 ++++++++++++++++++
 vshard/CMakeLists.txt                |   2 +-
 vshard/replicaset.lua                |   4 -
 vshard/router/init.lua               |  30 ++-
 vshard/storage/init.lua              |  17 +-
 vshard/util.lua                      |  28 +--
 vshard/version.lua                   | 148 +++++++++++++++
 20 files changed, 1275 insertions(+), 28 deletions(-)
 create mode 100755 test/instances/router.lua
 create mode 100755 test/instances/storage.lua
 create mode 100644 test/luatest_helpers.lua
 create mode 100644 test/luatest_helpers/asserts.lua
 create mode 100644 test/luatest_helpers/cluster.lua
 create mode 100644 test/luatest_helpers/server.lua
 create mode 100644 test/luatest_helpers/vtest.lua
 create mode 100644 test/router-luatest/router_test.lua
 create mode 100644 test/router-luatest/suite.ini
 create mode 100644 test/unit-luatest/suite.ini
 create mode 100644 test/unit-luatest/version_test.lua
 create mode 100644 vshard/version.lua

-- 
2.24.3 (Apple Git-128)


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

* [Tarantool-patches] [PATCH vshard 1/4] test: support luatest
  2022-02-09  0:32 [Tarantool-patches] [PATCH vshard 0/4] Router msgpack object and netbox return_raw Vladislav Shpilevoy via Tarantool-patches
@ 2022-02-09  0:32 ` Vladislav Shpilevoy via Tarantool-patches
  2022-02-09 17:53   ` Oleg Babin via Tarantool-patches
  2022-02-09  0:32 ` [Tarantool-patches] [PATCH vshard 2/4] util: introduce Tarantool's semver parser Vladislav Shpilevoy via Tarantool-patches
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 20+ messages in thread
From: Vladislav Shpilevoy via Tarantool-patches @ 2022-02-09  0:32 UTC (permalink / raw)
  To: tarantool-patches, olegrok

Test-run's most recent guideline is to write new tests in luatest
instead of diff console tests when possible. Luatest isn't exactly
in a perfect condition now, but it has 2 important features which
would be very useful in vshard:

- Easy cluster build. All can be done programmatically except a
  few basic things - from config creation to all replicasets
  start, router start, and buckets bootstrap. No need to hardcode
  that into files like storage_1_a.lua, router_1.lua, etc.

- Can opt-out certain tests depending on Tarantool version. For
  instance, soon coming support for netbox's return_raw option
  will need to run tests only for > 2.10.0-beta2. In diff tests it
  is also possible but would be notably complicated to achieve.

Needed for #312
---
All luatest_helpers/ except vtest.lua are blindly copied from Tarantool's main
repo. I didn't check their bugs, code style, etc.

 test-run                            |   2 +-
 test/instances/router.lua           |  15 ++
 test/instances/storage.lua          |  21 +++
 test/luatest_helpers.lua            |  72 ++++++++
 test/luatest_helpers/asserts.lua    |  43 +++++
 test/luatest_helpers/cluster.lua    | 132 ++++++++++++++
 test/luatest_helpers/server.lua     | 266 ++++++++++++++++++++++++++++
 test/luatest_helpers/vtest.lua      | 135 ++++++++++++++
 test/router-luatest/router_test.lua |  54 ++++++
 test/router-luatest/suite.ini       |   5 +
 10 files changed, 744 insertions(+), 1 deletion(-)
 create mode 100755 test/instances/router.lua
 create mode 100755 test/instances/storage.lua
 create mode 100644 test/luatest_helpers.lua
 create mode 100644 test/luatest_helpers/asserts.lua
 create mode 100644 test/luatest_helpers/cluster.lua
 create mode 100644 test/luatest_helpers/server.lua
 create mode 100644 test/luatest_helpers/vtest.lua
 create mode 100644 test/router-luatest/router_test.lua
 create mode 100644 test/router-luatest/suite.ini

diff --git a/test-run b/test-run
index c345003..2604c46 160000
--- a/test-run
+++ b/test-run
@@ -1 +1 @@
-Subproject commit c34500365efe8316e79c7936a2f2d04644602936
+Subproject commit 2604c46c7b6368dbde59489d5303ce3d1d430331
diff --git a/test/instances/router.lua b/test/instances/router.lua
new file mode 100755
index 0000000..ccec6c1
--- /dev/null
+++ b/test/instances/router.lua
@@ -0,0 +1,15 @@
+#!/usr/bin/env tarantool
+local helpers = require('test.luatest_helpers')
+-- Do not load entire vshard into the global namespace to catch errors when code
+-- relies on that.
+_G.vshard = {
+    router = require('vshard.router'),
+}
+-- Somewhy shutdown hangs on new Tarantools even though the nodes do not seem to
+-- have any long requests running.
+box.ctl.set_on_shutdown_timeout(0.001)
+
+box.cfg(helpers.box_cfg())
+box.schema.user.grant('guest', 'super', nil, nil, {if_not_exists = true})
+
+_G.ready = true
diff --git a/test/instances/storage.lua b/test/instances/storage.lua
new file mode 100755
index 0000000..2d679ba
--- /dev/null
+++ b/test/instances/storage.lua
@@ -0,0 +1,21 @@
+#!/usr/bin/env tarantool
+local helpers = require('test.luatest_helpers')
+-- Do not load entire vshard into the global namespace to catch errors when code
+-- relies on that.
+_G.vshard = {
+    storage = require('vshard.storage'),
+}
+-- Somewhy shutdown hangs on new Tarantools even though the nodes do not seem to
+-- have any long requests running.
+box.ctl.set_on_shutdown_timeout(0.001)
+
+box.cfg(helpers.box_cfg())
+box.schema.user.grant('guest', 'super', nil, nil, {if_not_exists = true})
+
+local function echo(...)
+    return ...
+end
+
+_G.echo = echo
+
+_G.ready = true
diff --git a/test/luatest_helpers.lua b/test/luatest_helpers.lua
new file mode 100644
index 0000000..283906c
--- /dev/null
+++ b/test/luatest_helpers.lua
@@ -0,0 +1,72 @@
+local fun = require('fun')
+local json = require('json')
+local fio = require('fio')
+local log = require('log')
+local yaml = require('yaml')
+local fiber = require('fiber')
+
+local luatest_helpers = {
+    SOCKET_DIR = fio.abspath(os.getenv('VARDIR') or 'test/var')
+}
+
+luatest_helpers.Server = require('test.luatest_helpers.server')
+
+local function default_cfg()
+    return {
+        work_dir = os.getenv('TARANTOOL_WORKDIR'),
+        listen = os.getenv('TARANTOOL_LISTEN'),
+        log = ('%s/%s.log'):format(os.getenv('TARANTOOL_WORKDIR'), os.getenv('TARANTOOL_ALIAS')),
+    }
+end
+
+local function env_cfg()
+    local src = os.getenv('TARANTOOL_BOX_CFG')
+    if src == nil then
+        return {}
+    end
+    local res = json.decode(src)
+    assert(type(res) == 'table')
+    return res
+end
+
+-- Collect box.cfg table from values passed through
+-- luatest_helpers.Server({<...>}) and from the given argument.
+--
+-- Use it from inside an instance script.
+function luatest_helpers.box_cfg(cfg)
+    return fun.chain(default_cfg(), env_cfg(), cfg or {}):tomap()
+end
+
+function luatest_helpers.instance_uri(alias, instance_id)
+    if instance_id == nil then
+        instance_id = ''
+    end
+    instance_id = tostring(instance_id)
+    return ('%s/%s%s.iproto'):format(luatest_helpers.SOCKET_DIR, alias, instance_id);
+end
+
+function luatest_helpers:get_vclock(server)
+    return server:eval('return box.info.vclock')
+end
+
+function luatest_helpers:wait_vclock(server, to_vclock)
+    while true do
+        local vclock = self:get_vclock(server)
+        local ok = true
+        for server_id, to_lsn in pairs(to_vclock) do
+            local lsn = vclock[server_id]
+            if lsn == nil or lsn < to_lsn then
+                ok = false
+                break
+            end
+        end
+        if ok then
+            return
+        end
+        log.info("wait vclock: %s to %s", yaml.encode(vclock),
+                 yaml.encode(to_vclock))
+        fiber.sleep(0.001)
+    end
+end
+
+return luatest_helpers
diff --git a/test/luatest_helpers/asserts.lua b/test/luatest_helpers/asserts.lua
new file mode 100644
index 0000000..77385d8
--- /dev/null
+++ b/test/luatest_helpers/asserts.lua
@@ -0,0 +1,43 @@
+local t = require('luatest')
+
+local asserts = {}
+
+function asserts:new(object)
+    self:inherit(object)
+    object:initialize()
+    return object
+end
+
+function asserts:inherit(object)
+    object = object or {}
+    setmetatable(object, self)
+    self.__index = self
+    return object
+end
+
+function asserts:assert_server_follow_upstream(server, id)
+    local status = server:eval(
+        ('return box.info.replication[%d].upstream.status'):format(id))
+    t.assert_equals(status, 'follow',
+        ('%s: this server does not follow others.'):format(server.alias))
+end
+
+
+function asserts:wait_fullmesh(servers, wait_time)
+    wait_time = wait_time or 20
+    t.helpers.retrying({timeout = wait_time}, function()
+        for _, server in pairs(servers) do
+            for _, server2 in pairs(servers) do
+                if server ~= server2 then
+                    local server_id = server:eval('return box.info.id')
+                    local server2_id = server2:eval('return box.info.id')
+                    if server_id ~= server2_id then
+                            self:assert_server_follow_upstream(server, server2_id)
+                    end
+                end
+            end
+        end
+    end)
+end
+
+return asserts
diff --git a/test/luatest_helpers/cluster.lua b/test/luatest_helpers/cluster.lua
new file mode 100644
index 0000000..43e3479
--- /dev/null
+++ b/test/luatest_helpers/cluster.lua
@@ -0,0 +1,132 @@
+local fio = require('fio')
+local Server = require('test.luatest_helpers.server')
+
+local root = os.environ()['SOURCEDIR'] or '.'
+
+local Cluster = {}
+
+function Cluster:new(object)
+    self:inherit(object)
+    object:initialize()
+    self.servers = object.servers
+    self.built_servers = object.built_servers
+    return object
+end
+
+function Cluster:inherit(object)
+    object = object or {}
+    setmetatable(object, self)
+    self.__index = self
+    self.servers = {}
+    self.built_servers = {}
+    return object
+end
+
+function Cluster:initialize()
+    self.servers = {}
+end
+
+function Cluster:server(alias)
+    for _, server in ipairs(self.servers) do
+        if server.alias == alias then
+            return server
+        end
+    end
+    return nil
+end
+
+function Cluster:drop()
+    for _, server in ipairs(self.servers) do
+        if server ~= nil then
+            server:stop()
+            server:cleanup()
+        end
+    end
+end
+
+function Cluster:get_index(server)
+    local index = nil
+    for i, v in ipairs(self.servers) do
+        if (v.id == server) then
+          index = i
+        end
+    end
+    return index
+end
+
+function Cluster:delete_server(server)
+    local idx = self:get_index(server)
+    if idx == nil then
+        print("Key does not exist")
+    else
+        table.remove(self.servers, idx)
+    end
+end
+
+function Cluster:stop()
+    for _, server in ipairs(self.servers) do
+        if server ~= nil then
+            server:stop()
+        end
+    end
+end
+
+function Cluster:start(opts)
+    for _, server in ipairs(self.servers) do
+        if not server.process then
+            server:start({wait_for_readiness = false})
+        end
+    end
+
+    -- The option is true by default.
+    local wait_for_readiness = true
+    if opts ~= nil and opts.wait_for_readiness ~= nil then
+        wait_for_readiness = opts.wait_for_readiness
+    end
+
+    if wait_for_readiness then
+        for _, server in ipairs(self.servers) do
+            server:wait_for_readiness()
+        end
+    end
+end
+
+function Cluster:build_server(server_config, instance_file)
+    instance_file = instance_file or 'default.lua'
+    server_config = table.deepcopy(server_config)
+    server_config.command = fio.pathjoin(root, 'test/instances/', instance_file)
+    assert(server_config.alias, 'Either replicaset.alias or server.alias must be given')
+    local server = Server:new(server_config)
+    table.insert(self.built_servers, server)
+    return server
+end
+
+function Cluster:add_server(server)
+    if self:server(server.alias) ~= nil then
+        error('Alias is not provided')
+    end
+    table.insert(self.servers, server)
+end
+
+function Cluster:build_and_add_server(config, replicaset_config, engine)
+    local server = self:build_server(config, replicaset_config, engine)
+    self:add_server(server)
+    return server
+end
+
+
+function Cluster:get_leader()
+    for _, instance in ipairs(self.servers) do
+        if instance:eval('return box.info.ro') == false then
+            return instance
+        end
+    end
+end
+
+function Cluster:exec_on_leader(bootstrap_function)
+    local leader = self:get_leader()
+    return leader:exec(bootstrap_function)
+end
+
+
+return Cluster
diff --git a/test/luatest_helpers/server.lua b/test/luatest_helpers/server.lua
new file mode 100644
index 0000000..714c537
--- /dev/null
+++ b/test/luatest_helpers/server.lua
@@ -0,0 +1,266 @@
+local clock = require('clock')
+local digest = require('digest')
+local ffi = require('ffi')
+local fiber = require('fiber')
+local fio = require('fio')
+local fun = require('fun')
+local json = require('json')
+local errno = require('errno')
+
+local checks = require('checks')
+local luatest = require('luatest')
+
+ffi.cdef([[
+    int kill(pid_t pid, int sig);
+]])
+
+local Server = luatest.Server:inherit({})
+
+local WAIT_TIMEOUT = 60
+local WAIT_DELAY = 0.1
+
+local DEFAULT_CHECKPOINT_PATTERNS = {"*.snap", "*.xlog", "*.vylog",
+                                     "*.inprogress", "[0-9]*/"}
+
+-- Differences from luatest.Server:
+--
+-- * 'alias' is mandatory.
+-- * 'command' is optional, assumed test/instances/default.lua by
+--   default.
+-- * 'workdir' is optional, determined by 'alias'.
+-- * The new 'box_cfg' parameter.
+-- * engine - provides engine for parameterized tests
+Server.constructor_checks = fun.chain(Server.constructor_checks, {
+    alias = 'string',
+    command = '?string',
+    workdir = '?string',
+    box_cfg = '?table',
+    engine = '?string',
+}):tomap()
+
+function Server:initialize()
+    local vardir = fio.abspath(os.getenv('VARDIR') or 'test/var')
+
+    if self.id == nil then
+        local random = digest.urandom(9)
+        self.id = digest.base64_encode(random, {urlsafe = true})
+    end
+    if self.command == nil then
+        self.command = 'test/instances/default.lua'
+    end
+    if self.workdir == nil then
+        self.workdir = ('%s/%s-%s'):format(vardir, self.alias, self.id)
+        fio.rmtree(self.workdir)
+        fio.mktree(self.workdir)
+    end
+    if self.net_box_port == nil and self.net_box_uri == nil then
+        self.net_box_uri = ('%s/%s.iproto'):format(vardir, self.alias)
+        fio.mktree(vardir)
+    end
+
+    -- AFAIU, the inner getmetatable() returns our helpers.Server
+    -- class, the outer one returns luatest.Server class.
+    getmetatable(getmetatable(self)).initialize(self)
+end
+
+--- Generates environment to run process with.
+-- The result is merged into os.environ().
+-- @return map
+function Server:build_env()
+    local res = getmetatable(getmetatable(self)).build_env(self)
+    if self.box_cfg ~= nil then
+        res.TARANTOOL_BOX_CFG = json.encode(self.box_cfg)
+    end
+    res.TARANTOOL_ENGINE = self.engine
+    return res
+end
+
+local function wait_cond(cond_name, server, func, ...)
+    local alias = server.alias
+    local id = server.id
+    local pid = server.process.pid
+
+    local deadline = clock.time() + WAIT_TIMEOUT
+    while true do
+        if func(...) then
+            return
+        end
+        if clock.time() > deadline then
+            error(('Waiting for "%s" on server %s-%s (PID %d) timed out')
+                  :format(cond_name, alias, id, pid))
+        end
+        fiber.sleep(WAIT_DELAY)
+    end
+end
+
+function Server:wait_for_readiness()
+    return wait_cond('readiness', self, function()
+        local ok, is_ready = pcall(function()
+            self:connect_net_box()
+            return self.net_box:eval('return _G.ready') == true
+        end)
+        return ok and is_ready
+    end)
+end
+
+function Server:wait_election_leader()
+    -- Include read-only property too because if an instance is a leader, it
+    -- does not mean it finished the synchro queue ownership transition. It is
+    -- read-only until that happens. But in tests usually the leader is needed
+    -- as a writable node.
+    return wait_cond('election leader', self, self.exec, self, function()
+        return box.info.election.state == 'leader' and not box.info.ro
+    end)
+end
+
+function Server:wait_election_leader_found()
+    return wait_cond('election leader is found', self, self.exec, self,
+                     function() return box.info.election.leader ~= 0 end)
+end
+
+-- Unlike the original luatest.Server function it waits for
+-- starting the server.
+function Server:start(opts)
+    checks('table', {
+        wait_for_readiness = '?boolean',
+    })
+    getmetatable(getmetatable(self)).start(self)
+
+    -- The option is true by default.
+    local wait_for_readiness = true
+    if opts ~= nil and opts.wait_for_readiness ~= nil then
+        wait_for_readiness = opts.wait_for_readiness
+    end
+
+    if wait_for_readiness then
+        self:wait_for_readiness()
+    end
+end
+
+function Server:instance_id()
+    -- Cache the value when found it first time.
+    if self.instance_id_value then
+        return self.instance_id_value
+    end
+    local id = self:exec(function() return box.info.id end)
+    -- But do not cache 0 - it is an anon instance, its ID might change.
+    if id ~= 0 then
+        self.instance_id_value = id
+    end
+    return id
+end
+
+function Server:instance_uuid()
+    -- Cache the value when found it first time.
+    if self.instance_uuid_value then
+        return self.instance_uuid_value
+    end
+    local uuid = self:exec(function() return box.info.uuid end)
+    self.instance_uuid_value = uuid
+    return uuid
+end
+
+-- TODO: Add the 'wait_for_readiness' parameter for the restart()
+-- method.
+
+-- Unlike the original luatest.Server function it waits until
+-- the server will stop.
+function Server:stop()
+    local alias = self.alias
+    local id = self.id
+    if self.process then
+        local pid = self.process.pid
+        getmetatable(getmetatable(self)).stop(self)
+
+        local deadline = clock.time() + WAIT_TIMEOUT
+        while true do
+            if ffi.C.kill(pid, 0) ~= 0 then
+                break
+            end
+            if clock.time() > deadline then
+                error(('Stopping of server %s-%s (PID %d) was timed out'):format(
+                    alias, id, pid))
+            end
+            fiber.sleep(WAIT_DELAY)
+        end
+    end
+end
+
+function Server:cleanup()
+    for _, pattern in ipairs(DEFAULT_CHECKPOINT_PATTERNS) do
+        fio.rmtree(('%s/%s'):format(self.workdir, pattern))
+    end
+    self.instance_id_value = nil
+    self.instance_uuid_value = nil
+end
+
+function Server:drop()
+    self:stop()
+    self:cleanup()
+end
+
+-- A copy of test_run:grep_log.
+function Server:grep_log(what, bytes, opts)
+    local opts = opts or {}
+    local noreset = opts.noreset or false
+    -- if instance has crashed provide filename to use grep_log
+    local filename = opts.filename or self:eval('return box.cfg.log')
+    local file = fio.open(filename, {'O_RDONLY', 'O_NONBLOCK'})
+
+    local function fail(msg)
+        local err = errno.strerror()
+        if file ~= nil then
+            file:close()
+        end
+        error(string.format("%s: %s: %s", msg, filename, err))
+    end
+
+    if file == nil then
+        fail("Failed to open log file")
+    end
+    io.flush() -- attempt to flush stdout == log fd
+    local filesize = file:seek(0, 'SEEK_END')
+    if filesize == nil then
+        fail("Failed to get log file size")
+    end
+    local bytes = bytes or 65536 -- don't read whole log - it can be huge
+    bytes = bytes > filesize and filesize or bytes
+    if file:seek(-bytes, 'SEEK_END') == nil then
+        fail("Failed to seek log file")
+    end
+    local found, buf
+    repeat -- read file in chunks
+        local s = file:read(2048)
+        if s == nil then
+            fail("Failed to read log file")
+        end
+        local pos = 1
+        repeat -- split read string in lines
+            local endpos = string.find(s, '\n', pos)
+            endpos = endpos and endpos - 1 -- strip terminating \n
+            local line = string.sub(s, pos, endpos)
+            if endpos == nil and s ~= '' then
+                -- line doesn't end with \n or eof, append it to buffer
+                -- to be checked on next iteration
+                buf = buf or {}
+                table.insert(buf, line)
+            else
+                if buf ~= nil then -- prepend line with buffered data
+                    table.insert(buf, line)
+                    line = table.concat(buf)
+                    buf = nil
+                end
+                if string.match(line, "Starting instance") and not noreset then
+                    found = nil -- server was restarted, reset search
+                else
+                    found = string.match(line, what) or found
+                end
+            end
+            pos = endpos and endpos + 2 -- jump to char after \n
+        until pos == nil
+    until s == ''
+    file:close()
+    return found
+end
+
+return Server
diff --git a/test/luatest_helpers/vtest.lua b/test/luatest_helpers/vtest.lua
new file mode 100644
index 0000000..affc008
--- /dev/null
+++ b/test/luatest_helpers/vtest.lua
@@ -0,0 +1,135 @@
+local helpers = require('test.luatest_helpers')
+local cluster = require('test.luatest_helpers.cluster')
+
+local uuid_idx = 1
+
+--
+-- New UUID unique per this process. Generation is not random - for simplicity
+-- and reproducibility.
+--
+local function uuid_next()
+    local last = tostring(uuid_idx)
+    uuid_idx = uuid_idx + 1
+    assert(#last <= 12)
+    return '00000000-0000-0000-0000-'..string.rep('0', 12 - #last)..last
+end
+
+--
+-- Build a valid vshard config by a template. A template does not specify
+-- anything volatile such as URIs, UUIDs - these are installed at runtime.
+--
+local function config_new(templ)
+    local res = table.deepcopy(templ)
+    local sharding = {}
+    res.sharding = sharding
+    for _, replicaset_templ in pairs(templ.sharding) do
+        local replicaset_uuid = uuid_next()
+        local replicas = {}
+        local replicaset = table.deepcopy(replicaset_templ)
+        replicaset.replicas = replicas
+        for replica_name, replica_templ in pairs(replicaset_templ.replicas) do
+            local replica_uuid = uuid_next()
+            local replica = table.deepcopy(replica_templ)
+            replica.name = replica_name
+            replica.uri = 'storage:storage@'..helpers.instance_uri(replica_name)
+            replicas[replica_uuid] = replica
+        end
+        sharding[replicaset_uuid] = replicaset
+    end
+    return res
+end
+
+--
+-- Build new cluster by a given config.
+--
+local function storage_new(g, cfg)
+    if not g.cluster then
+        g.cluster = cluster:new({})
+    end
+    local all_servers = {}
+    local masters = {}
+    local replicas = {}
+    for replicaset_uuid, replicaset in pairs(cfg.sharding) do
+        -- Luatest depends on box.cfg being ready and listening. Need to
+        -- configure it before vshard.storage.cfg().
+        local box_repl = {}
+        for _, replica in pairs(replicaset.replicas) do
+            table.insert(box_repl, replica.uri)
+        end
+        local box_cfg = {
+            replication = box_repl,
+            -- Speed retries up.
+            replication_timeout = 0.1,
+        }
+        for replica_uuid, replica in pairs(replicaset.replicas) do
+            local name = replica.name
+            box_cfg.instance_uuid = replica_uuid
+            box_cfg.replicaset_uuid = replicaset_uuid
+            box_cfg.listen = helpers.instance_uri(replica.name)
+            -- Need to specify read-only explicitly to know how is master.
+            box_cfg.read_only = not replica.master
+            local server = g.cluster:build_server({
+                alias = name,
+                box_cfg = box_cfg,
+            }, 'storage.lua')
+            g[name] = server
+            g.cluster:add_server(server)
+
+            table.insert(all_servers, server)
+            if replica.master then
+                table.insert(masters, server)
+            else
+                table.insert(replicas, server)
+            end
+        end
+    end
+    for _, replica in pairs(all_servers) do
+        replica:start({wait_for_readiness = false})
+    end
+    for _, master in pairs(masters) do
+        master:wait_for_readiness()
+        master:exec(function(cfg)
+            -- Logged in as guest with 'super' access rights. Yet 'super' is not
+            -- enough to grant 'replication' privilege. The simplest way - login
+            -- as admin for that temporary.
+            local user = box.session.user()
+            box.session.su('admin')
+
+            vshard.storage.cfg(cfg, box.info.uuid)
+            box.schema.user.grant('storage', 'super')
+
+            box.session.su(user)
+        end, {cfg})
+    end
+    for _, replica in pairs(replicas) do
+        replica:wait_for_readiness()
+        replica:exec(function(cfg)
+            vshard.storage.cfg(cfg, box.info.uuid)
+        end, {cfg})
+    end
+end
+
+--
+-- Create a new router in the cluster.
+--
+local function router_new(g, name, cfg)
+    if not g.cluster then
+        g.cluster = cluster:new({})
+    end
+    local server = g.cluster:build_server({
+        alias = name,
+    }, 'router.lua')
+    g[name] = server
+    g.cluster:add_server(server)
+    server:start()
+    server:exec(function(cfg)
+        vshard.router.cfg(cfg)
+    end, {cfg})
+    return server
+end
+
+return {
+    config_new = config_new,
+    storage_new = storage_new,
+    router_new = router_new,
+}
diff --git a/test/router-luatest/router_test.lua b/test/router-luatest/router_test.lua
new file mode 100644
index 0000000..621794a
--- /dev/null
+++ b/test/router-luatest/router_test.lua
@@ -0,0 +1,54 @@
+local t = require('luatest')
+local vtest = require('test.luatest_helpers.vtest')
+local wait_timeout = 120
+
+local g = t.group('router')
+local cluster_cfg = vtest.config_new({
+    sharding = {
+        {
+            replicas = {
+                replica_1_a = {
+                    master = true,
+                },
+                replica_1_b = {},
+            },
+        },
+        {
+            replicas = {
+                replica_2_a = {
+                    master = true,
+                },
+                replica_2_b = {},
+            },
+        },
+    },
+    bucket_count = 100
+})
+
+g.before_all(function()
+    vtest.storage_new(g, cluster_cfg)
+
+    t.assert_equals(g.replica_1_a:exec(function()
+        return #vshard.storage.info().alerts
+    end), 0, 'no alerts after boot')
+
+    local router = vtest.router_new(g, 'router', cluster_cfg)
+    g.router = router
+    local res, err = router:exec(function(timeout)
+        return vshard.router.bootstrap({timeout = timeout})
+    end, {wait_timeout})
+    t.assert(res and not err, 'bootstrap buckets')
+end)
+
+g.after_all(function()
+    g.cluster:drop()
+end)
+
+g.test_basic = function(g)
+    local router = g.router
+    local res, err = router:exec(function(timeout)
+        return vshard.router.callrw(1, 'echo', {1}, {timeout = timeout})
+    end, {wait_timeout})
+    t.assert(not err, 'no error')
+    t.assert_equals(res, 1, 'good result')
+end
diff --git a/test/router-luatest/suite.ini b/test/router-luatest/suite.ini
new file mode 100644
index 0000000..ae79147
--- /dev/null
+++ b/test/router-luatest/suite.ini
@@ -0,0 +1,5 @@
+[default]
+core = luatest
+description = Router tests
+is_parallel = True
+release_disabled =
-- 
2.24.3 (Apple Git-128)


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

* [Tarantool-patches] [PATCH vshard 2/4] util: introduce Tarantool's semver parser
  2022-02-09  0:32 [Tarantool-patches] [PATCH vshard 0/4] Router msgpack object and netbox return_raw Vladislav Shpilevoy via Tarantool-patches
  2022-02-09  0:32 ` [Tarantool-patches] [PATCH vshard 1/4] test: support luatest Vladislav Shpilevoy via Tarantool-patches
@ 2022-02-09  0:32 ` Vladislav Shpilevoy via Tarantool-patches
  2022-02-09 17:53   ` Oleg Babin via Tarantool-patches
  2022-02-09  0:32 ` [Tarantool-patches] [PATCH vshard 3/4] router: support msgpack object args Vladislav Shpilevoy via Tarantool-patches
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 20+ messages in thread
From: Vladislav Shpilevoy via Tarantool-patches @ 2022-02-09  0:32 UTC (permalink / raw)
  To: tarantool-patches, olegrok

Tarantool's version since recently has semver-like format. It
made impossible to check for features existence if they were
introduced not in the first release of one x.x.x triplet.

An alternative would be to test for feature existence by its
usage, but it is not always possible with ease.

This patch improves version parser in vshard to understand new
version names.

It will be used by a future commit about netbox's return_raw
feature adoption to skip its tests for old versions
(<= 2.10.0-beta2).

Needed for #312
---
 test/unit-luatest/suite.ini        |   5 +
 test/unit-luatest/version_test.lua | 179 +++++++++++++++++++++++++++++
 vshard/CMakeLists.txt              |   2 +-
 vshard/router/init.lua             |   2 +-
 vshard/storage/init.lua            |   2 +-
 vshard/util.lua                    |  17 +--
 vshard/version.lua                 | 148 ++++++++++++++++++++++++
 7 files changed, 339 insertions(+), 16 deletions(-)
 create mode 100644 test/unit-luatest/suite.ini
 create mode 100644 test/unit-luatest/version_test.lua
 create mode 100644 vshard/version.lua

diff --git a/test/unit-luatest/suite.ini b/test/unit-luatest/suite.ini
new file mode 100644
index 0000000..303032c
--- /dev/null
+++ b/test/unit-luatest/suite.ini
@@ -0,0 +1,5 @@
+[default]
+core = luatest
+description = Unit tests
+is_parallel = True
+release_disabled =
diff --git a/test/unit-luatest/version_test.lua b/test/unit-luatest/version_test.lua
new file mode 100644
index 0000000..905d36e
--- /dev/null
+++ b/test/unit-luatest/version_test.lua
@@ -0,0 +1,179 @@
+local t = require('luatest')
+local lversion = require('vshard.version')
+
+local g = t.group('version')
+
+g.test_order = function(g)
+    -- Example of a full version: 2.10.0-beta2-86-gc9981a567.
+    local versions = {
+        {
+            str = '1.2.3-alpha',
+            ver = lversion.new(1, 2, 3, 'alpha', 0, 0),
+        },
+        {
+            str = '1.2.3-alpha-30',
+            ver = lversion.new(1, 2, 3, 'alpha', 0, 30),
+        },
+        {
+            str = '1.2.3-alpha-45',
+            ver = lversion.new(1, 2, 3, 'alpha', 0, 45),
+        },
+        {
+            str = '1.2.3-alpha1',
+            ver = lversion.new(1, 2, 3, 'alpha', 1, 0),
+        },
+        {
+            str = '1.2.3-alpha1-45',
+            ver = lversion.new(1, 2, 3, 'alpha', 1, 45),
+        },
+        {
+            str = '1.2.3-alpha2',
+            ver = lversion.new(1, 2, 3, 'alpha', 2, 0),
+        },
+        {
+            str = '1.2.3-alpha2-45',
+            ver = lversion.new(1, 2, 3, 'alpha', 2, 45),
+        },
+        {
+            str = '1.2.3-beta',
+            ver = lversion.new(1, 2, 3, 'beta', 0, 0),
+        },
+        {
+            str = '1.2.3-beta-45',
+            ver = lversion.new(1, 2, 3, 'beta', 0, 45),
+        },
+        {
+            str = '1.2.3-beta1',
+            ver = lversion.new(1, 2, 3, 'beta', 1, 0),
+        },
+        {
+            str = '1.2.3-beta1-45',
+            ver = lversion.new(1, 2, 3, 'beta', 1, 45),
+        },
+        {
+            str = '1.2.3-beta2',
+            ver = lversion.new(1, 2, 3, 'beta', 2, 0),
+        },
+        {
+            str = '1.2.3-beta2-45',
+            ver = lversion.new(1, 2, 3, 'beta', 2, 45),
+        },
+        {
+            str = '1.2.3-rc',
+            ver = lversion.new(1, 2, 3, 'rc', 0, 0),
+        },
+        {
+            str = '1.2.3-rc-45',
+            ver = lversion.new(1, 2, 3, 'rc', 0, 45),
+        },
+        {
+            str = '1.2.3-rc1',
+            ver = lversion.new(1, 2, 3, 'rc', 1, 0),
+        },
+        {
+            str = '1.2.3-rc1-45',
+            ver = lversion.new(1, 2, 3, 'rc', 1, 45),
+        },
+        {
+            str = '1.2.3-rc2',
+            ver = lversion.new(1, 2, 3, 'rc', 2, 0),
+        },
+        {
+            str = '1.2.3-rc2-45',
+            ver = lversion.new(1, 2, 3, 'rc', 2, 45),
+        },
+        {
+            str = '1.2.3-rc3',
+            ver = lversion.new(1, 2, 3, 'rc', 3, 0),
+        },
+        {
+            str = '1.2.3-rc4',
+            ver = lversion.new(1, 2, 3, 'rc', 4, 0),
+        },
+        {
+            str = '1.2.3',
+            ver = lversion.new(1, 2, 3, nil, 0, 0),
+        },
+        {
+            str = '1.2.4',
+            ver = lversion.new(1, 2, 4, nil, 0, 0),
+        },
+        {
+            str = '1.2.5-alpha',
+            ver = lversion.new(1, 2, 5, 'alpha', 0, 0),
+        },
+        {
+            str = '1.2.5-alpha1-45-gc9981a567',
+            ver = lversion.new(1, 2, 5, 'alpha', 1, 45),
+        },
+        {
+            str = '1.2.6-',
+            ver = lversion.new(1, 2, 6, nil, 0, 0),
+        },
+        {
+            str = '1.2.7-alpha-',
+            ver = lversion.new(1, 2, 7, 'alpha', 0, 0),
+        },
+        {
+            str = '1.2.7-alpha1-',
+            ver = lversion.new(1, 2, 7, 'alpha', 1, 0),
+        },
+        {
+            str = '1.2.7-alpha1-45',
+            ver = lversion.new(1, 2, 7, 'alpha', 1, 45),
+        },
+        {
+            str = '1.2.7-alpha1-46-',
+            ver = lversion.new(1, 2, 7, 'alpha', 1, 46),
+        },
+        {
+            str = '1.2.8-alpha',
+            ver = lversion.new(1, 2, 8, 'alpha', 0, 0),
+        },
+        {
+            str = '1.2.8-beta',
+            ver = lversion.new(1, 2, 8, 'beta', 0, 0),
+        },
+        {
+            str = '1.2.8-rc',
+            ver = lversion.new(1, 2, 8, 'rc', 0, 0),
+        },
+        {
+            str = '1.2.9',
+            ver = lversion.new(1, 2, 9, nil, 0, 0),
+        },
+    }
+    for i, v in pairs(versions) do
+        local ver = lversion.parse(v.str)
+        t.assert(ver == v.ver, ('versions ==, %d'):format(i))
+        t.assert(not (ver ~= v.ver), ('versions not ~=, %d'):format(i))
+        t.assert(not (ver < v.ver), ('versions not <, %d'):format(i))
+        t.assert(not (ver > v.ver), ('versions not >, %d'):format(i))
+        t.assert(ver <= v.ver, ('versions <=, %d'):format(i))
+        t.assert(ver >= v.ver, ('versions <=, %d'):format(i))
+        if i > 1 then
+            local prev = versions[i - 1].ver
+            t.assert(prev < ver, ('versions <, %d'):format(i))
+            t.assert(prev <= ver, ('versions <=, %d'):format(i))
+            t.assert(not (prev > ver), ('versions not >, %d'):format(i))
+            t.assert(not (prev >= ver), ('versions not >=, %d'):format(i))
+
+            t.assert(not (ver < prev), ('versions not <, %d'):format(i))
+            t.assert(not (ver <= prev), ('versions not <=, %d'):format(i))
+            t.assert(ver > prev, ('versions >, %d'):format(i))
+            t.assert(ver >= prev, ('versions >=, %d'):format(i))
+
+            t.assert(ver ~= prev, ('versions ~=, %d'):format(i))
+            t.assert(not (ver == prev), ('versions not ==, %d'):format(i))
+        end
+    end
+end
+
+g.test_error = function(g)
+    t.assert_error_msg_contains('Could not parse version', lversion.parse,
+                                'bad version')
+    t.assert_error_msg_contains('Could not parse version', lversion.parse,
+                                '1.x.x')
+    t.assert_error_msg_contains('Could not parse version', lversion.parse,
+                                '1.2.x')
+end
diff --git a/vshard/CMakeLists.txt b/vshard/CMakeLists.txt
index 2d9be25..29500f6 100644
--- a/vshard/CMakeLists.txt
+++ b/vshard/CMakeLists.txt
@@ -7,5 +7,5 @@ add_subdirectory(router)
 
 # Install module
 install(FILES cfg.lua error.lua consts.lua hash.lua init.lua replicaset.lua
-        util.lua rlist.lua heap.lua registry.lua
+        util.lua rlist.lua heap.lua registry.lua version.lua
         DESTINATION ${TARANTOOL_INSTALL_LUADIR}/vshard)
diff --git a/vshard/router/init.lua b/vshard/router/init.lua
index 064bd5a..44ed801 100644
--- a/vshard/router/init.lua
+++ b/vshard/router/init.lua
@@ -7,7 +7,7 @@ local MODULE_INTERNALS = '__module_vshard_router'
 -- Reload requirements, in case this module is reloaded manually.
 if rawget(_G, MODULE_INTERNALS) then
     local vshard_modules = {
-        'vshard.consts', 'vshard.error', 'vshard.cfg',
+        'vshard.consts', 'vshard.error', 'vshard.cfg', 'vshard.version',
         'vshard.hash', 'vshard.replicaset', 'vshard.util'
     }
     for _, module in pairs(vshard_modules) do
diff --git a/vshard/storage/init.lua b/vshard/storage/init.lua
index 1642609..6820ad0 100644
--- a/vshard/storage/init.lua
+++ b/vshard/storage/init.lua
@@ -13,7 +13,7 @@ local MODULE_INTERNALS = '__module_vshard_storage'
 -- Reload requirements, in case this module is reloaded manually.
 if rawget(_G, MODULE_INTERNALS) then
     local vshard_modules = {
-        'vshard.consts', 'vshard.error', 'vshard.cfg',
+        'vshard.consts', 'vshard.error', 'vshard.cfg', 'vshard.version',
         'vshard.replicaset', 'vshard.util',
         'vshard.storage.reload_evolution', 'vshard.rlist', 'vshard.registry',
         'vshard.heap', 'vshard.storage.ref', 'vshard.storage.sched',
diff --git a/vshard/util.lua b/vshard/util.lua
index 366fdea..9e0212a 100644
--- a/vshard/util.lua
+++ b/vshard/util.lua
@@ -2,6 +2,7 @@
 local log = require('log')
 local fiber = require('fiber')
 local lerror = require('vshard.error')
+local lversion = require('vshard.version')
 
 local MODULE_INTERNALS = '__module_vshard_util'
 local M = rawget(_G, MODULE_INTERNALS)
@@ -16,13 +17,7 @@ if not M then
     rawset(_G, MODULE_INTERNALS, M)
 end
 
-local version_str = _TARANTOOL:sub(1, _TARANTOOL:find('-')-1)
-local dot = version_str:find('%.')
-local major = tonumber(version_str:sub(1, dot - 1))
-version_str = version_str:sub(dot + 1)
-dot = version_str:find('%.')
-local middle = tonumber(version_str:sub(1, dot - 1))
-local minor = tonumber(version_str:sub(dot + 1))
+local tnt_version = lversion.parse(_TARANTOOL)
 
 --
 -- Extract parts of a tuple.
@@ -146,12 +141,8 @@ end
 --
 -- Check if Tarantool version is >= that a specified one.
 --
-local function version_is_at_least(major_need, middle_need, minor_need)
-    if major > major_need then return true end
-    if major < major_need then return false end
-    if middle > middle_need then return true end
-    if middle < middle_need then return false end
-    return minor >= minor_need
+local function version_is_at_least(...)
+    return tnt_version >= lversion.new(...)
 end
 
 if not version_is_at_least(1, 10, 1) then
diff --git a/vshard/version.lua b/vshard/version.lua
new file mode 100644
index 0000000..578f6a8
--- /dev/null
+++ b/vshard/version.lua
@@ -0,0 +1,148 @@
+--
+-- Semver parser adopted to Tarantool's versions.
+-- Almost everything is the same as in https://semver.org.
+--
+-- Tarantool's version has format:
+--
+--     x.x.x-typen-commit-ghash
+--
+-- * x.x.x - major, middle, minor release numbers;
+-- * typen - release type and its optional number: alpha1, beta5, rc10.
+--   Optional;
+-- * commit - commit count since the latest release. Optional;
+-- * ghash - latest commit hash in format g<hash>. Optional.
+--
+-- Differences with the semver docs:
+--
+-- * No support for nested releases like x.x.x-alpha.beta. Only x.x.x-alpha.
+-- * Release number is written right after its type. Not 'alpha.1' but 'alpha1'.
+--
+
+local release_type_weight = {
+    alpha = 0,
+    beta = 1,
+    rc = 2,
+}
+
+local function release_type_cmp(t1, t2)
+    t1 = release_type_weight[t1]
+    t2 = release_type_weight[t2]
+    -- 'No release type' means the greatest.
+    if not t1 then
+        if not t2 then
+            return 0
+        end
+        return 1
+    end
+    if not t2 then
+        return -1
+    end
+    return t1 - t2
+end
+
+local function version_cmp(ver1, ver2)
+    if ver1.id_major ~= ver2.id_major then
+        return ver1.id_major - ver2.id_major
+    end
+    if ver1.id_middle ~= ver2.id_middle then
+        return ver1.id_middle - ver2.id_middle
+    end
+    if ver1.id_minor ~= ver2.id_minor then
+        return ver1.id_minor - ver2.id_minor
+    end
+    if ver1.rel_type ~= ver2.rel_type then
+        return release_type_cmp(ver1.rel_type, ver2.rel_type)
+    end
+    if ver1.rel_num ~= ver2.rel_num then
+        return ver1.rel_num - ver2.rel_num
+    end
+    if ver1.id_commit ~= ver2.id_commit then
+        return ver1.id_commit - ver2.id_commit
+    end
+    return 0
+end
+
+local version_mt = {
+    __eq = function(l, r)
+        return version_cmp(l, r) == 0
+    end,
+    __lt = function(l, r)
+        return version_cmp(l, r) < 0
+    end,
+    __le = function(l, r)
+        return version_cmp(l, r) <= 0
+    end,
+}
+
+local function version_new(id_major, id_middle, id_minor, rel_type, rel_num,
+                           id_commit)
+    -- There is no any validation - the API is not public.
+    return setmetatable({
+        id_major = id_major,
+        id_middle = id_middle,
+        id_minor = id_minor,
+        rel_type = rel_type,
+        rel_num = rel_num,
+        id_commit = id_commit,
+    }, version_mt)
+end
+
+local function version_parse(version_str)
+    --  x.x.x-name<num>-<num>-g<commit>
+    -- \____/\___/\___/\_____/
+    --   P1   P2   P3    P4
+    local id_major, id_middle, id_minor
+    local rel_type
+    local rel_num = 0
+    local id_commit = 0
+    local pos
+
+    -- Part 1 - version ID triplet.
+    id_major, id_middle, id_minor = version_str:match('^(%d+)%.(%d+)%.(%d+)')
+    if not id_major or not id_middle or not id_minor then
+        error(('Could not parse version: %s'):format(version_str))
+    end
+    id_major = tonumber(id_major)
+    id_middle = tonumber(id_middle)
+    id_minor = tonumber(id_minor)
+
+    -- Cut to 'name<num>-<num>-g<commit>'.
+    pos = version_str:find('-')
+    if not pos then
+        goto finish
+    end
+    version_str = version_str:sub(pos + 1)
+
+    -- Part 2 and 3 - release name, might be absent.
+    rel_type, rel_num = version_str:match('^(%a+)(%d+)')
+    if not rel_type then
+        rel_type = version_str:match('^(%a+)')
+        rel_num = 0
+    else
+        rel_num = tonumber(rel_num)
+    end
+
+    -- Cut to '<num>-g<commit>'.
+    pos = version_str:find('-')
+    if not pos then
+        goto finish
+    end
+    version_str = version_str:sub(pos + 1)
+
+    -- Part 4 - commit count since latest release, might be absent.
+    id_commit = version_str:match('^(%d+)')
+    if not id_commit then
+        id_commit = 0
+    else
+        id_commit = tonumber(id_commit)
+    end
+
+::finish::
+    return version_new(id_major, id_middle, id_minor, rel_type, rel_num,
+                       id_commit)
+end
+
+return {
+    parse = version_parse,
+    new = version_new,
+}
-- 
2.24.3 (Apple Git-128)


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

* [Tarantool-patches] [PATCH vshard 3/4] router: support msgpack object args
  2022-02-09  0:32 [Tarantool-patches] [PATCH vshard 0/4] Router msgpack object and netbox return_raw Vladislav Shpilevoy via Tarantool-patches
  2022-02-09  0:32 ` [Tarantool-patches] [PATCH vshard 1/4] test: support luatest Vladislav Shpilevoy via Tarantool-patches
  2022-02-09  0:32 ` [Tarantool-patches] [PATCH vshard 2/4] util: introduce Tarantool's semver parser Vladislav Shpilevoy via Tarantool-patches
@ 2022-02-09  0:32 ` Vladislav Shpilevoy via Tarantool-patches
  2022-02-09 17:53   ` Oleg Babin via Tarantool-patches
  2022-02-09  0:32 ` [Tarantool-patches] [PATCH vshard 4/4] router: support netbox return_raw Vladislav Shpilevoy via Tarantool-patches
  2022-02-11 23:05 ` [Tarantool-patches] [PATCH vshard 0/4] Router msgpack object and " Vladislav Shpilevoy via Tarantool-patches
  4 siblings, 1 reply; 20+ messages in thread
From: Vladislav Shpilevoy via Tarantool-patches @ 2022-02-09  0:32 UTC (permalink / raw)
  To: tarantool-patches, olegrok

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 | 47 +++++++++++++++++++++++++++++
 vshard/replicaset.lua               |  4 ---
 vshard/util.lua                     |  9 ++++++
 4 files changed, 57 insertions(+), 4 deletions(-)

diff --git a/test/instances/router.lua b/test/instances/router.lua
index ccec6c1..921ac73 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..13ab74d 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,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
+
+g.test_msgpack_args = function(g)
+    local router = g.router
+    --
+    -- Normal call ro.
+    --
+    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
+
+end -- vutil.feature.msgpack_object
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..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),
+}
+
 return {
     tuple_extract_key = tuple_extract_key,
     reloadable_fiber_create = reloadable_fiber_create,
@@ -241,4 +249,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)


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

* [Tarantool-patches] [PATCH vshard 4/4] router: support netbox return_raw
  2022-02-09  0:32 [Tarantool-patches] [PATCH vshard 0/4] Router msgpack object and netbox return_raw Vladislav Shpilevoy via Tarantool-patches
                   ` (2 preceding siblings ...)
  2022-02-09  0:32 ` [Tarantool-patches] [PATCH vshard 3/4] router: support msgpack object args Vladislav Shpilevoy via Tarantool-patches
@ 2022-02-09  0:32 ` Vladislav Shpilevoy via Tarantool-patches
  2022-02-09 17:53   ` Oleg Babin via Tarantool-patches
  2022-02-11 23:05 ` [Tarantool-patches] [PATCH vshard 0/4] Router msgpack object and " Vladislav Shpilevoy via Tarantool-patches
  4 siblings, 1 reply; 20+ messages in thread
From: Vladislav Shpilevoy via Tarantool-patches @ 2022-02-09  0:32 UTC (permalink / raw)
  To: tarantool-patches, olegrok

Netbox return_raw option allows to return remote call result
without decoding, as an msgpack object. It will be introduced
after 2.10.0-beta2.

It can be used with router working as a proxy. Pass arguments to
it in msgpack object and get back another msgpack object by usage
of return_raw option. This way the router won't decode nor encode
user's function arguments at all.

Part of #312
---
 test/instances/storage.lua           |  5 ++
 test/reload_evolution/storage.result |  4 --
 test/router-luatest/router_test.lua  | 86 ++++++++++++++++++++++++++++
 test/storage/storage.result          |  2 -
 vshard/router/init.lua               | 28 ++++++++-
 vshard/storage/init.lua              | 15 +++++
 vshard/util.lua                      |  4 +-
 7 files changed, 136 insertions(+), 8 deletions(-)

diff --git a/test/instances/storage.lua b/test/instances/storage.lua
index 2d679ba..4a05d82 100755
--- a/test/instances/storage.lua
+++ b/test/instances/storage.lua
@@ -12,10 +12,15 @@ box.ctl.set_on_shutdown_timeout(0.001)
 box.cfg(helpers.box_cfg())
 box.schema.user.grant('guest', 'super', nil, nil, {if_not_exists = true})
 
+local function box_error()
+    box.error(box.error.PROC_LUA, 'box_error')
+end
+
 local function echo(...)
     return ...
 end
 
+_G.box_error = box_error
 _G.echo = echo
 
 _G.ready = true
diff --git a/test/reload_evolution/storage.result b/test/reload_evolution/storage.result
index 77010a2..a282bba 100644
--- a/test/reload_evolution/storage.result
+++ b/test/reload_evolution/storage.result
@@ -122,8 +122,6 @@ vshard.storage.call(bucket_id_to_move, 'read', 'do_select', {42})
 ---
 - true
 - - [42, 3000]
-- null
-- null
 ...
 vshard.storage.bucket_send(bucket_id_to_move, util.replicasets[1])
 ---
@@ -155,8 +153,6 @@ vshard.storage.call(bucket_id_to_move, 'read', 'do_select', {42})
 ---
 - true
 - - [42, 3000]
-- null
-- null
 ...
 -- Check info() does not fail.
 vshard.storage.info() ~= nil
diff --git a/test/router-luatest/router_test.lua b/test/router-luatest/router_test.lua
index 13ab74d..23929dc 100644
--- a/test/router-luatest/router_test.lua
+++ b/test/router-luatest/router_test.lua
@@ -99,3 +99,89 @@ g.test_msgpack_args = function(g)
 end
 
 end -- vutil.feature.msgpack_object
+
+if vutil.feature.netbox_return_raw then
+
+local function test_return_raw_template(g, mode)
+    local router = g.router
+    --
+    -- Normal call.
+    --
+    local res = router:exec(function(timeout, mode)
+        return add_details(vshard.router[mode](1, 'echo', {1, 2, 3},
+                           {timeout = timeout, return_raw = true}))
+    end, {wait_timeout, mode})
+    t.assert_equals(res.val, {1, 2, 3}, 'value value')
+    t.assert_equals(res.val_type, 'userdata', 'value type')
+    t.assert(not res.err, 'no error')
+
+    --
+    -- Route call.
+    --
+    res = router:exec(function(timeout, mode)
+        local route = vshard.router.route(1)
+        return add_details(route[mode](route, 'echo', {1, 2, 3},
+                           {timeout = timeout, return_raw = true}))
+    end, {wait_timeout, mode})
+    t.assert_equals(res.val, {1, 2, 3}, 'value value')
+    t.assert_equals(res.val_type, 'userdata', 'value type')
+    t.assert(not res.err, 'no error')
+
+    --
+    -- Empty result set.
+    --
+    res = router:exec(function(timeout, mode)
+        return add_details(vshard.router[mode](1, 'echo', {},
+                           {timeout = timeout, return_raw = true}))
+    end, {wait_timeout, mode})
+    t.assert(not res.val, 'no value')
+    t.assert(not res.err, 'no error')
+
+    --
+    -- Error.
+    --
+    res = router:exec(function(timeout, mode)
+        return add_details(vshard.router[mode](1, 'box_error', {1, 2, 3},
+                           {timeout = timeout}))
+    end, {wait_timeout, mode})
+    t.assert(not res.val, 'no value')
+    t.assert_equals(res.err_type, 'table', 'error type')
+    t.assert_covers(res.err, {type = 'ClientError', code = box.error.PROC_LUA},
+                    'error value')
+
+    --
+    -- Route error.
+    --
+    res = router:exec(function(timeout, mode)
+        local route = vshard.router.route(1)
+        return add_details(route[mode](route, 'box_error', {1, 2, 3},
+                           {timeout = timeout}))
+    end, {wait_timeout, mode})
+    t.assert(not res.val, 'no value')
+    t.assert_equals(res.err_type, 'table', 'error type')
+    t.assert_covers(res.err, {type = 'ClientError', code = box.error.PROC_LUA},
+                    'error value')
+end
+
+g.test_return_raw = function(g)
+    g.router:exec(function()
+        rawset(_G, 'add_details', function(val, err)
+            -- Direct return would turn nils into box.NULLs. The tests want to
+            -- ensure it doesn't happen. Table wrap makes the actual nils
+            -- eliminate themselves.
+            return {
+                val = val,
+                val_type = type(val),
+                err = err,
+                err_type = type(err),
+            }
+        end)
+    end)
+    test_return_raw_template(g, 'callrw')
+    test_return_raw_template(g, 'callro')
+    g.router:exec(function()
+        _G.add_details = nil
+    end)
+end
+
+end -- vutil.feature.netbox_return_raw
diff --git a/test/storage/storage.result b/test/storage/storage.result
index dcd1c1f..73c171a 100644
--- a/test/storage/storage.result
+++ b/test/storage/storage.result
@@ -480,8 +480,6 @@ vshard.storage.call(1, 'read', 'space_get', {'test', {1}})
 ---
 - true
 - [1, 1]
-- null
-- null
 ...
 vshard.storage.call(100500, 'read', 'space_get', {'test', {1}})
 ---
diff --git a/vshard/router/init.lua b/vshard/router/init.lua
index 44ed801..d39f489 100644
--- a/vshard/router/init.lua
+++ b/vshard/router/init.lua
@@ -1,7 +1,10 @@
 local log = require('log')
 local lfiber = require('fiber')
+local lmsgpack = require('msgpack')
 local table_new = require('table.new')
 local fiber_clock = lfiber.clock
+local msgpack_is_object = lmsgpack.is_object
+local msgpack_object = lmsgpack.object
 
 local MODULE_INTERNALS = '__module_vshard_router'
 -- Reload requirements, in case this module is reloaded manually.
@@ -530,14 +533,17 @@ end
 --
 local function router_call_impl(router, bucket_id, mode, prefer_replica,
                                 balance, func, args, opts)
+    local do_return_raw
     if opts then
         if type(opts) ~= 'table' or
            (opts.timeout and type(opts.timeout) ~= 'number') then
             error('Usage: call(bucket_id, mode, func, args, opts)')
         end
         opts = table.copy(opts)
-    elseif not opts then
+        do_return_raw = opts.return_raw
+    else
         opts = {}
+        do_return_raw = false
     end
     local timeout = opts.timeout or consts.CALL_TIMEOUT_MIN
     local replicaset, err
@@ -569,6 +575,26 @@ local function router_call_impl(router, bucket_id, mode, prefer_replica,
             local storage_call_status, call_status, call_error =
                 replicaset[call](replicaset, 'vshard.storage.call',
                                  {bucket_id, mode, func, args}, opts)
+            if do_return_raw and msgpack_is_object(storage_call_status) then
+                -- Storage.call returns in the first value a flag whether user's
+                -- function threw an exception or not. Need to extract it.
+                -- Unfortunately, it forces to repack the rest of values into a
+                -- new array. But the values themselves are not decoded.
+                local it = storage_call_status:iterator()
+                local count = it:decode_array_header()
+                storage_call_status = it:decode()
+                -- When no values, nil is not packed into msgpack object. Same
+                -- as in raw netbox.
+                if count > 1 then
+                    count = count - 1
+                    local res = table_new(count, 0)
+                    for i = 1, count do
+                        res[i] = it:take()
+                    end
+                    call_status = msgpack_object(res)
+                end
+                call_error = nil
+            end
             if storage_call_status then
                 if call_status == nil and call_error ~= nil then
                     return call_status, call_error
diff --git a/vshard/storage/init.lua b/vshard/storage/init.lua
index 6820ad0..5083911 100644
--- a/vshard/storage/init.lua
+++ b/vshard/storage/init.lua
@@ -2512,6 +2512,21 @@ local function storage_call(bucket_id, mode, name, args)
     if not ok then
         ret1 = lerror.make(ret1)
     end
+    -- Truncate nil values. Can't return them all because empty values turn into
+    -- box.NULL. Even if user's function actually returned just 1 value, this
+    -- would lead to '1, box.NULL, box.NULL' on the client. Not visible on the
+    -- router when called normally, but won't help if the router did
+    -- 'return_raw' and just forwards everything as is without truncation.
+    -- Although this solution truncates really returned nils.
+    if ret3 == nil then
+        if ret2 == nil then
+            if ret1 == nil then
+                return ok
+            end
+            return ok, ret1
+        end
+        return ok, ret1, ret2
+    end
     return ok, ret1, ret2, ret3
 end
 
diff --git a/vshard/util.lua b/vshard/util.lua
index 72176f7..ad469f2 100644
--- a/vshard/util.lua
+++ b/vshard/util.lua
@@ -234,8 +234,10 @@ 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 is_ge_2_10_0_beta2_86 = version_is_at_least(2, 10, 0, 'beta', 2, 86)
 local feature = {
-    msgpack_object = version_is_at_least(2, 10, 0, 'beta', 2, 86),
+    msgpack_object = is_ge_2_10_0_beta2_86,
+    netbox_return_raw = is_ge_2_10_0_beta2_86,
 }
 
 return {
-- 
2.24.3 (Apple Git-128)


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

* Re: [Tarantool-patches] [PATCH vshard 1/4] test: support luatest
  2022-02-09  0:32 ` [Tarantool-patches] [PATCH vshard 1/4] test: support luatest Vladislav Shpilevoy via Tarantool-patches
@ 2022-02-09 17:53   ` Oleg Babin via Tarantool-patches
  2022-02-10 22:32     ` Vladislav Shpilevoy via Tarantool-patches
  0 siblings, 1 reply; 20+ messages in thread
From: Oleg Babin via Tarantool-patches @ 2022-02-09 17:53 UTC (permalink / raw)
  To: Vladislav Shpilevoy, tarantool-patches

Thanks for your patch.

Am I right that it's partially imported from tarantool 
https://github.com/tarantool/tarantool/tree/master/test/luatest_helpers ?


LGTM. See 2 minor comments/questions below.



On 09.02.2022 03:32, Vladislav Shpilevoy wrote:
> Test-run's most recent guideline is to write new tests in luatest
> instead of diff console tests when possible. Luatest isn't exactly
> in a perfect condition now, but it has 2 important features which
> would be very useful in vshard:
>
> - Easy cluster build. All can be done programmatically except a
>    few basic things - from config creation to all replicasets
>    start, router start, and buckets bootstrap. No need to hardcode
>    that into files like storage_1_a.lua, router_1.lua, etc.
>
> - Can opt-out certain tests depending on Tarantool version. For
>    instance, soon coming support for netbox's return_raw option
>    will need to run tests only for > 2.10.0-beta2. In diff tests it
>    is also possible but would be notably complicated to achieve.
>
> Needed for #312
> ---
> All luatest_helpers/ except vtest.lua are blindly copied from Tarantool's main
> repo. I didn't check their bugs, code style, etc.
>
>   test-run                            |   2 +-
>   test/instances/router.lua           |  15 ++
>   test/instances/storage.lua          |  21 +++
>   test/luatest_helpers.lua            |  72 ++++++++
>   test/luatest_helpers/asserts.lua    |  43 +++++
>   test/luatest_helpers/cluster.lua    | 132 ++++++++++++++
>   test/luatest_helpers/server.lua     | 266 ++++++++++++++++++++++++++++
>   test/luatest_helpers/vtest.lua      | 135 ++++++++++++++
>   test/router-luatest/router_test.lua |  54 ++++++
>   test/router-luatest/suite.ini       |   5 +
>   10 files changed, 744 insertions(+), 1 deletion(-)
>   create mode 100755 test/instances/router.lua
>   create mode 100755 test/instances/storage.lua
>   create mode 100644 test/luatest_helpers.lua
>   create mode 100644 test/luatest_helpers/asserts.lua
>   create mode 100644 test/luatest_helpers/cluster.lua
>   create mode 100644 test/luatest_helpers/server.lua
>   create mode 100644 test/luatest_helpers/vtest.lua
>   create mode 100644 test/router-luatest/router_test.lua
>   create mode 100644 test/router-luatest/suite.ini
>
> diff --git a/test-run b/test-run
> index c345003..2604c46 160000
> --- a/test-run
> +++ b/test-run
> @@ -1 +1 @@
> -Subproject commit c34500365efe8316e79c7936a2f2d04644602936
> +Subproject commit 2604c46c7b6368dbde59489d5303ce3d1d430331
> diff --git a/test/instances/router.lua b/test/instances/router.lua
> new file mode 100755
> index 0000000..ccec6c1
> --- /dev/null
> +++ b/test/instances/router.lua
> @@ -0,0 +1,15 @@
> +#!/usr/bin/env tarantool
> +local helpers = require('test.luatest_helpers')
> +-- Do not load entire vshard into the global namespace to catch errors when code
> +-- relies on that.
> +_G.vshard = {
> +    router = require('vshard.router'),
> +}
> +-- Somewhy shutdown hangs on new Tarantools even though the nodes do not seem to
> +-- have any long requests running.
> +box.ctl.set_on_shutdown_timeout(0.001)
> +
> +box.cfg(helpers.box_cfg())
> +box.schema.user.grant('guest', 'super', nil, nil, {if_not_exists = true})
> +
> +_G.ready = true
> diff --git a/test/instances/storage.lua b/test/instances/storage.lua
> new file mode 100755
> index 0000000..2d679ba
> --- /dev/null
> +++ b/test/instances/storage.lua
> @@ -0,0 +1,21 @@
> +#!/usr/bin/env tarantool
> +local helpers = require('test.luatest_helpers')
> +-- Do not load entire vshard into the global namespace to catch errors when code
> +-- relies on that.
> +_G.vshard = {
> +    storage = require('vshard.storage'),
> +}
> +-- Somewhy shutdown hangs on new Tarantools even though the nodes do not seem to
> +-- have any long requests running.
> +box.ctl.set_on_shutdown_timeout(0.001)
> +
> +box.cfg(helpers.box_cfg())
> +box.schema.user.grant('guest', 'super', nil, nil, {if_not_exists = true})
> +
> +local function echo(...)
> +    return ...
> +end
> +
> +_G.echo = echo
> +
> +_G.ready = true
> diff --git a/test/luatest_helpers.lua b/test/luatest_helpers.lua
> new file mode 100644
> index 0000000..283906c
> --- /dev/null
> +++ b/test/luatest_helpers.lua
> @@ -0,0 +1,72 @@
> +local fun = require('fun')
> +local json = require('json')
> +local fio = require('fio')
> +local log = require('log')
> +local yaml = require('yaml')
> +local fiber = require('fiber')
> +
> +local luatest_helpers = {
> +    SOCKET_DIR = fio.abspath(os.getenv('VARDIR') or 'test/var')
> +}

Is fio.abspath really needed here? AFAIK the max length of unix socket 
is 108 symbols. Relative paths give a more chances that we don't face 
any issues.

> +
> +luatest_helpers.Server = require('test.luatest_helpers.server')
> +
> +local function default_cfg()
> +    return {
> +        work_dir = os.getenv('TARANTOOL_WORKDIR'),
> +        listen = os.getenv('TARANTOOL_LISTEN'),
> +        log = ('%s/%s.log'):format(os.getenv('TARANTOOL_WORKDIR'), os.getenv('TARANTOOL_ALIAS')),
> +    }
> +end
> +
> +local function env_cfg()
> +    local src = os.getenv('TARANTOOL_BOX_CFG')
> +    if src == nil then
> +        return {}
> +    end
> +    local res = json.decode(src)
> +    assert(type(res) == 'table')
> +    return res
> +end
> +
> +-- Collect box.cfg table from values passed through
> +-- luatest_helpers.Server({<...>}) and from the given argument.
> +--
> +-- Use it from inside an instance script.
> +function luatest_helpers.box_cfg(cfg)
> +    return fun.chain(default_cfg(), env_cfg(), cfg or {}):tomap()
> +end
> +
> +function luatest_helpers.instance_uri(alias, instance_id)
> +    if instance_id == nil then
> +        instance_id = ''
> +    end
> +    instance_id = tostring(instance_id)
> +    return ('%s/%s%s.iproto'):format(luatest_helpers.SOCKET_DIR, alias, instance_id);
> +end
> +
> +function luatest_helpers:get_vclock(server)
> +    return server:eval('return box.info.vclock')
> +end
> +
> +function luatest_helpers:wait_vclock(server, to_vclock)
> +    while true do
> +        local vclock = self:get_vclock(server)
> +        local ok = true
> +        for server_id, to_lsn in pairs(to_vclock) do
> +            local lsn = vclock[server_id]
> +            if lsn == nil or lsn < to_lsn then
> +                ok = false
> +                break
> +            end
> +        end
> +        if ok then
> +            return
> +        end
> +        log.info("wait vclock: %s to %s", yaml.encode(vclock),
> +                 yaml.encode(to_vclock))
> +        fiber.sleep(0.001)
> +    end
> +end
> +
> +return luatest_helpers
> diff --git a/test/luatest_helpers/asserts.lua b/test/luatest_helpers/asserts.lua
> new file mode 100644
> index 0000000..77385d8
> --- /dev/null
> +++ b/test/luatest_helpers/asserts.lua
> @@ -0,0 +1,43 @@
> +local t = require('luatest')
> +
> +local asserts = {}
> +
> +function asserts:new(object)
> +    self:inherit(object)
> +    object:initialize()
> +    return object
> +end
> +
> +function asserts:inherit(object)
> +    object = object or {}
> +    setmetatable(object, self)
> +    self.__index = self
> +    return object
> +end
> +
> +function asserts:assert_server_follow_upstream(server, id)
> +    local status = server:eval(
> +        ('return box.info.replication[%d].upstream.status'):format(id))
> +    t.assert_equals(status, 'follow',
> +        ('%s: this server does not follow others.'):format(server.alias))
> +end
> +
> +
> +function asserts:wait_fullmesh(servers, wait_time)
> +    wait_time = wait_time or 20
> +    t.helpers.retrying({timeout = wait_time}, function()
> +        for _, server in pairs(servers) do
> +            for _, server2 in pairs(servers) do
> +                if server ~= server2 then
> +                    local server_id = server:eval('return box.info.id')
> +                    local server2_id = server2:eval('return box.info.id')
> +                    if server_id ~= server2_id then
> +                            self:assert_server_follow_upstream(server, server2_id)
Indention looks broken here (8 spaces instead of 4).
> +                    end
> +                end
> +            end
> +        end
> +    end)
> +end
> +
> +return asserts
> diff --git a/test/luatest_helpers/cluster.lua b/test/luatest_helpers/cluster.lua
> new file mode 100644
> index 0000000..43e3479

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

* Re: [Tarantool-patches] [PATCH vshard 2/4] util: introduce Tarantool's semver parser
  2022-02-09  0:32 ` [Tarantool-patches] [PATCH vshard 2/4] util: introduce Tarantool's semver parser Vladislav Shpilevoy via Tarantool-patches
@ 2022-02-09 17:53   ` Oleg Babin via Tarantool-patches
  2022-02-10 22:33     ` Vladislav Shpilevoy via Tarantool-patches
  0 siblings, 1 reply; 20+ messages in thread
From: Oleg Babin via Tarantool-patches @ 2022-02-09 17:53 UTC (permalink / raw)
  To: Vladislav Shpilevoy, tarantool-patches

Thanks for your patch.


Looks good. Probably it's worth to move this code into separate module.

There are several modules in Tarantool ecosystem that implement its own 
parsers.

One note below.


On 09.02.2022 03:32, Vladislav Shpilevoy wrote:
> Tarantool's version since recently has semver-like format. It
> made impossible to check for features existence if they were
> introduced not in the first release of one x.x.x triplet.
>
> An alternative would be to test for feature existence by its
> usage, but it is not always possible with ease.
>
> This patch improves version parser in vshard to understand new
> version names.
>
> It will be used by a future commit about netbox's return_raw
> feature adoption to skip its tests for old versions
> (<= 2.10.0-beta2).
>
<stripped>

> +local release_type_weight = {
> +    alpha = 0,
> +    beta = 1,
> +    rc = 2,
> +}
> +
> +local function release_type_cmp(t1, t2)
> +    t1 = release_type_weight[t1]
> +    t2 = release_type_weight[t2]
> +    -- 'No release type' means the greatest.
> +    if not t1 then
> +        if not t2 then
> +            return 0
> +        end
> +        return 1
> +    end
> +    if not t2 then
> +        return -1
> +    end
> +    return t1 - t2
> +end
> +
> +local function version_cmp(ver1, ver2)
> +    if ver1.id_major ~= ver2.id_major then
> +        return ver1.id_major - ver2.id_major
> +    end
> +    if ver1.id_middle ~= ver2.id_middle then
> +        return ver1.id_middle - ver2.id_middle
> +    end
> +    if ver1.id_minor ~= ver2.id_minor then
> +        return ver1.id_minor - ver2.id_minor
> +    end
> +    if ver1.rel_type ~= ver2.rel_type then
> +        return release_type_cmp(ver1.rel_type, ver2.rel_type)
> +    end
> +    if ver1.rel_num ~= ver2.rel_num then
> +        return ver1.rel_num - ver2.rel_num
> +    end
> +    if ver1.id_commit ~= ver2.id_commit then
> +        return ver1.id_commit - ver2.id_commit
> +    end
> +    return 0
> +end
> +
> +local version_mt = {
> +    __eq = function(l, r)
> +        return version_cmp(l, r) == 0
> +    end,
> +    __lt = function(l, r)
> +        return version_cmp(l, r) < 0
> +    end,
> +    __le = function(l, r)
> +        return version_cmp(l, r) <= 0
> +    end,
> +}
> +
> +local function version_new(id_major, id_middle, id_minor, rel_type, rel_num,
> +                           id_commit)

I think we could validate that at least id_major is not nil.

> +    -- There is no any validation - the API is not public.
> +    return setmetatable({
> +        id_major = id_major,
> +        id_middle = id_middle,
> +        id_minor = id_minor,
> +        rel_type = rel_type,
> +        rel_num = rel_num,
> +        id_commit = id_commit,
> +    }, version_mt)
> +end
> +
>

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

* Re: [Tarantool-patches] [PATCH vshard 3/4] router: support msgpack object args
  2022-02-09  0:32 ` [Tarantool-patches] [PATCH vshard 3/4] router: support msgpack object args Vladislav Shpilevoy via Tarantool-patches
@ 2022-02-09 17:53   ` Oleg Babin via Tarantool-patches
  2022-02-10 22:33     ` Vladislav Shpilevoy via Tarantool-patches
  0 siblings, 1 reply; 20+ messages in thread
From: Oleg Babin via Tarantool-patches @ 2022-02-09 17:53 UTC (permalink / raw)
  To: Vladislav Shpilevoy, tarantool-patches

Thanks for your patch. Two comments below.


On 09.02.2022 03:32, Vladislav Shpilevoy wrote:
> 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 | 47 +++++++++++++++++++++++++++++
>   vshard/replicaset.lua               |  4 ---
>   vshard/util.lua                     |  9 ++++++
>   4 files changed, 57 insertions(+), 4 deletions(-)
>
> diff --git a/test/instances/router.lua b/test/instances/router.lua
> index ccec6c1..921ac73 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..13ab74d 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,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.
> +
> +g.test_msgpack_args = function(g)
> +    local router = g.router
> +    --
> +    -- Normal call ro.
> +    --
> +    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
> +
> +end -- vutil.feature.msgpack_object
> 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..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`
> +
>   return {
>       tuple_extract_key = tuple_extract_key,
>       reloadable_fiber_create = reloadable_fiber_create,
> @@ -241,4 +249,5 @@ return {
>       table_minus_yield = table_minus_yield,
>       fiber_cond_wait = fiber_cond_wait,
>       fiber_is_self_canceled = fiber_is_self_canceled,
> +    feature = feature,
>   }

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

* Re: [Tarantool-patches] [PATCH vshard 4/4] router: support netbox return_raw
  2022-02-09  0:32 ` [Tarantool-patches] [PATCH vshard 4/4] router: support netbox return_raw Vladislav Shpilevoy via Tarantool-patches
@ 2022-02-09 17:53   ` Oleg Babin via Tarantool-patches
  2022-02-10 22:34     ` Vladislav Shpilevoy via Tarantool-patches
  0 siblings, 1 reply; 20+ messages in thread
From: Oleg Babin via Tarantool-patches @ 2022-02-09 17:53 UTC (permalink / raw)
  To: Vladislav Shpilevoy, tarantool-patches

Thanks for your patch. Two comments below.

On 09.02.2022 03:32, Vladislav Shpilevoy wrote:
> Netbox return_raw option allows to return remote call result
> without decoding, as an msgpack object. It will be introduced
> after 2.10.0-beta2.
>
> It can be used with router working as a proxy. Pass arguments to
> it in msgpack object and get back another msgpack object by usage
> of return_raw option. This way the router won't decode nor encode
> user's function arguments at all.
>
> Part of #312
> ---
>   test/instances/storage.lua           |  5 ++
>   test/reload_evolution/storage.result |  4 --
>   test/router-luatest/router_test.lua  | 86 ++++++++++++++++++++++++++++
>   test/storage/storage.result          |  2 -
>   vshard/router/init.lua               | 28 ++++++++-
>   vshard/storage/init.lua              | 15 +++++
>   vshard/util.lua                      |  4 +-
>   7 files changed, 136 insertions(+), 8 deletions(-)
>
> diff --git a/test/instances/storage.lua b/test/instances/storage.lua
> index 2d679ba..4a05d82 100755
> --- a/test/instances/storage.lua
> +++ b/test/instances/storage.lua
> @@ -12,10 +12,15 @@ box.ctl.set_on_shutdown_timeout(0.001)
>   box.cfg(helpers.box_cfg())
>   box.schema.user.grant('guest', 'super', nil, nil, {if_not_exists = true})
>   
> +local function box_error()
> +    box.error(box.error.PROC_LUA, 'box_error')
> +end
> +
>   local function echo(...)
>       return ...
>   end
>   
> +_G.box_error = box_error
>   _G.echo = echo
>   
>   _G.ready = true
> diff --git a/test/reload_evolution/storage.result b/test/reload_evolution/storage.result
> index 77010a2..a282bba 100644
> --- a/test/reload_evolution/storage.result
> +++ b/test/reload_evolution/storage.result
> @@ -122,8 +122,6 @@ vshard.storage.call(bucket_id_to_move, 'read', 'do_select', {42})
>   ---
>   - true
>   - - [42, 3000]
> -- null
> -- null
>   ...
>   vshard.storage.bucket_send(bucket_id_to_move, util.replicasets[1])
>   ---
> @@ -155,8 +153,6 @@ vshard.storage.call(bucket_id_to_move, 'read', 'do_select', {42})
>   ---
>   - true
>   - - [42, 3000]
> -- null
> -- null
>   ...
>   -- Check info() does not fail.
>   vshard.storage.info() ~= nil
> diff --git a/test/router-luatest/router_test.lua b/test/router-luatest/router_test.lua
> index 13ab74d..23929dc 100644
> --- a/test/router-luatest/router_test.lua
> +++ b/test/router-luatest/router_test.lua
> @@ -99,3 +99,89 @@ g.test_msgpack_args = function(g)
>   end
>   
>   end -- vutil.feature.msgpack_object
> +
> +if vutil.feature.netbox_return_raw then
> +
> +local function test_return_raw_template(g, mode)
> +    local router = g.router
> +    --
> +    -- Normal call.
> +    --
> +    local res = router:exec(function(timeout, mode)
> +        return add_details(vshard.router[mode](1, 'echo', {1, 2, 3},
> +                           {timeout = timeout, return_raw = true}))
> +    end, {wait_timeout, mode})
> +    t.assert_equals(res.val, {1, 2, 3}, 'value value')
> +    t.assert_equals(res.val_type, 'userdata', 'value type')
> +    t.assert(not res.err, 'no error')
> +
> +    --
> +    -- Route call.
> +    --
> +    res = router:exec(function(timeout, mode)
> +        local route = vshard.router.route(1)
> +        return add_details(route[mode](route, 'echo', {1, 2, 3},
> +                           {timeout = timeout, return_raw = true}))
> +    end, {wait_timeout, mode})
> +    t.assert_equals(res.val, {1, 2, 3}, 'value value')
> +    t.assert_equals(res.val_type, 'userdata', 'value type')
> +    t.assert(not res.err, 'no error')
> +
> +    --
> +    -- Empty result set.
> +    --
> +    res = router:exec(function(timeout, mode)
> +        return add_details(vshard.router[mode](1, 'echo', {},
> +                           {timeout = timeout, return_raw = true}))
> +    end, {wait_timeout, mode})
> +    t.assert(not res.val, 'no value')
> +    t.assert(not res.err, 'no error')
> +
> +    --
> +    -- Error.
> +    --
> +    res = router:exec(function(timeout, mode)
> +        return add_details(vshard.router[mode](1, 'box_error', {1, 2, 3},
> +                           {timeout = timeout}))
> +    end, {wait_timeout, mode})
> +    t.assert(not res.val, 'no value')
> +    t.assert_equals(res.err_type, 'table', 'error type')
> +    t.assert_covers(res.err, {type = 'ClientError', code = box.error.PROC_LUA},
> +                    'error value')
> +
> +    --
> +    -- Route error.
> +    --
> +    res = router:exec(function(timeout, mode)
> +        local route = vshard.router.route(1)
> +        return add_details(route[mode](route, 'box_error', {1, 2, 3},
> +                           {timeout = timeout}))
> +    end, {wait_timeout, mode})
> +    t.assert(not res.val, 'no value')
> +    t.assert_equals(res.err_type, 'table', 'error type')
> +    t.assert_covers(res.err, {type = 'ClientError', code = box.error.PROC_LUA},
> +                    'error value')
> +end
> +
> +g.test_return_raw = function(g)

g.skip_if could be used

Also probably you could use parametrization (however I won't insist):

```

local pg = t.group('pgroup', {{mode = 'callrw'}, {engine = 'callro'}})
pg.test = function(cg)
    ...
end

```

> +    g.router:exec(function()
> +        rawset(_G, 'add_details', function(val, err)
> +            -- Direct return would turn nils into box.NULLs. The tests want to
> +            -- ensure it doesn't happen. Table wrap makes the actual nils
> +            -- eliminate themselves.
> +            return {
> +                val = val,
> +                val_type = type(val),
> +                err = err,
> +                err_type = type(err),
> +            }
> +        end)
> +    end)
> +    test_return_raw_template(g, 'callrw')
> +    test_return_raw_template(g, 'callro')
> +    g.router:exec(function()
> +        _G.add_details = nil
> +    end)
> +end
> +
> +end -- vutil.feature.netbox_return_raw
> diff --git a/test/storage/storage.result b/test/storage/storage.result
> index dcd1c1f..73c171a 100644
> --- a/test/storage/storage.result
> +++ b/test/storage/storage.result
> @@ -480,8 +480,6 @@ vshard.storage.call(1, 'read', 'space_get', {'test', {1}})
>   ---
>   - true
>   - [1, 1]
> -- null
> -- null
>   ...
>   vshard.storage.call(100500, 'read', 'space_get', {'test', {1}})
>   ---
> diff --git a/vshard/router/init.lua b/vshard/router/init.lua
> index 44ed801..d39f489 100644
> --- a/vshard/router/init.lua
> +++ b/vshard/router/init.lua
> @@ -1,7 +1,10 @@
>   local log = require('log')
>   local lfiber = require('fiber')
> +local lmsgpack = require('msgpack')
>   local table_new = require('table.new')
>   local fiber_clock = lfiber.clock
> +local msgpack_is_object = lmsgpack.is_object
> +local msgpack_object = lmsgpack.object

I suggest to add something like

```

if msgpack_is_object == nil then

     msgpack_is_object = function() error("Msgpack object feature is not 
supported by current Tarantool version") end

end

```


>   
>   local MODULE_INTERNALS = '__module_vshard_router'
>   -- Reload requirements, in case this module is reloaded manually.
> @@ -530,14 +533,17 @@ end
>   --
>   local function router_call_impl(router, bucket_id, mode, prefer_replica,
>                                   balance, func, args, opts)
> +    local do_return_raw
>       if opts then
>           if type(opts) ~= 'table' or
>              (opts.timeout and type(opts.timeout) ~= 'number') then
>               error('Usage: call(bucket_id, mode, func, args, opts)')
>           end
>           opts = table.copy(opts)
> -    elseif not opts then
> +        do_return_raw = opts.return_raw
> +    else
>           opts = {}
> +        do_return_raw = false
>       end
>       local timeout = opts.timeout or consts.CALL_TIMEOUT_MIN
>       local replicaset, err
> @@ -569,6 +575,26 @@ local function router_call_impl(router, bucket_id, mode, prefer_replica,
>               local storage_call_status, call_status, call_error =
>                   replicaset[call](replicaset, 'vshard.storage.call',
>                                    {bucket_id, mode, func, args}, opts)
> +            if do_return_raw and msgpack_is_object(storage_call_status) then
> +                -- Storage.call returns in the first value a flag whether user's
> +                -- function threw an exception or not. Need to extract it.
> +                -- Unfortunately, it forces to repack the rest of values into a
> +                -- new array. But the values themselves are not decoded.
> +                local it = storage_call_status:iterator()
> +                local count = it:decode_array_header()
> +                storage_call_status = it:decode()
> +                -- When no values, nil is not packed into msgpack object. Same
> +                -- as in raw netbox.
> +                if count > 1 then
> +                    count = count - 1
> +                    local res = table_new(count, 0)
> +                    for i = 1, count do
> +                        res[i] = it:take()
> +                    end
> +                    call_status = msgpack_object(res)
> +                end
> +                call_error = nil
> +            end
>               if storage_call_status then
>                   if call_status == nil and call_error ~= nil then
>                       return call_status, call_error
> diff --git a/vshard/storage/init.lua b/vshard/storage/init.lua
> index 6820ad0..5083911 100644
> --- a/vshard/storage/init.lua
> +++ b/vshard/storage/init.lua
> @@ -2512,6 +2512,21 @@ local function storage_call(bucket_id, mode, name, args)
>       if not ok then
>           ret1 = lerror.make(ret1)
>       end
> +    -- Truncate nil values. Can't return them all because empty values turn into
> +    -- box.NULL. Even if user's function actually returned just 1 value, this
> +    -- would lead to '1, box.NULL, box.NULL' on the client. Not visible on the
> +    -- router when called normally, but won't help if the router did
> +    -- 'return_raw' and just forwards everything as is without truncation.
> +    -- Although this solution truncates really returned nils.
> +    if ret3 == nil then
> +        if ret2 == nil then
> +            if ret1 == nil then
> +                return ok
> +            end
> +            return ok, ret1
> +        end
> +        return ok, ret1, ret2
> +    end
>       return ok, ret1, ret2, ret3
>   end
>   
> diff --git a/vshard/util.lua b/vshard/util.lua
> index 72176f7..ad469f2 100644
> --- a/vshard/util.lua
> +++ b/vshard/util.lua
> @@ -234,8 +234,10 @@ 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 is_ge_2_10_0_beta2_86 = version_is_at_least(2, 10, 0, 'beta', 2, 86)
>   local feature = {
> -    msgpack_object = version_is_at_least(2, 10, 0, 'beta', 2, 86),
> +    msgpack_object = is_ge_2_10_0_beta2_86,
> +    netbox_return_raw = is_ge_2_10_0_beta2_86,
>   }
>   
>   return {

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

* Re: [Tarantool-patches] [PATCH vshard 1/4] test: support luatest
  2022-02-09 17:53   ` Oleg Babin via Tarantool-patches
@ 2022-02-10 22:32     ` Vladislav Shpilevoy via Tarantool-patches
  2022-02-11 16:38       ` Oleg Babin via Tarantool-patches
  0 siblings, 1 reply; 20+ messages in thread
From: Vladislav Shpilevoy via Tarantool-patches @ 2022-02-10 22:32 UTC (permalink / raw)
  To: Oleg Babin, tarantool-patches

Hi! Thanks for the review!

On 09.02.2022 18:53, Oleg Babin wrote:
> Thanks for your patch.
> 
> Am I right that it's partially imported from tarantool https://github.com/tarantool/tarantool/tree/master/test/luatest_helpers ?

It is not just partially imported. It is a complete 100% copy-paste
of everything except vtest.lua. I wrote it under `---` below.

>> diff --git a/test/luatest_helpers.lua b/test/luatest_helpers.lua
>> new file mode 100644
>> index 0000000..283906c
>> --- /dev/null
>> +++ b/test/luatest_helpers.lua
>> @@ -0,0 +1,72 @@
>> +local fun = require('fun')
>> +local json = require('json')
>> +local fio = require('fio')
>> +local log = require('log')
>> +local yaml = require('yaml')
>> +local fiber = require('fiber')
>> +
>> +local luatest_helpers = {
>> +    SOCKET_DIR = fio.abspath(os.getenv('VARDIR') or 'test/var')
>> +}
> 
> Is fio.abspath really needed here? AFAIK the max length of unix socket is 108 symbols. Relative paths give a more chances that we don't face any issues.

VARDIR is already absolute path set by luatest, it won't help much.
As for why fio.abspath is used then - I don't know. I would rather treat
this file as a part of abomination called luatest (which it should have
been from the beginning instead of being copy-pasted across projects) and
try not to change anything here. Unless something breaks.

>> diff --git a/test/luatest_helpers/asserts.lua b/test/luatest_helpers/asserts.lua
>> new file mode 100644
>> index 0000000..77385d8
>> --- /dev/null
>> +++ b/test/luatest_helpers/asserts.lua
>> @@ -0,0 +1,43 @@

<...>

>> +
>> +
>> +function asserts:wait_fullmesh(servers, wait_time)
>> +    wait_time = wait_time or 20
>> +    t.helpers.retrying({timeout = wait_time}, function()
>> +        for _, server in pairs(servers) do
>> +            for _, server2 in pairs(servers) do
>> +                if server ~= server2 then
>> +                    local server_id = server:eval('return box.info.id')
>> +                    local server2_id = server2:eval('return box.info.id')
>> +                    if server_id ~= server2_id then
>> +                            self:assert_server_follow_upstream(server, server2_id)
> Indention looks broken here (8 spaces instead of 4).

Thanks, fixed:

====================
@@ -32,7 +32,7 @@ function asserts:wait_fullmesh(servers, wait_time)
                     local server_id = server:eval('return box.info.id')
                     local server2_id = server2:eval('return box.info.id')
                     if server_id ~= server2_id then
-                            self:assert_server_follow_upstream(server, server2_id)
+                        self:assert_server_follow_upstream(server, server2_id)
                     end
                 end
             end
====================

I also applied this diff to make it run on 1.10:

====================
diff --git a/test/instances/router.lua b/test/instances/router.lua
index ccec6c1..587a473 100755
--- a/test/instances/router.lua
+++ b/test/instances/router.lua
@@ -7,7 +7,9 @@ _G.vshard = {
 }
 -- Somewhy shutdown hangs on new Tarantools even though the nodes do not seem to
 -- have any long requests running.
-box.ctl.set_on_shutdown_timeout(0.001)
+if box.ctl.set_on_shutdown_timeout then
+    box.ctl.set_on_shutdown_timeout(0.001)
+end
 
 box.cfg(helpers.box_cfg())
 box.schema.user.grant('guest', 'super', nil, nil, {if_not_exists = true})
diff --git a/test/instances/storage.lua b/test/instances/storage.lua
index 2d679ba..7ad2af3 100755
--- a/test/instances/storage.lua
+++ b/test/instances/storage.lua
@@ -7,7 +7,9 @@ _G.vshard = {
 }
 -- Somewhy shutdown hangs on new Tarantools even though the nodes do not seem to
 -- have any long requests running.
-box.ctl.set_on_shutdown_timeout(0.001)
+if box.ctl.set_on_shutdown_timeout then
+    box.ctl.set_on_shutdown_timeout(0.001)
+end
 
 box.cfg(helpers.box_cfg())
====================


New patch:

====================
test: support luatest

Test-run's most recent guideline is to write new tests in luatest
instead of diff console tests when possible. Luatest isn't exactly
in a perfect condition now, but it has 2 important features which
would be very useful in vshard:

- Easy cluster build. All can be done programmatically except a
  few basic things - from config creation to all replicasets
  start, router start, and buckets bootstrap. No need to hardcode
  that into files like storage_1_a.lua, router_1.lua, etc.

- Can opt-out certain tests depending on Tarantool version. For
  instance, soon coming support for netbox's return_raw option
  will need to run tests only for > 2.10.0-beta2. In diff tests it
  is also possible but would be notably complicated to achieve.

Needed for #312
---
 test-run                            |   2 +-
 test/instances/router.lua           |  17 ++
 test/instances/storage.lua          |  23 +++
 test/luatest_helpers.lua            |  72 ++++++++
 test/luatest_helpers/asserts.lua    |  43 +++++
 test/luatest_helpers/cluster.lua    | 132 ++++++++++++++
 test/luatest_helpers/server.lua     | 266 ++++++++++++++++++++++++++++
 test/luatest_helpers/vtest.lua      | 135 ++++++++++++++
 test/router-luatest/router_test.lua |  54 ++++++
 test/router-luatest/suite.ini       |   5 +
 10 files changed, 748 insertions(+), 1 deletion(-)
 create mode 100755 test/instances/router.lua
 create mode 100755 test/instances/storage.lua
 create mode 100644 test/luatest_helpers.lua
 create mode 100644 test/luatest_helpers/asserts.lua
 create mode 100644 test/luatest_helpers/cluster.lua
 create mode 100644 test/luatest_helpers/server.lua
 create mode 100644 test/luatest_helpers/vtest.lua
 create mode 100644 test/router-luatest/router_test.lua
 create mode 100644 test/router-luatest/suite.ini

diff --git a/test-run b/test-run
index c345003..2604c46 160000
--- a/test-run
+++ b/test-run
@@ -1 +1 @@
-Subproject commit c34500365efe8316e79c7936a2f2d04644602936
+Subproject commit 2604c46c7b6368dbde59489d5303ce3d1d430331
diff --git a/test/instances/router.lua b/test/instances/router.lua
new file mode 100755
index 0000000..587a473
--- /dev/null
+++ b/test/instances/router.lua
@@ -0,0 +1,17 @@
+#!/usr/bin/env tarantool
+local helpers = require('test.luatest_helpers')
+-- Do not load entire vshard into the global namespace to catch errors when code
+-- relies on that.
+_G.vshard = {
+    router = require('vshard.router'),
+}
+-- Somewhy shutdown hangs on new Tarantools even though the nodes do not seem to
+-- have any long requests running.
+if box.ctl.set_on_shutdown_timeout then
+    box.ctl.set_on_shutdown_timeout(0.001)
+end
+
+box.cfg(helpers.box_cfg())
+box.schema.user.grant('guest', 'super', nil, nil, {if_not_exists = true})
+
+_G.ready = true
diff --git a/test/instances/storage.lua b/test/instances/storage.lua
new file mode 100755
index 0000000..7ad2af3
--- /dev/null
+++ b/test/instances/storage.lua
@@ -0,0 +1,23 @@
+#!/usr/bin/env tarantool
+local helpers = require('test.luatest_helpers')
+-- Do not load entire vshard into the global namespace to catch errors when code
+-- relies on that.
+_G.vshard = {
+    storage = require('vshard.storage'),
+}
+-- Somewhy shutdown hangs on new Tarantools even though the nodes do not seem to
+-- have any long requests running.
+if box.ctl.set_on_shutdown_timeout then
+    box.ctl.set_on_shutdown_timeout(0.001)
+end
+
+box.cfg(helpers.box_cfg())
+box.schema.user.grant('guest', 'super', nil, nil, {if_not_exists = true})
+
+local function echo(...)
+    return ...
+end
+
+_G.echo = echo
+
+_G.ready = true
diff --git a/test/luatest_helpers.lua b/test/luatest_helpers.lua
new file mode 100644
index 0000000..283906c
--- /dev/null
+++ b/test/luatest_helpers.lua
@@ -0,0 +1,72 @@
+local fun = require('fun')
+local json = require('json')
+local fio = require('fio')
+local log = require('log')
+local yaml = require('yaml')
+local fiber = require('fiber')
+
+local luatest_helpers = {
+    SOCKET_DIR = fio.abspath(os.getenv('VARDIR') or 'test/var')
+}
+
+luatest_helpers.Server = require('test.luatest_helpers.server')
+
+local function default_cfg()
+    return {
+        work_dir = os.getenv('TARANTOOL_WORKDIR'),
+        listen = os.getenv('TARANTOOL_LISTEN'),
+        log = ('%s/%s.log'):format(os.getenv('TARANTOOL_WORKDIR'), os.getenv('TARANTOOL_ALIAS')),
+    }
+end
+
+local function env_cfg()
+    local src = os.getenv('TARANTOOL_BOX_CFG')
+    if src == nil then
+        return {}
+    end
+    local res = json.decode(src)
+    assert(type(res) == 'table')
+    return res
+end
+
+-- Collect box.cfg table from values passed through
+-- luatest_helpers.Server({<...>}) and from the given argument.
+--
+-- Use it from inside an instance script.
+function luatest_helpers.box_cfg(cfg)
+    return fun.chain(default_cfg(), env_cfg(), cfg or {}):tomap()
+end
+
+function luatest_helpers.instance_uri(alias, instance_id)
+    if instance_id == nil then
+        instance_id = ''
+    end
+    instance_id = tostring(instance_id)
+    return ('%s/%s%s.iproto'):format(luatest_helpers.SOCKET_DIR, alias, instance_id);
+end
+
+function luatest_helpers:get_vclock(server)
+    return server:eval('return box.info.vclock')
+end
+
+function luatest_helpers:wait_vclock(server, to_vclock)
+    while true do
+        local vclock = self:get_vclock(server)
+        local ok = true
+        for server_id, to_lsn in pairs(to_vclock) do
+            local lsn = vclock[server_id]
+            if lsn == nil or lsn < to_lsn then
+                ok = false
+                break
+            end
+        end
+        if ok then
+            return
+        end
+        log.info("wait vclock: %s to %s", yaml.encode(vclock),
+                 yaml.encode(to_vclock))
+        fiber.sleep(0.001)
+    end
+end
+
+return luatest_helpers
diff --git a/test/luatest_helpers/asserts.lua b/test/luatest_helpers/asserts.lua
new file mode 100644
index 0000000..fa015cd
--- /dev/null
+++ b/test/luatest_helpers/asserts.lua
@@ -0,0 +1,43 @@
+local t = require('luatest')
+
+local asserts = {}
+
+function asserts:new(object)
+    self:inherit(object)
+    object:initialize()
+    return object
+end
+
+function asserts:inherit(object)
+    object = object or {}
+    setmetatable(object, self)
+    self.__index = self
+    return object
+end
+
+function asserts:assert_server_follow_upstream(server, id)
+    local status = server:eval(
+        ('return box.info.replication[%d].upstream.status'):format(id))
+    t.assert_equals(status, 'follow',
+        ('%s: this server does not follow others.'):format(server.alias))
+end
+
+
+function asserts:wait_fullmesh(servers, wait_time)
+    wait_time = wait_time or 20
+    t.helpers.retrying({timeout = wait_time}, function()
+        for _, server in pairs(servers) do
+            for _, server2 in pairs(servers) do
+                if server ~= server2 then
+                    local server_id = server:eval('return box.info.id')
+                    local server2_id = server2:eval('return box.info.id')
+                    if server_id ~= server2_id then
+                        self:assert_server_follow_upstream(server, server2_id)
+                    end
+                end
+            end
+        end
+    end)
+end
+
+return asserts
diff --git a/test/luatest_helpers/cluster.lua b/test/luatest_helpers/cluster.lua
new file mode 100644
index 0000000..43e3479
--- /dev/null
+++ b/test/luatest_helpers/cluster.lua
@@ -0,0 +1,132 @@
+local fio = require('fio')
+local Server = require('test.luatest_helpers.server')
+
+local root = os.environ()['SOURCEDIR'] or '.'
+
+local Cluster = {}
+
+function Cluster:new(object)
+    self:inherit(object)
+    object:initialize()
+    self.servers = object.servers
+    self.built_servers = object.built_servers
+    return object
+end
+
+function Cluster:inherit(object)
+    object = object or {}
+    setmetatable(object, self)
+    self.__index = self
+    self.servers = {}
+    self.built_servers = {}
+    return object
+end
+
+function Cluster:initialize()
+    self.servers = {}
+end
+
+function Cluster:server(alias)
+    for _, server in ipairs(self.servers) do
+        if server.alias == alias then
+            return server
+        end
+    end
+    return nil
+end
+
+function Cluster:drop()
+    for _, server in ipairs(self.servers) do
+        if server ~= nil then
+            server:stop()
+            server:cleanup()
+        end
+    end
+end
+
+function Cluster:get_index(server)
+    local index = nil
+    for i, v in ipairs(self.servers) do
+        if (v.id == server) then
+          index = i
+        end
+    end
+    return index
+end
+
+function Cluster:delete_server(server)
+    local idx = self:get_index(server)
+    if idx == nil then
+        print("Key does not exist")
+    else
+        table.remove(self.servers, idx)
+    end
+end
+
+function Cluster:stop()
+    for _, server in ipairs(self.servers) do
+        if server ~= nil then
+            server:stop()
+        end
+    end
+end
+
+function Cluster:start(opts)
+    for _, server in ipairs(self.servers) do
+        if not server.process then
+            server:start({wait_for_readiness = false})
+        end
+    end
+
+    -- The option is true by default.
+    local wait_for_readiness = true
+    if opts ~= nil and opts.wait_for_readiness ~= nil then
+        wait_for_readiness = opts.wait_for_readiness
+    end
+
+    if wait_for_readiness then
+        for _, server in ipairs(self.servers) do
+            server:wait_for_readiness()
+        end
+    end
+end
+
+function Cluster:build_server(server_config, instance_file)
+    instance_file = instance_file or 'default.lua'
+    server_config = table.deepcopy(server_config)
+    server_config.command = fio.pathjoin(root, 'test/instances/', instance_file)
+    assert(server_config.alias, 'Either replicaset.alias or server.alias must be given')
+    local server = Server:new(server_config)
+    table.insert(self.built_servers, server)
+    return server
+end
+
+function Cluster:add_server(server)
+    if self:server(server.alias) ~= nil then
+        error('Alias is not provided')
+    end
+    table.insert(self.servers, server)
+end
+
+function Cluster:build_and_add_server(config, replicaset_config, engine)
+    local server = self:build_server(config, replicaset_config, engine)
+    self:add_server(server)
+    return server
+end
+
+
+function Cluster:get_leader()
+    for _, instance in ipairs(self.servers) do
+        if instance:eval('return box.info.ro') == false then
+            return instance
+        end
+    end
+end
+
+function Cluster:exec_on_leader(bootstrap_function)
+    local leader = self:get_leader()
+    return leader:exec(bootstrap_function)
+end
+
+
+return Cluster
diff --git a/test/luatest_helpers/server.lua b/test/luatest_helpers/server.lua
new file mode 100644
index 0000000..714c537
--- /dev/null
+++ b/test/luatest_helpers/server.lua
@@ -0,0 +1,266 @@
+local clock = require('clock')
+local digest = require('digest')
+local ffi = require('ffi')
+local fiber = require('fiber')
+local fio = require('fio')
+local fun = require('fun')
+local json = require('json')
+local errno = require('errno')
+
+local checks = require('checks')
+local luatest = require('luatest')
+
+ffi.cdef([[
+    int kill(pid_t pid, int sig);
+]])
+
+local Server = luatest.Server:inherit({})
+
+local WAIT_TIMEOUT = 60
+local WAIT_DELAY = 0.1
+
+local DEFAULT_CHECKPOINT_PATTERNS = {"*.snap", "*.xlog", "*.vylog",
+                                     "*.inprogress", "[0-9]*/"}
+
+-- Differences from luatest.Server:
+--
+-- * 'alias' is mandatory.
+-- * 'command' is optional, assumed test/instances/default.lua by
+--   default.
+-- * 'workdir' is optional, determined by 'alias'.
+-- * The new 'box_cfg' parameter.
+-- * engine - provides engine for parameterized tests
+Server.constructor_checks = fun.chain(Server.constructor_checks, {
+    alias = 'string',
+    command = '?string',
+    workdir = '?string',
+    box_cfg = '?table',
+    engine = '?string',
+}):tomap()
+
+function Server:initialize()
+    local vardir = fio.abspath(os.getenv('VARDIR') or 'test/var')
+
+    if self.id == nil then
+        local random = digest.urandom(9)
+        self.id = digest.base64_encode(random, {urlsafe = true})
+    end
+    if self.command == nil then
+        self.command = 'test/instances/default.lua'
+    end
+    if self.workdir == nil then
+        self.workdir = ('%s/%s-%s'):format(vardir, self.alias, self.id)
+        fio.rmtree(self.workdir)
+        fio.mktree(self.workdir)
+    end
+    if self.net_box_port == nil and self.net_box_uri == nil then
+        self.net_box_uri = ('%s/%s.iproto'):format(vardir, self.alias)
+        fio.mktree(vardir)
+    end
+
+    -- AFAIU, the inner getmetatable() returns our helpers.Server
+    -- class, the outer one returns luatest.Server class.
+    getmetatable(getmetatable(self)).initialize(self)
+end
+
+--- Generates environment to run process with.
+-- The result is merged into os.environ().
+-- @return map
+function Server:build_env()
+    local res = getmetatable(getmetatable(self)).build_env(self)
+    if self.box_cfg ~= nil then
+        res.TARANTOOL_BOX_CFG = json.encode(self.box_cfg)
+    end
+    res.TARANTOOL_ENGINE = self.engine
+    return res
+end
+
+local function wait_cond(cond_name, server, func, ...)
+    local alias = server.alias
+    local id = server.id
+    local pid = server.process.pid
+
+    local deadline = clock.time() + WAIT_TIMEOUT
+    while true do
+        if func(...) then
+            return
+        end
+        if clock.time() > deadline then
+            error(('Waiting for "%s" on server %s-%s (PID %d) timed out')
+                  :format(cond_name, alias, id, pid))
+        end
+        fiber.sleep(WAIT_DELAY)
+    end
+end
+
+function Server:wait_for_readiness()
+    return wait_cond('readiness', self, function()
+        local ok, is_ready = pcall(function()
+            self:connect_net_box()
+            return self.net_box:eval('return _G.ready') == true
+        end)
+        return ok and is_ready
+    end)
+end
+
+function Server:wait_election_leader()
+    -- Include read-only property too because if an instance is a leader, it
+    -- does not mean it finished the synchro queue ownership transition. It is
+    -- read-only until that happens. But in tests usually the leader is needed
+    -- as a writable node.
+    return wait_cond('election leader', self, self.exec, self, function()
+        return box.info.election.state == 'leader' and not box.info.ro
+    end)
+end
+
+function Server:wait_election_leader_found()
+    return wait_cond('election leader is found', self, self.exec, self,
+                     function() return box.info.election.leader ~= 0 end)
+end
+
+-- Unlike the original luatest.Server function it waits for
+-- starting the server.
+function Server:start(opts)
+    checks('table', {
+        wait_for_readiness = '?boolean',
+    })
+    getmetatable(getmetatable(self)).start(self)
+
+    -- The option is true by default.
+    local wait_for_readiness = true
+    if opts ~= nil and opts.wait_for_readiness ~= nil then
+        wait_for_readiness = opts.wait_for_readiness
+    end
+
+    if wait_for_readiness then
+        self:wait_for_readiness()
+    end
+end
+
+function Server:instance_id()
+    -- Cache the value when found it first time.
+    if self.instance_id_value then
+        return self.instance_id_value
+    end
+    local id = self:exec(function() return box.info.id end)
+    -- But do not cache 0 - it is an anon instance, its ID might change.
+    if id ~= 0 then
+        self.instance_id_value = id
+    end
+    return id
+end
+
+function Server:instance_uuid()
+    -- Cache the value when found it first time.
+    if self.instance_uuid_value then
+        return self.instance_uuid_value
+    end
+    local uuid = self:exec(function() return box.info.uuid end)
+    self.instance_uuid_value = uuid
+    return uuid
+end
+
+-- TODO: Add the 'wait_for_readiness' parameter for the restart()
+-- method.
+
+-- Unlike the original luatest.Server function it waits until
+-- the server will stop.
+function Server:stop()
+    local alias = self.alias
+    local id = self.id
+    if self.process then
+        local pid = self.process.pid
+        getmetatable(getmetatable(self)).stop(self)
+
+        local deadline = clock.time() + WAIT_TIMEOUT
+        while true do
+            if ffi.C.kill(pid, 0) ~= 0 then
+                break
+            end
+            if clock.time() > deadline then
+                error(('Stopping of server %s-%s (PID %d) was timed out'):format(
+                    alias, id, pid))
+            end
+            fiber.sleep(WAIT_DELAY)
+        end
+    end
+end
+
+function Server:cleanup()
+    for _, pattern in ipairs(DEFAULT_CHECKPOINT_PATTERNS) do
+        fio.rmtree(('%s/%s'):format(self.workdir, pattern))
+    end
+    self.instance_id_value = nil
+    self.instance_uuid_value = nil
+end
+
+function Server:drop()
+    self:stop()
+    self:cleanup()
+end
+
+-- A copy of test_run:grep_log.
+function Server:grep_log(what, bytes, opts)
+    local opts = opts or {}
+    local noreset = opts.noreset or false
+    -- if instance has crashed provide filename to use grep_log
+    local filename = opts.filename or self:eval('return box.cfg.log')
+    local file = fio.open(filename, {'O_RDONLY', 'O_NONBLOCK'})
+
+    local function fail(msg)
+        local err = errno.strerror()
+        if file ~= nil then
+            file:close()
+        end
+        error(string.format("%s: %s: %s", msg, filename, err))
+    end
+
+    if file == nil then
+        fail("Failed to open log file")
+    end
+    io.flush() -- attempt to flush stdout == log fd
+    local filesize = file:seek(0, 'SEEK_END')
+    if filesize == nil then
+        fail("Failed to get log file size")
+    end
+    local bytes = bytes or 65536 -- don't read whole log - it can be huge
+    bytes = bytes > filesize and filesize or bytes
+    if file:seek(-bytes, 'SEEK_END') == nil then
+        fail("Failed to seek log file")
+    end
+    local found, buf
+    repeat -- read file in chunks
+        local s = file:read(2048)
+        if s == nil then
+            fail("Failed to read log file")
+        end
+        local pos = 1
+        repeat -- split read string in lines
+            local endpos = string.find(s, '\n', pos)
+            endpos = endpos and endpos - 1 -- strip terminating \n
+            local line = string.sub(s, pos, endpos)
+            if endpos == nil and s ~= '' then
+                -- line doesn't end with \n or eof, append it to buffer
+                -- to be checked on next iteration
+                buf = buf or {}
+                table.insert(buf, line)
+            else
+                if buf ~= nil then -- prepend line with buffered data
+                    table.insert(buf, line)
+                    line = table.concat(buf)
+                    buf = nil
+                end
+                if string.match(line, "Starting instance") and not noreset then
+                    found = nil -- server was restarted, reset search
+                else
+                    found = string.match(line, what) or found
+                end
+            end
+            pos = endpos and endpos + 2 -- jump to char after \n
+        until pos == nil
+    until s == ''
+    file:close()
+    return found
+end
+
+return Server
diff --git a/test/luatest_helpers/vtest.lua b/test/luatest_helpers/vtest.lua
new file mode 100644
index 0000000..affc008
--- /dev/null
+++ b/test/luatest_helpers/vtest.lua
@@ -0,0 +1,135 @@
+local helpers = require('test.luatest_helpers')
+local cluster = require('test.luatest_helpers.cluster')
+
+local uuid_idx = 1
+
+--
+-- New UUID unique per this process. Generation is not random - for simplicity
+-- and reproducibility.
+--
+local function uuid_next()
+    local last = tostring(uuid_idx)
+    uuid_idx = uuid_idx + 1
+    assert(#last <= 12)
+    return '00000000-0000-0000-0000-'..string.rep('0', 12 - #last)..last
+end
+
+--
+-- Build a valid vshard config by a template. A template does not specify
+-- anything volatile such as URIs, UUIDs - these are installed at runtime.
+--
+local function config_new(templ)
+    local res = table.deepcopy(templ)
+    local sharding = {}
+    res.sharding = sharding
+    for _, replicaset_templ in pairs(templ.sharding) do
+        local replicaset_uuid = uuid_next()
+        local replicas = {}
+        local replicaset = table.deepcopy(replicaset_templ)
+        replicaset.replicas = replicas
+        for replica_name, replica_templ in pairs(replicaset_templ.replicas) do
+            local replica_uuid = uuid_next()
+            local replica = table.deepcopy(replica_templ)
+            replica.name = replica_name
+            replica.uri = 'storage:storage@'..helpers.instance_uri(replica_name)
+            replicas[replica_uuid] = replica
+        end
+        sharding[replicaset_uuid] = replicaset
+    end
+    return res
+end
+
+--
+-- Build new cluster by a given config.
+--
+local function storage_new(g, cfg)
+    if not g.cluster then
+        g.cluster = cluster:new({})
+    end
+    local all_servers = {}
+    local masters = {}
+    local replicas = {}
+    for replicaset_uuid, replicaset in pairs(cfg.sharding) do
+        -- Luatest depends on box.cfg being ready and listening. Need to
+        -- configure it before vshard.storage.cfg().
+        local box_repl = {}
+        for _, replica in pairs(replicaset.replicas) do
+            table.insert(box_repl, replica.uri)
+        end
+        local box_cfg = {
+            replication = box_repl,
+            -- Speed retries up.
+            replication_timeout = 0.1,
+        }
+        for replica_uuid, replica in pairs(replicaset.replicas) do
+            local name = replica.name
+            box_cfg.instance_uuid = replica_uuid
+            box_cfg.replicaset_uuid = replicaset_uuid
+            box_cfg.listen = helpers.instance_uri(replica.name)
+            -- Need to specify read-only explicitly to know how is master.
+            box_cfg.read_only = not replica.master
+            local server = g.cluster:build_server({
+                alias = name,
+                box_cfg = box_cfg,
+            }, 'storage.lua')
+            g[name] = server
+            g.cluster:add_server(server)
+
+            table.insert(all_servers, server)
+            if replica.master then
+                table.insert(masters, server)
+            else
+                table.insert(replicas, server)
+            end
+        end
+    end
+    for _, replica in pairs(all_servers) do
+        replica:start({wait_for_readiness = false})
+    end
+    for _, master in pairs(masters) do
+        master:wait_for_readiness()
+        master:exec(function(cfg)
+            -- Logged in as guest with 'super' access rights. Yet 'super' is not
+            -- enough to grant 'replication' privilege. The simplest way - login
+            -- as admin for that temporary.
+            local user = box.session.user()
+            box.session.su('admin')
+
+            vshard.storage.cfg(cfg, box.info.uuid)
+            box.schema.user.grant('storage', 'super')
+
+            box.session.su(user)
+        end, {cfg})
+    end
+    for _, replica in pairs(replicas) do
+        replica:wait_for_readiness()
+        replica:exec(function(cfg)
+            vshard.storage.cfg(cfg, box.info.uuid)
+        end, {cfg})
+    end
+end
+
+--
+-- Create a new router in the cluster.
+--
+local function router_new(g, name, cfg)
+    if not g.cluster then
+        g.cluster = cluster:new({})
+    end
+    local server = g.cluster:build_server({
+        alias = name,
+    }, 'router.lua')
+    g[name] = server
+    g.cluster:add_server(server)
+    server:start()
+    server:exec(function(cfg)
+        vshard.router.cfg(cfg)
+    end, {cfg})
+    return server
+end
+
+return {
+    config_new = config_new,
+    storage_new = storage_new,
+    router_new = router_new,
+}
diff --git a/test/router-luatest/router_test.lua b/test/router-luatest/router_test.lua
new file mode 100644
index 0000000..621794a
--- /dev/null
+++ b/test/router-luatest/router_test.lua
@@ -0,0 +1,54 @@
+local t = require('luatest')
+local vtest = require('test.luatest_helpers.vtest')
+local wait_timeout = 120
+
+local g = t.group('router')
+local cluster_cfg = vtest.config_new({
+    sharding = {
+        {
+            replicas = {
+                replica_1_a = {
+                    master = true,
+                },
+                replica_1_b = {},
+            },
+        },
+        {
+            replicas = {
+                replica_2_a = {
+                    master = true,
+                },
+                replica_2_b = {},
+            },
+        },
+    },
+    bucket_count = 100
+})
+
+g.before_all(function()
+    vtest.storage_new(g, cluster_cfg)
+
+    t.assert_equals(g.replica_1_a:exec(function()
+        return #vshard.storage.info().alerts
+    end), 0, 'no alerts after boot')
+
+    local router = vtest.router_new(g, 'router', cluster_cfg)
+    g.router = router
+    local res, err = router:exec(function(timeout)
+        return vshard.router.bootstrap({timeout = timeout})
+    end, {wait_timeout})
+    t.assert(res and not err, 'bootstrap buckets')
+end)
+
+g.after_all(function()
+    g.cluster:drop()
+end)
+
+g.test_basic = function(g)
+    local router = g.router
+    local res, err = router:exec(function(timeout)
+        return vshard.router.callrw(1, 'echo', {1}, {timeout = timeout})
+    end, {wait_timeout})
+    t.assert(not err, 'no error')
+    t.assert_equals(res, 1, 'good result')
+end
diff --git a/test/router-luatest/suite.ini b/test/router-luatest/suite.ini
new file mode 100644
index 0000000..ae79147
--- /dev/null
+++ b/test/router-luatest/suite.ini
@@ -0,0 +1,5 @@
+[default]
+core = luatest
+description = Router tests
+is_parallel = True
+release_disabled =
-- 
2.24.3 (Apple Git-128)

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

* Re: [Tarantool-patches] [PATCH vshard 2/4] util: introduce Tarantool's semver parser
  2022-02-09 17:53   ` Oleg Babin via Tarantool-patches
@ 2022-02-10 22:33     ` Vladislav Shpilevoy via Tarantool-patches
  2022-02-11 16:38       ` Oleg Babin via Tarantool-patches
  0 siblings, 1 reply; 20+ messages in thread
From: Vladislav Shpilevoy via Tarantool-patches @ 2022-02-10 22:33 UTC (permalink / raw)
  To: Oleg Babin, tarantool-patches

Thanks for the review!

On 09.02.2022 18:53, Oleg Babin via Tarantool-patches wrote:
> Thanks for your patch.
> 
> 
> Looks good. Probably it's worth to move this code into separate module.
> 
> There are several modules in Tarantool ecosystem that implement its own parsers.

I don't mind if somebody would move it. And heap.lua too. But I don't really
want to do that myself since it requires too much work not related to
programming. Just the necessity to install CI in a new repository is enough
for me to give that up.

>> +
>> +local function version_new(id_major, id_middle, id_minor, rel_type, rel_num,
>> +                           id_commit)
> 
> I think we could validate that at least id_major is not nil.

Yes, we can.

====================
@@ -76,7 +76,8 @@ local version_mt = {
 
 local function version_new(id_major, id_middle, id_minor, rel_type, rel_num,
                            id_commit)
-    -- There is no any validation - the API is not public.
+    -- There is no any proper validation - the API is not public.
+    assert(id_major and id_middle and id_minor)
     return setmetatable({
         id_major = id_major,
         id_middle = id_middle,
====================


New patch:

====================
util: introduce Tarantool's semver parser

Tarantool's version since recently has semver-like format. It
made impossible to check for features existence if they were
introduced not in the first release of one x.x.x triplet.

An alternative would be to test for feature existence by its
usage, but it is not always possible with ease.

This patch improves version parser in vshard to understand new
version names.

It will be used by a future commit about netbox's return_raw
feature adoption to skip its tests for old versions
(<= 2.10.0-beta2).

Needed for #312
---
 test/unit-luatest/suite.ini        |   5 +
 test/unit-luatest/version_test.lua | 179 +++++++++++++++++++++++++++++
 vshard/CMakeLists.txt              |   2 +-
 vshard/router/init.lua             |   2 +-
 vshard/storage/init.lua            |   2 +-
 vshard/util.lua                    |  17 +--
 vshard/version.lua                 | 149 ++++++++++++++++++++++++
 7 files changed, 340 insertions(+), 16 deletions(-)
 create mode 100644 test/unit-luatest/suite.ini
 create mode 100644 test/unit-luatest/version_test.lua
 create mode 100644 vshard/version.lua

diff --git a/test/unit-luatest/suite.ini b/test/unit-luatest/suite.ini
new file mode 100644
index 0000000..303032c
--- /dev/null
+++ b/test/unit-luatest/suite.ini
@@ -0,0 +1,5 @@
+[default]
+core = luatest
+description = Unit tests
+is_parallel = True
+release_disabled =
diff --git a/test/unit-luatest/version_test.lua b/test/unit-luatest/version_test.lua
new file mode 100644
index 0000000..905d36e
--- /dev/null
+++ b/test/unit-luatest/version_test.lua
@@ -0,0 +1,179 @@
+local t = require('luatest')
+local lversion = require('vshard.version')
+
+local g = t.group('version')
+
+g.test_order = function(g)
+    -- Example of a full version: 2.10.0-beta2-86-gc9981a567.
+    local versions = {
+        {
+            str = '1.2.3-alpha',
+            ver = lversion.new(1, 2, 3, 'alpha', 0, 0),
+        },
+        {
+            str = '1.2.3-alpha-30',
+            ver = lversion.new(1, 2, 3, 'alpha', 0, 30),
+        },
+        {
+            str = '1.2.3-alpha-45',
+            ver = lversion.new(1, 2, 3, 'alpha', 0, 45),
+        },
+        {
+            str = '1.2.3-alpha1',
+            ver = lversion.new(1, 2, 3, 'alpha', 1, 0),
+        },
+        {
+            str = '1.2.3-alpha1-45',
+            ver = lversion.new(1, 2, 3, 'alpha', 1, 45),
+        },
+        {
+            str = '1.2.3-alpha2',
+            ver = lversion.new(1, 2, 3, 'alpha', 2, 0),
+        },
+        {
+            str = '1.2.3-alpha2-45',
+            ver = lversion.new(1, 2, 3, 'alpha', 2, 45),
+        },
+        {
+            str = '1.2.3-beta',
+            ver = lversion.new(1, 2, 3, 'beta', 0, 0),
+        },
+        {
+            str = '1.2.3-beta-45',
+            ver = lversion.new(1, 2, 3, 'beta', 0, 45),
+        },
+        {
+            str = '1.2.3-beta1',
+            ver = lversion.new(1, 2, 3, 'beta', 1, 0),
+        },
+        {
+            str = '1.2.3-beta1-45',
+            ver = lversion.new(1, 2, 3, 'beta', 1, 45),
+        },
+        {
+            str = '1.2.3-beta2',
+            ver = lversion.new(1, 2, 3, 'beta', 2, 0),
+        },
+        {
+            str = '1.2.3-beta2-45',
+            ver = lversion.new(1, 2, 3, 'beta', 2, 45),
+        },
+        {
+            str = '1.2.3-rc',
+            ver = lversion.new(1, 2, 3, 'rc', 0, 0),
+        },
+        {
+            str = '1.2.3-rc-45',
+            ver = lversion.new(1, 2, 3, 'rc', 0, 45),
+        },
+        {
+            str = '1.2.3-rc1',
+            ver = lversion.new(1, 2, 3, 'rc', 1, 0),
+        },
+        {
+            str = '1.2.3-rc1-45',
+            ver = lversion.new(1, 2, 3, 'rc', 1, 45),
+        },
+        {
+            str = '1.2.3-rc2',
+            ver = lversion.new(1, 2, 3, 'rc', 2, 0),
+        },
+        {
+            str = '1.2.3-rc2-45',
+            ver = lversion.new(1, 2, 3, 'rc', 2, 45),
+        },
+        {
+            str = '1.2.3-rc3',
+            ver = lversion.new(1, 2, 3, 'rc', 3, 0),
+        },
+        {
+            str = '1.2.3-rc4',
+            ver = lversion.new(1, 2, 3, 'rc', 4, 0),
+        },
+        {
+            str = '1.2.3',
+            ver = lversion.new(1, 2, 3, nil, 0, 0),
+        },
+        {
+            str = '1.2.4',
+            ver = lversion.new(1, 2, 4, nil, 0, 0),
+        },
+        {
+            str = '1.2.5-alpha',
+            ver = lversion.new(1, 2, 5, 'alpha', 0, 0),
+        },
+        {
+            str = '1.2.5-alpha1-45-gc9981a567',
+            ver = lversion.new(1, 2, 5, 'alpha', 1, 45),
+        },
+        {
+            str = '1.2.6-',
+            ver = lversion.new(1, 2, 6, nil, 0, 0),
+        },
+        {
+            str = '1.2.7-alpha-',
+            ver = lversion.new(1, 2, 7, 'alpha', 0, 0),
+        },
+        {
+            str = '1.2.7-alpha1-',
+            ver = lversion.new(1, 2, 7, 'alpha', 1, 0),
+        },
+        {
+            str = '1.2.7-alpha1-45',
+            ver = lversion.new(1, 2, 7, 'alpha', 1, 45),
+        },
+        {
+            str = '1.2.7-alpha1-46-',
+            ver = lversion.new(1, 2, 7, 'alpha', 1, 46),
+        },
+        {
+            str = '1.2.8-alpha',
+            ver = lversion.new(1, 2, 8, 'alpha', 0, 0),
+        },
+        {
+            str = '1.2.8-beta',
+            ver = lversion.new(1, 2, 8, 'beta', 0, 0),
+        },
+        {
+            str = '1.2.8-rc',
+            ver = lversion.new(1, 2, 8, 'rc', 0, 0),
+        },
+        {
+            str = '1.2.9',
+            ver = lversion.new(1, 2, 9, nil, 0, 0),
+        },
+    }
+    for i, v in pairs(versions) do
+        local ver = lversion.parse(v.str)
+        t.assert(ver == v.ver, ('versions ==, %d'):format(i))
+        t.assert(not (ver ~= v.ver), ('versions not ~=, %d'):format(i))
+        t.assert(not (ver < v.ver), ('versions not <, %d'):format(i))
+        t.assert(not (ver > v.ver), ('versions not >, %d'):format(i))
+        t.assert(ver <= v.ver, ('versions <=, %d'):format(i))
+        t.assert(ver >= v.ver, ('versions <=, %d'):format(i))
+        if i > 1 then
+            local prev = versions[i - 1].ver
+            t.assert(prev < ver, ('versions <, %d'):format(i))
+            t.assert(prev <= ver, ('versions <=, %d'):format(i))
+            t.assert(not (prev > ver), ('versions not >, %d'):format(i))
+            t.assert(not (prev >= ver), ('versions not >=, %d'):format(i))
+
+            t.assert(not (ver < prev), ('versions not <, %d'):format(i))
+            t.assert(not (ver <= prev), ('versions not <=, %d'):format(i))
+            t.assert(ver > prev, ('versions >, %d'):format(i))
+            t.assert(ver >= prev, ('versions >=, %d'):format(i))
+
+            t.assert(ver ~= prev, ('versions ~=, %d'):format(i))
+            t.assert(not (ver == prev), ('versions not ==, %d'):format(i))
+        end
+    end
+end
+
+g.test_error = function(g)
+    t.assert_error_msg_contains('Could not parse version', lversion.parse,
+                                'bad version')
+    t.assert_error_msg_contains('Could not parse version', lversion.parse,
+                                '1.x.x')
+    t.assert_error_msg_contains('Could not parse version', lversion.parse,
+                                '1.2.x')
+end
diff --git a/vshard/CMakeLists.txt b/vshard/CMakeLists.txt
index 2d9be25..29500f6 100644
--- a/vshard/CMakeLists.txt
+++ b/vshard/CMakeLists.txt
@@ -7,5 +7,5 @@ add_subdirectory(router)
 
 # Install module
 install(FILES cfg.lua error.lua consts.lua hash.lua init.lua replicaset.lua
-        util.lua rlist.lua heap.lua registry.lua
+        util.lua rlist.lua heap.lua registry.lua version.lua
         DESTINATION ${TARANTOOL_INSTALL_LUADIR}/vshard)
diff --git a/vshard/router/init.lua b/vshard/router/init.lua
index 064bd5a..44ed801 100644
--- a/vshard/router/init.lua
+++ b/vshard/router/init.lua
@@ -7,7 +7,7 @@ local MODULE_INTERNALS = '__module_vshard_router'
 -- Reload requirements, in case this module is reloaded manually.
 if rawget(_G, MODULE_INTERNALS) then
     local vshard_modules = {
-        'vshard.consts', 'vshard.error', 'vshard.cfg',
+        'vshard.consts', 'vshard.error', 'vshard.cfg', 'vshard.version',
         'vshard.hash', 'vshard.replicaset', 'vshard.util'
     }
     for _, module in pairs(vshard_modules) do
diff --git a/vshard/storage/init.lua b/vshard/storage/init.lua
index 1642609..6820ad0 100644
--- a/vshard/storage/init.lua
+++ b/vshard/storage/init.lua
@@ -13,7 +13,7 @@ local MODULE_INTERNALS = '__module_vshard_storage'
 -- Reload requirements, in case this module is reloaded manually.
 if rawget(_G, MODULE_INTERNALS) then
     local vshard_modules = {
-        'vshard.consts', 'vshard.error', 'vshard.cfg',
+        'vshard.consts', 'vshard.error', 'vshard.cfg', 'vshard.version',
         'vshard.replicaset', 'vshard.util',
         'vshard.storage.reload_evolution', 'vshard.rlist', 'vshard.registry',
         'vshard.heap', 'vshard.storage.ref', 'vshard.storage.sched',
diff --git a/vshard/util.lua b/vshard/util.lua
index 366fdea..9e0212a 100644
--- a/vshard/util.lua
+++ b/vshard/util.lua
@@ -2,6 +2,7 @@
 local log = require('log')
 local fiber = require('fiber')
 local lerror = require('vshard.error')
+local lversion = require('vshard.version')
 
 local MODULE_INTERNALS = '__module_vshard_util'
 local M = rawget(_G, MODULE_INTERNALS)
@@ -16,13 +17,7 @@ if not M then
     rawset(_G, MODULE_INTERNALS, M)
 end
 
-local version_str = _TARANTOOL:sub(1, _TARANTOOL:find('-')-1)
-local dot = version_str:find('%.')
-local major = tonumber(version_str:sub(1, dot - 1))
-version_str = version_str:sub(dot + 1)
-dot = version_str:find('%.')
-local middle = tonumber(version_str:sub(1, dot - 1))
-local minor = tonumber(version_str:sub(dot + 1))
+local tnt_version = lversion.parse(_TARANTOOL)
 
 --
 -- Extract parts of a tuple.
@@ -146,12 +141,8 @@ end
 --
 -- Check if Tarantool version is >= that a specified one.
 --
-local function version_is_at_least(major_need, middle_need, minor_need)
-    if major > major_need then return true end
-    if major < major_need then return false end
-    if middle > middle_need then return true end
-    if middle < middle_need then return false end
-    return minor >= minor_need
+local function version_is_at_least(...)
+    return tnt_version >= lversion.new(...)
 end
 
 if not version_is_at_least(1, 10, 1) then
diff --git a/vshard/version.lua b/vshard/version.lua
new file mode 100644
index 0000000..a53eb60
--- /dev/null
+++ b/vshard/version.lua
@@ -0,0 +1,149 @@
+--
+-- Semver parser adopted to Tarantool's versions.
+-- Almost everything is the same as in https://semver.org.
+--
+-- Tarantool's version has format:
+--
+--     x.x.x-typen-commit-ghash
+--
+-- * x.x.x - major, middle, minor release numbers;
+-- * typen - release type and its optional number: alpha1, beta5, rc10.
+--   Optional;
+-- * commit - commit count since the latest release. Optional;
+-- * ghash - latest commit hash in format g<hash>. Optional.
+--
+-- Differences with the semver docs:
+--
+-- * No support for nested releases like x.x.x-alpha.beta. Only x.x.x-alpha.
+-- * Release number is written right after its type. Not 'alpha.1' but 'alpha1'.
+--
+
+local release_type_weight = {
+    alpha = 0,
+    beta = 1,
+    rc = 2,
+}
+
+local function release_type_cmp(t1, t2)
+    t1 = release_type_weight[t1]
+    t2 = release_type_weight[t2]
+    -- 'No release type' means the greatest.
+    if not t1 then
+        if not t2 then
+            return 0
+        end
+        return 1
+    end
+    if not t2 then
+        return -1
+    end
+    return t1 - t2
+end
+
+local function version_cmp(ver1, ver2)
+    if ver1.id_major ~= ver2.id_major then
+        return ver1.id_major - ver2.id_major
+    end
+    if ver1.id_middle ~= ver2.id_middle then
+        return ver1.id_middle - ver2.id_middle
+    end
+    if ver1.id_minor ~= ver2.id_minor then
+        return ver1.id_minor - ver2.id_minor
+    end
+    if ver1.rel_type ~= ver2.rel_type then
+        return release_type_cmp(ver1.rel_type, ver2.rel_type)
+    end
+    if ver1.rel_num ~= ver2.rel_num then
+        return ver1.rel_num - ver2.rel_num
+    end
+    if ver1.id_commit ~= ver2.id_commit then
+        return ver1.id_commit - ver2.id_commit
+    end
+    return 0
+end
+
+local version_mt = {
+    __eq = function(l, r)
+        return version_cmp(l, r) == 0
+    end,
+    __lt = function(l, r)
+        return version_cmp(l, r) < 0
+    end,
+    __le = function(l, r)
+        return version_cmp(l, r) <= 0
+    end,
+}
+
+local function version_new(id_major, id_middle, id_minor, rel_type, rel_num,
+                           id_commit)
+    -- There is no any proper validation - the API is not public.
+    assert(id_major and id_middle and id_minor)
+    return setmetatable({
+        id_major = id_major,
+        id_middle = id_middle,
+        id_minor = id_minor,
+        rel_type = rel_type,
+        rel_num = rel_num,
+        id_commit = id_commit,
+    }, version_mt)
+end
+
+local function version_parse(version_str)
+    --  x.x.x-name<num>-<num>-g<commit>
+    -- \____/\___/\___/\_____/
+    --   P1   P2   P3    P4
+    local id_major, id_middle, id_minor
+    local rel_type
+    local rel_num = 0
+    local id_commit = 0
+    local pos
+
+    -- Part 1 - version ID triplet.
+    id_major, id_middle, id_minor = version_str:match('^(%d+)%.(%d+)%.(%d+)')
+    if not id_major or not id_middle or not id_minor then
+        error(('Could not parse version: %s'):format(version_str))
+    end
+    id_major = tonumber(id_major)
+    id_middle = tonumber(id_middle)
+    id_minor = tonumber(id_minor)
+
+    -- Cut to 'name<num>-<num>-g<commit>'.
+    pos = version_str:find('-')
+    if not pos then
+        goto finish
+    end
+    version_str = version_str:sub(pos + 1)
+
+    -- Part 2 and 3 - release name, might be absent.
+    rel_type, rel_num = version_str:match('^(%a+)(%d+)')
+    if not rel_type then
+        rel_type = version_str:match('^(%a+)')
+        rel_num = 0
+    else
+        rel_num = tonumber(rel_num)
+    end
+
+    -- Cut to '<num>-g<commit>'.
+    pos = version_str:find('-')
+    if not pos then
+        goto finish
+    end
+    version_str = version_str:sub(pos + 1)
+
+    -- Part 4 - commit count since latest release, might be absent.
+    id_commit = version_str:match('^(%d+)')
+    if not id_commit then
+        id_commit = 0
+    else
+        id_commit = tonumber(id_commit)
+    end
+
+::finish::
+    return version_new(id_major, id_middle, id_minor, rel_type, rel_num,
+                       id_commit)
+end
+
+return {
+    parse = version_parse,
+    new = version_new,
+}
-- 
2.24.3 (Apple Git-128)

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

* Re: [Tarantool-patches] [PATCH vshard 3/4] router: support msgpack object args
  2022-02-09 17:53   ` Oleg Babin via Tarantool-patches
@ 2022-02-10 22:33     ` Vladislav Shpilevoy via Tarantool-patches
  2022-02-11 16:38       ` Oleg Babin via Tarantool-patches
  0 siblings, 1 reply; 20+ messages in thread
From: Vladislav Shpilevoy via Tarantool-patches @ 2022-02-10 22:33 UTC (permalink / raw)
  To: Oleg Babin, 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)

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

* Re: [Tarantool-patches] [PATCH vshard 4/4] router: support netbox return_raw
  2022-02-09 17:53   ` Oleg Babin via Tarantool-patches
@ 2022-02-10 22:34     ` Vladislav Shpilevoy via Tarantool-patches
  2022-02-11 16:38       ` Oleg Babin via Tarantool-patches
  0 siblings, 1 reply; 20+ messages in thread
From: Vladislav Shpilevoy via Tarantool-patches @ 2022-02-10 22:34 UTC (permalink / raw)
  To: Oleg Babin, tarantool-patches

Thanks for the review!

>> diff --git a/test/router-luatest/router_test.lua b/test/router-luatest/router_test.lua
>> index 13ab74d..23929dc 100644
>> --- a/test/router-luatest/router_test.lua
>> +++ b/test/router-luatest/router_test.lua

<...>

>> +    --
>> +    -- Route error.
>> +    --
>> +    res = router:exec(function(timeout, mode)
>> +        local route = vshard.router.route(1)
>> +        return add_details(route[mode](route, 'box_error', {1, 2, 3},
>> +                           {timeout = timeout}))
>> +    end, {wait_timeout, mode})
>> +    t.assert(not res.val, 'no value')
>> +    t.assert_equals(res.err_type, 'table', 'error type')
>> +    t.assert_covers(res.err, {type = 'ClientError', code = box.error.PROC_LUA},
>> +                    'error value')
>> +end
>> +
>> +g.test_return_raw = function(g)
> 
> g.skip_if could be used

Done during interactive rebase, diff wasn't saved.

> Also probably you could use parametrization (however I won't insist):
> 
> ```
> 
> local pg = t.group('pgroup', {{mode = 'callrw'}, {engine = 'callro'}})
> pg.test = function(cg)
>    ...
> end
> 
> ```

I tried, but then realized it will affect all the tests in the group.
If I add in the future more tests which won't need both callro and callrw,
they will run 2 times without need.

If I make 2 groups - one with 2 modes and other without a config, then
AFAIU I will need to recreate the cluster when the first group ends and the
second one starts. I am trying to avoid that because it takes too a lot of
time (compared to running the test cases).

>> diff --git a/vshard/router/init.lua b/vshard/router/init.lua
>> index 44ed801..d39f489 100644
>> --- a/vshard/router/init.lua
>> +++ b/vshard/router/init.lua
>> @@ -1,7 +1,10 @@
>>   local log = require('log')
>>   local lfiber = require('fiber')
>> +local lmsgpack = require('msgpack')
>>   local table_new = require('table.new')
>>   local fiber_clock = lfiber.clock
>> +local msgpack_is_object = lmsgpack.is_object
>> +local msgpack_object = lmsgpack.object
> 
> I suggest to add something like
> 
> ```
> 
> if msgpack_is_object == nil then
> 
>     msgpack_is_object = function() error("Msgpack object feature is not supported by current Tarantool version") end
> 
> end
> 
> ```

Looks good to me:

====================
@@ -3,8 +3,6 @@ local lfiber = require('fiber')
 local lmsgpack = require('msgpack')
 local table_new = require('table.new')
 local fiber_clock = lfiber.clock
-local msgpack_is_object = lmsgpack.is_object
-local msgpack_object = lmsgpack.object
 
 local MODULE_INTERNALS = '__module_vshard_router'
 -- Reload requirements, in case this module is reloaded manually.
@@ -25,6 +23,20 @@ local lreplicaset = require('vshard.replicaset')
 local util = require('vshard.util')
 local seq_serializer = { __serialize = 'seq' }
 
+local msgpack_is_object = lmsgpack.is_object
+local msgpack_object = lmsgpack.object
+
+if not util.feature.msgpack_object then
+    local msg = 'Msgpack object feature is not supported by current '..
+                'Tarantool version'
+    msgpack_is_object = function()
+        error(msg)
+    end
+    msgpack_object = function()
+        error(msg)
+    end
+end
+
 local M = rawget(_G, MODULE_INTERNALS)
====================


New patch:

====================
router: support netbox return_raw

Netbox return_raw option allows to return remote call result
without decoding, as an msgpack object. It will be introduced
after 2.10.0-beta2.

It can be used with router working as a proxy. Pass arguments to
it in msgpack object and get back another msgpack object by usage
of return_raw option. This way the router won't decode nor encode
user's function arguments at all.

Part of #312
---
 test/instances/storage.lua           |  5 ++
 test/reload_evolution/storage.result |  4 --
 test/router-luatest/router_test.lua  | 84 ++++++++++++++++++++++++++++
 test/storage/storage.result          |  2 -
 vshard/router/init.lua               | 40 ++++++++++++-
 vshard/storage/init.lua              | 15 +++++
 vshard/util.lua                      |  1 +
 7 files changed, 144 insertions(+), 7 deletions(-)

diff --git a/test/instances/storage.lua b/test/instances/storage.lua
index 7ad2af3..2afc942 100755
--- a/test/instances/storage.lua
+++ b/test/instances/storage.lua
@@ -14,10 +14,15 @@ end
 box.cfg(helpers.box_cfg())
 box.schema.user.grant('guest', 'super', nil, nil, {if_not_exists = true})
 
+local function box_error()
+    box.error(box.error.PROC_LUA, 'box_error')
+end
+
 local function echo(...)
     return ...
 end
 
+_G.box_error = box_error
 _G.echo = echo
 
 _G.ready = true
diff --git a/test/reload_evolution/storage.result b/test/reload_evolution/storage.result
index 77010a2..a282bba 100644
--- a/test/reload_evolution/storage.result
+++ b/test/reload_evolution/storage.result
@@ -122,8 +122,6 @@ vshard.storage.call(bucket_id_to_move, 'read', 'do_select', {42})
 ---
 - true
 - - [42, 3000]
-- null
-- null
 ...
 vshard.storage.bucket_send(bucket_id_to_move, util.replicasets[1])
 ---
@@ -155,8 +153,6 @@ vshard.storage.call(bucket_id_to_move, 'read', 'do_select', {42})
 ---
 - true
 - - [42, 3000]
-- null
-- null
 ...
 -- Check info() does not fail.
 vshard.storage.info() ~= nil
diff --git a/test/router-luatest/router_test.lua b/test/router-luatest/router_test.lua
index b7aeea9..39dedcb 100644
--- a/test/router-luatest/router_test.lua
+++ b/test/router-luatest/router_test.lua
@@ -96,3 +96,87 @@ g.test_msgpack_args = function(g)
     t.assert(err == nil, 'no error')
     t.assert_equals(res, 100, 'good result')
 end
+
+local function test_return_raw_template(g, mode)
+    --
+    -- Normal call.
+    --
+    local router = g.router
+    local res = router:exec(function(timeout, mode)
+        return add_details(vshard.router[mode](1, 'echo', {1, 2, 3},
+                           {timeout = timeout, return_raw = true}))
+    end, {wait_timeout, mode})
+    t.assert_equals(res.val, {1, 2, 3}, 'value value')
+    t.assert_equals(res.val_type, 'userdata', 'value type')
+    t.assert(not res.err, 'no error')
+
+    --
+    -- Route call.
+    --
+    res = router:exec(function(timeout, mode)
+        local route = vshard.router.route(1)
+        return add_details(route[mode](route, 'echo', {1, 2, 3},
+                           {timeout = timeout, return_raw = true}))
+    end, {wait_timeout, mode})
+    t.assert_equals(res.val, {1, 2, 3}, 'value value')
+    t.assert_equals(res.val_type, 'userdata', 'value type')
+    t.assert(not res.err, 'no error')
+
+    --
+    -- Empty result set.
+    --
+    res = router:exec(function(timeout, mode)
+        return add_details(vshard.router[mode](1, 'echo', {},
+                           {timeout = timeout, return_raw = true}))
+    end, {wait_timeout, mode})
+    t.assert(not res.val, 'no value')
+    t.assert(not res.err, 'no error')
+
+    --
+    -- Error.
+    --
+    res = router:exec(function(timeout, mode)
+        return add_details(vshard.router[mode](1, 'box_error', {1, 2, 3},
+                           {timeout = timeout}))
+    end, {wait_timeout, mode})
+    t.assert(not res.val, 'no value')
+    t.assert_equals(res.err_type, 'table', 'error type')
+    t.assert_covers(res.err, {type = 'ClientError', code = box.error.PROC_LUA},
+                    'error value')
+
+    --
+    -- Route error.
+    --
+    res = router:exec(function(timeout, mode)
+        local route = vshard.router.route(1)
+        return add_details(route[mode](route, 'box_error', {1, 2, 3},
+                           {timeout = timeout}))
+    end, {wait_timeout, mode})
+    t.assert(not res.val, 'no value')
+    t.assert_equals(res.err_type, 'table', 'error type')
+    t.assert_covers(res.err, {type = 'ClientError', code = box.error.PROC_LUA},
+                    'error value')
+end
+
+g.test_return_raw = function(g)
+    t.run_only_if(vutil.feature.netbox_return_raw)
+
+    g.router:exec(function()
+        rawset(_G, 'add_details', function(val, err)
+            -- Direct return would turn nils into box.NULLs. The tests want to
+            -- ensure it doesn't happen. Table wrap makes the actual nils
+            -- eliminate themselves.
+            return {
+                val = val,
+                val_type = type(val),
+                err = err,
+                err_type = type(err),
+            }
+        end)
+    end)
+    test_return_raw_template(g, 'callrw')
+    test_return_raw_template(g, 'callro')
+    g.router:exec(function()
+        _G.add_details = nil
+    end)
+end
diff --git a/test/storage/storage.result b/test/storage/storage.result
index dcd1c1f..73c171a 100644
--- a/test/storage/storage.result
+++ b/test/storage/storage.result
@@ -480,8 +480,6 @@ vshard.storage.call(1, 'read', 'space_get', {'test', {1}})
 ---
 - true
 - [1, 1]
-- null
-- null
 ...
 vshard.storage.call(100500, 'read', 'space_get', {'test', {1}})
 ---
diff --git a/vshard/router/init.lua b/vshard/router/init.lua
index 44ed801..8f0f489 100644
--- a/vshard/router/init.lua
+++ b/vshard/router/init.lua
@@ -1,5 +1,6 @@
 local log = require('log')
 local lfiber = require('fiber')
+local lmsgpack = require('msgpack')
 local table_new = require('table.new')
 local fiber_clock = lfiber.clock
 
@@ -22,6 +23,20 @@ local lreplicaset = require('vshard.replicaset')
 local util = require('vshard.util')
 local seq_serializer = { __serialize = 'seq' }
 
+local msgpack_is_object = lmsgpack.is_object
+local msgpack_object = lmsgpack.object
+
+if not util.feature.msgpack_object then
+    local msg = 'Msgpack object feature is not supported by current '..
+                'Tarantool version'
+    msgpack_is_object = function()
+        error(msg)
+    end
+    msgpack_object = function()
+        error(msg)
+    end
+end
+
 local M = rawget(_G, MODULE_INTERNALS)
 if not M then
     M = {
@@ -530,14 +545,17 @@ end
 --
 local function router_call_impl(router, bucket_id, mode, prefer_replica,
                                 balance, func, args, opts)
+    local do_return_raw
     if opts then
         if type(opts) ~= 'table' or
            (opts.timeout and type(opts.timeout) ~= 'number') then
             error('Usage: call(bucket_id, mode, func, args, opts)')
         end
         opts = table.copy(opts)
-    elseif not opts then
+        do_return_raw = opts.return_raw
+    else
         opts = {}
+        do_return_raw = false
     end
     local timeout = opts.timeout or consts.CALL_TIMEOUT_MIN
     local replicaset, err
@@ -569,6 +587,26 @@ local function router_call_impl(router, bucket_id, mode, prefer_replica,
             local storage_call_status, call_status, call_error =
                 replicaset[call](replicaset, 'vshard.storage.call',
                                  {bucket_id, mode, func, args}, opts)
+            if do_return_raw and msgpack_is_object(storage_call_status) then
+                -- Storage.call returns in the first value a flag whether user's
+                -- function threw an exception or not. Need to extract it.
+                -- Unfortunately, it forces to repack the rest of values into a
+                -- new array. But the values themselves are not decoded.
+                local it = storage_call_status:iterator()
+                local count = it:decode_array_header()
+                storage_call_status = it:decode()
+                -- When no values, nil is not packed into msgpack object. Same
+                -- as in raw netbox.
+                if count > 1 then
+                    count = count - 1
+                    local res = table_new(count, 0)
+                    for i = 1, count do
+                        res[i] = it:take()
+                    end
+                    call_status = msgpack_object(res)
+                end
+                call_error = nil
+            end
             if storage_call_status then
                 if call_status == nil and call_error ~= nil then
                     return call_status, call_error
diff --git a/vshard/storage/init.lua b/vshard/storage/init.lua
index 6820ad0..5083911 100644
--- a/vshard/storage/init.lua
+++ b/vshard/storage/init.lua
@@ -2512,6 +2512,21 @@ local function storage_call(bucket_id, mode, name, args)
     if not ok then
         ret1 = lerror.make(ret1)
     end
+    -- Truncate nil values. Can't return them all because empty values turn into
+    -- box.NULL. Even if user's function actually returned just 1 value, this
+    -- would lead to '1, box.NULL, box.NULL' on the client. Not visible on the
+    -- router when called normally, but won't help if the router did
+    -- 'return_raw' and just forwards everything as is without truncation.
+    -- Although this solution truncates really returned nils.
+    if ret3 == nil then
+        if ret2 == nil then
+            if ret1 == nil then
+                return ok
+            end
+            return ok, ret1
+        end
+        return ok, ret1, ret2
+    end
     return ok, ret1, ret2, ret3
 end
 
diff --git a/vshard/util.lua b/vshard/util.lua
index 354727d..c6975cf 100644
--- a/vshard/util.lua
+++ b/vshard/util.lua
@@ -237,6 +237,7 @@ end
 --
 local feature = {
     msgpack_object = lmsgpack.object ~= nil,
+    netbox_return_raw = version_is_at_least(2, 10, 0, 'beta', 2, 86),
 }
 
 return {
-- 
2.24.3 (Apple Git-128)

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

* Re: [Tarantool-patches] [PATCH vshard 1/4] test: support luatest
  2022-02-10 22:32     ` Vladislav Shpilevoy via Tarantool-patches
@ 2022-02-11 16:38       ` Oleg Babin via Tarantool-patches
  0 siblings, 0 replies; 20+ messages in thread
From: Oleg Babin via Tarantool-patches @ 2022-02-11 16:38 UTC (permalink / raw)
  To: Vladislav Shpilevoy, tarantool-patches

Thanks for your changes. LGTM.

On 11.02.2022 01:32, Vladislav Shpilevoy wrote:
> Hi! Thanks for the review!
>
> On 09.02.2022 18:53, Oleg Babin wrote:
>> Thanks for your patch.
>>
>> Am I right that it's partially imported from tarantoolhttps://github.com/tarantool/tarantool/tree/master/test/luatest_helpers  ?
> It is not just partially imported. It is a complete 100% copy-paste
> of everything except vtest.lua. I wrote it under `---` below.

Ops, I missed this sentence.


>>> diff --git a/test/luatest_helpers.lua b/test/luatest_helpers.lua
>>> new file mode 100644
>>> index 0000000..283906c
>>> --- /dev/null
>>> +++ b/test/luatest_helpers.lua
>>> @@ -0,0 +1,72 @@
>>> +local fun = require('fun')
>>> +local json = require('json')
>>> +local fio = require('fio')
>>> +local log = require('log')
>>> +local yaml = require('yaml')
>>> +local fiber = require('fiber')
>>> +
>>> +local luatest_helpers = {
>>> +    SOCKET_DIR = fio.abspath(os.getenv('VARDIR') or 'test/var')
>>> +}
>> Is fio.abspath really needed here? AFAIK the max length of unix socket is 108 symbols. Relative paths give a more chances that we don't face any issues.
> VARDIR is already absolute path set by luatest, it won't help much.
> As for why fio.abspath is used then - I don't know. I would rather treat
> this file as a part of abomination called luatest (which it should have
> been from the beginning instead of being copy-pasted across projects) and
> try not to change anything here. Unless something breaks.
>
>>> diff --git a/test/luatest_helpers/asserts.lua b/test/luatest_helpers/asserts.lua
>>> new file mode 100644
>>> index 0000000..77385d8
>>> --- /dev/null
>>> +++ b/test/luatest_helpers/asserts.lua
>>> @@ -0,0 +1,43 @@
> <...>
>
>>> +
>>> +
>>> +function asserts:wait_fullmesh(servers, wait_time)
>>> +    wait_time = wait_time or 20
>>> +    t.helpers.retrying({timeout = wait_time}, function()
>>> +        for _, server in pairs(servers) do
>>> +            for _, server2 in pairs(servers) do
>>> +                if server ~= server2 then
>>> +                    local server_id = server:eval('return box.info.id')
>>> +                    local server2_id = server2:eval('return box.info.id')
>>> +                    if server_id ~= server2_id then
>>> +                            self:assert_server_follow_upstream(server, server2_id)
>> Indention looks broken here (8 spaces instead of 4).
> Thanks, fixed:
>
> ====================
> @@ -32,7 +32,7 @@ function asserts:wait_fullmesh(servers, wait_time)
>                       local server_id = server:eval('return box.info.id')
>                       local server2_id = server2:eval('return box.info.id')
>                       if server_id ~= server2_id then
> -                            self:assert_server_follow_upstream(server, server2_id)
> +                        self:assert_server_follow_upstream(server, server2_id)
>                       end
>                   end
>               end
> ====================
>
> I also applied this diff to make it run on 1.10:
>
> ====================
> diff --git a/test/instances/router.lua b/test/instances/router.lua
> index ccec6c1..587a473 100755
> --- a/test/instances/router.lua
> +++ b/test/instances/router.lua
> @@ -7,7 +7,9 @@ _G.vshard = {
>   }
>   -- Somewhy shutdown hangs on new Tarantools even though the nodes do not seem to
>   -- have any long requests running.
> -box.ctl.set_on_shutdown_timeout(0.001)
> +if box.ctl.set_on_shutdown_timeout then
> +    box.ctl.set_on_shutdown_timeout(0.001)
> +end
>   
>   box.cfg(helpers.box_cfg())
>   box.schema.user.grant('guest', 'super', nil, nil, {if_not_exists = true})
> diff --git a/test/instances/storage.lua b/test/instances/storage.lua
> index 2d679ba..7ad2af3 100755
> --- a/test/instances/storage.lua
> +++ b/test/instances/storage.lua
> @@ -7,7 +7,9 @@ _G.vshard = {
>   }
>   -- Somewhy shutdown hangs on new Tarantools even though the nodes do not seem to
>   -- have any long requests running.
> -box.ctl.set_on_shutdown_timeout(0.001)
> +if box.ctl.set_on_shutdown_timeout then
> +    box.ctl.set_on_shutdown_timeout(0.001)
> +end
>   
>   box.cfg(helpers.box_cfg())
> ====================
>
>
> New patch:
>
> ====================
> test: support luatest
>
> Test-run's most recent guideline is to write new tests in luatest
> instead of diff console tests when possible. Luatest isn't exactly
> in a perfect condition now, but it has 2 important features which
> would be very useful in vshard:
>
> - Easy cluster build. All can be done programmatically except a
>    few basic things - from config creation to all replicasets
>    start, router start, and buckets bootstrap. No need to hardcode
>    that into files like storage_1_a.lua, router_1.lua, etc.
>
> - Can opt-out certain tests depending on Tarantool version. For
>    instance, soon coming support for netbox's return_raw option
>    will need to run tests only for > 2.10.0-beta2. In diff tests it
>    is also possible but would be notably complicated to achieve.
>
> Needed for #312
> ---
>   test-run                            |   2 +-
>   test/instances/router.lua           |  17 ++
>   test/instances/storage.lua          |  23 +++
>   test/luatest_helpers.lua            |  72 ++++++++
>   test/luatest_helpers/asserts.lua    |  43 +++++
>   test/luatest_helpers/cluster.lua    | 132 ++++++++++++++
>   test/luatest_helpers/server.lua     | 266 ++++++++++++++++++++++++++++
>   test/luatest_helpers/vtest.lua      | 135 ++++++++++++++
>   test/router-luatest/router_test.lua |  54 ++++++
>   test/router-luatest/suite.ini       |   5 +
>   10 files changed, 748 insertions(+), 1 deletion(-)
>   create mode 100755 test/instances/router.lua
>   create mode 100755 test/instances/storage.lua
>   create mode 100644 test/luatest_helpers.lua
>   create mode 100644 test/luatest_helpers/asserts.lua
>   create mode 100644 test/luatest_helpers/cluster.lua
>   create mode 100644 test/luatest_helpers/server.lua
>   create mode 100644 test/luatest_helpers/vtest.lua
>   create mode 100644 test/router-luatest/router_test.lua
>   create mode 100644 test/router-luatest/suite.ini
>
> diff --git a/test-run b/test-run
> index c345003..2604c46 160000
> --- a/test-run
> +++ b/test-run
> @@ -1 +1 @@
> -Subproject commit c34500365efe8316e79c7936a2f2d04644602936
> +Subproject commit 2604c46c7b6368dbde59489d5303ce3d1d430331
> diff --git a/test/instances/router.lua b/test/instances/router.lua
> new file mode 100755
> index 0000000..587a473
> --- /dev/null
> +++ b/test/instances/router.lua
> @@ -0,0 +1,17 @@
> +#!/usr/bin/env tarantool
> +local helpers = require('test.luatest_helpers')
> +-- Do not load entire vshard into the global namespace to catch errors when code
> +-- relies on that.
> +_G.vshard = {
> +    router = require('vshard.router'),
> +}
> +-- Somewhy shutdown hangs on new Tarantools even though the nodes do not seem to
> +-- have any long requests running.
> +if box.ctl.set_on_shutdown_timeout then
> +    box.ctl.set_on_shutdown_timeout(0.001)
> +end
> +
> +box.cfg(helpers.box_cfg())
> +box.schema.user.grant('guest', 'super', nil, nil, {if_not_exists = true})
> +
> +_G.ready = true
> diff --git a/test/instances/storage.lua b/test/instances/storage.lua
> new file mode 100755
> index 0000000..7ad2af3
> --- /dev/null
> +++ b/test/instances/storage.lua
> @@ -0,0 +1,23 @@
> +#!/usr/bin/env tarantool
> +local helpers = require('test.luatest_helpers')
> +-- Do not load entire vshard into the global namespace to catch errors when code
> +-- relies on that.
> +_G.vshard = {
> +    storage = require('vshard.storage'),
> +}
> +-- Somewhy shutdown hangs on new Tarantools even though the nodes do not seem to
> +-- have any long requests running.
> +if box.ctl.set_on_shutdown_timeout then
> +    box.ctl.set_on_shutdown_timeout(0.001)
> +end
> +
> +box.cfg(helpers.box_cfg())
> +box.schema.user.grant('guest', 'super', nil, nil, {if_not_exists = true})
> +
> +local function echo(...)
> +    return ...
> +end
> +
> +_G.echo = echo
> +
> +_G.ready = true
> diff --git a/test/luatest_helpers.lua b/test/luatest_helpers.lua
> new file mode 100644
> index 0000000..283906c
> --- /dev/null
> +++ b/test/luatest_helpers.lua
> @@ -0,0 +1,72 @@
> +local fun = require('fun')
> +local json = require('json')
> +local fio = require('fio')
> +local log = require('log')
> +local yaml = require('yaml')
> +local fiber = require('fiber')
> +
> +local luatest_helpers = {
> +    SOCKET_DIR = fio.abspath(os.getenv('VARDIR') or 'test/var')
> +}
> +
> +luatest_helpers.Server = require('test.luatest_helpers.server')
> +
> +local function default_cfg()
> +    return {
> +        work_dir = os.getenv('TARANTOOL_WORKDIR'),
> +        listen = os.getenv('TARANTOOL_LISTEN'),
> +        log = ('%s/%s.log'):format(os.getenv('TARANTOOL_WORKDIR'), os.getenv('TARANTOOL_ALIAS')),
> +    }
> +end
> +
> +local function env_cfg()
> +    local src = os.getenv('TARANTOOL_BOX_CFG')
> +    if src == nil then
> +        return {}
> +    end
> +    local res = json.decode(src)
> +    assert(type(res) == 'table')
> +    return res
> +end
> +
> +-- Collect box.cfg table from values passed through
> +-- luatest_helpers.Server({<...>}) and from the given argument.
> +--
> +-- Use it from inside an instance script.
> +function luatest_helpers.box_cfg(cfg)
> +    return fun.chain(default_cfg(), env_cfg(), cfg or {}):tomap()
> +end
> +
> +function luatest_helpers.instance_uri(alias, instance_id)
> +    if instance_id == nil then
> +        instance_id = ''
> +    end
> +    instance_id = tostring(instance_id)
> +    return ('%s/%s%s.iproto'):format(luatest_helpers.SOCKET_DIR, alias, instance_id);
> +end
> +
> +function luatest_helpers:get_vclock(server)
> +    return server:eval('return box.info.vclock')
> +end
> +
> +function luatest_helpers:wait_vclock(server, to_vclock)
> +    while true do
> +        local vclock = self:get_vclock(server)
> +        local ok = true
> +        for server_id, to_lsn in pairs(to_vclock) do
> +            local lsn = vclock[server_id]
> +            if lsn == nil or lsn < to_lsn then
> +                ok = false
> +                break
> +            end
> +        end
> +        if ok then
> +            return
> +        end
> +        log.info("wait vclock: %s to %s", yaml.encode(vclock),
> +                 yaml.encode(to_vclock))
> +        fiber.sleep(0.001)
> +    end
> +end
> +
> +return luatest_helpers
> diff --git a/test/luatest_helpers/asserts.lua b/test/luatest_helpers/asserts.lua
> new file mode 100644
> index 0000000..fa015cd
> --- /dev/null
> +++ b/test/luatest_helpers/asserts.lua
> @@ -0,0 +1,43 @@
> +local t = require('luatest')
> +
> +local asserts = {}
> +
> +function asserts:new(object)
> +    self:inherit(object)
> +    object:initialize()
> +    return object
> +end
> +
> +function asserts:inherit(object)
> +    object = object or {}
> +    setmetatable(object, self)
> +    self.__index = self
> +    return object
> +end
> +
> +function asserts:assert_server_follow_upstream(server, id)
> +    local status = server:eval(
> +        ('return box.info.replication[%d].upstream.status'):format(id))
> +    t.assert_equals(status, 'follow',
> +        ('%s: this server does not follow others.'):format(server.alias))
> +end
> +
> +
> +function asserts:wait_fullmesh(servers, wait_time)
> +    wait_time = wait_time or 20
> +    t.helpers.retrying({timeout = wait_time}, function()
> +        for _, server in pairs(servers) do
> +            for _, server2 in pairs(servers) do
> +                if server ~= server2 then
> +                    local server_id = server:eval('return box.info.id')
> +                    local server2_id = server2:eval('return box.info.id')
> +                    if server_id ~= server2_id then
> +                        self:assert_server_follow_upstream(server, server2_id)
> +                    end
> +                end
> +            end
> +        end
> +    end)
> +end
> +
> +return asserts
> diff --git a/test/luatest_helpers/cluster.lua b/test/luatest_helpers/cluster.lua
> new file mode 100644
> index 0000000..43e3479
> --- /dev/null
> +++ b/test/luatest_helpers/cluster.lua
> @@ -0,0 +1,132 @@
> +local fio = require('fio')
> +local Server = require('test.luatest_helpers.server')
> +
> +local root = os.environ()['SOURCEDIR'] or '.'
> +
> +local Cluster = {}
> +
> +function Cluster:new(object)
> +    self:inherit(object)
> +    object:initialize()
> +    self.servers = object.servers
> +    self.built_servers = object.built_servers
> +    return object
> +end
> +
> +function Cluster:inherit(object)
> +    object = object or {}
> +    setmetatable(object, self)
> +    self.__index = self
> +    self.servers = {}
> +    self.built_servers = {}
> +    return object
> +end
> +
> +function Cluster:initialize()
> +    self.servers = {}
> +end
> +
> +function Cluster:server(alias)
> +    for _, server in ipairs(self.servers) do
> +        if server.alias == alias then
> +            return server
> +        end
> +    end
> +    return nil
> +end
> +
> +function Cluster:drop()
> +    for _, server in ipairs(self.servers) do
> +        if server ~= nil then
> +            server:stop()
> +            server:cleanup()
> +        end
> +    end
> +end
> +
> +function Cluster:get_index(server)
> +    local index = nil
> +    for i, v in ipairs(self.servers) do
> +        if (v.id == server) then
> +          index = i
> +        end
> +    end
> +    return index
> +end
> +
> +function Cluster:delete_server(server)
> +    local idx = self:get_index(server)
> +    if idx == nil then
> +        print("Key does not exist")
> +    else
> +        table.remove(self.servers, idx)
> +    end
> +end
> +
> +function Cluster:stop()
> +    for _, server in ipairs(self.servers) do
> +        if server ~= nil then
> +            server:stop()
> +        end
> +    end
> +end
> +
> +function Cluster:start(opts)
> +    for _, server in ipairs(self.servers) do
> +        if not server.process then
> +            server:start({wait_for_readiness = false})
> +        end
> +    end
> +
> +    -- The option is true by default.
> +    local wait_for_readiness = true
> +    if opts ~= nil and opts.wait_for_readiness ~= nil then
> +        wait_for_readiness = opts.wait_for_readiness
> +    end
> +
> +    if wait_for_readiness then
> +        for _, server in ipairs(self.servers) do
> +            server:wait_for_readiness()
> +        end
> +    end
> +end
> +
> +function Cluster:build_server(server_config, instance_file)
> +    instance_file = instance_file or 'default.lua'
> +    server_config = table.deepcopy(server_config)
> +    server_config.command = fio.pathjoin(root, 'test/instances/', instance_file)
> +    assert(server_config.alias, 'Either replicaset.alias or server.alias must be given')
> +    local server = Server:new(server_config)
> +    table.insert(self.built_servers, server)
> +    return server
> +end
> +
> +function Cluster:add_server(server)
> +    if self:server(server.alias) ~= nil then
> +        error('Alias is not provided')
> +    end
> +    table.insert(self.servers, server)
> +end
> +
> +function Cluster:build_and_add_server(config, replicaset_config, engine)
> +    local server = self:build_server(config, replicaset_config, engine)
> +    self:add_server(server)
> +    return server
> +end
> +
> +
> +function Cluster:get_leader()
> +    for _, instance in ipairs(self.servers) do
> +        if instance:eval('return box.info.ro') == false then
> +            return instance
> +        end
> +    end
> +end
> +
> +function Cluster:exec_on_leader(bootstrap_function)
> +    local leader = self:get_leader()
> +    return leader:exec(bootstrap_function)
> +end
> +
> +
> +return Cluster
> diff --git a/test/luatest_helpers/server.lua b/test/luatest_helpers/server.lua
> new file mode 100644
> index 0000000..714c537
> --- /dev/null
> +++ b/test/luatest_helpers/server.lua
> @@ -0,0 +1,266 @@
> +local clock = require('clock')
> +local digest = require('digest')
> +local ffi = require('ffi')
> +local fiber = require('fiber')
> +local fio = require('fio')
> +local fun = require('fun')
> +local json = require('json')
> +local errno = require('errno')
> +
> +local checks = require('checks')
> +local luatest = require('luatest')
> +
> +ffi.cdef([[
> +    int kill(pid_t pid, int sig);
> +]])
> +
> +local Server = luatest.Server:inherit({})
> +
> +local WAIT_TIMEOUT = 60
> +local WAIT_DELAY = 0.1
> +
> +local DEFAULT_CHECKPOINT_PATTERNS = {"*.snap", "*.xlog", "*.vylog",
> +                                     "*.inprogress", "[0-9]*/"}
> +
> +-- Differences from luatest.Server:
> +--
> +-- * 'alias' is mandatory.
> +-- * 'command' is optional, assumed test/instances/default.lua by
> +--   default.
> +-- * 'workdir' is optional, determined by 'alias'.
> +-- * The new 'box_cfg' parameter.
> +-- * engine - provides engine for parameterized tests
> +Server.constructor_checks = fun.chain(Server.constructor_checks, {
> +    alias = 'string',
> +    command = '?string',
> +    workdir = '?string',
> +    box_cfg = '?table',
> +    engine = '?string',
> +}):tomap()
> +
> +function Server:initialize()
> +    local vardir = fio.abspath(os.getenv('VARDIR') or 'test/var')
> +
> +    if self.id == nil then
> +        local random = digest.urandom(9)
> +        self.id = digest.base64_encode(random, {urlsafe = true})
> +    end
> +    if self.command == nil then
> +        self.command = 'test/instances/default.lua'
> +    end
> +    if self.workdir == nil then
> +        self.workdir = ('%s/%s-%s'):format(vardir, self.alias, self.id)
> +        fio.rmtree(self.workdir)
> +        fio.mktree(self.workdir)
> +    end
> +    if self.net_box_port == nil and self.net_box_uri == nil then
> +        self.net_box_uri = ('%s/%s.iproto'):format(vardir, self.alias)
> +        fio.mktree(vardir)
> +    end
> +
> +    -- AFAIU, the inner getmetatable() returns our helpers.Server
> +    -- class, the outer one returns luatest.Server class.
> +    getmetatable(getmetatable(self)).initialize(self)
> +end
> +
> +--- Generates environment to run process with.
> +-- The result is merged into os.environ().
> +-- @return map
> +function Server:build_env()
> +    local res = getmetatable(getmetatable(self)).build_env(self)
> +    if self.box_cfg ~= nil then
> +        res.TARANTOOL_BOX_CFG = json.encode(self.box_cfg)
> +    end
> +    res.TARANTOOL_ENGINE = self.engine
> +    return res
> +end
> +
> +local function wait_cond(cond_name, server, func, ...)
> +    local alias = server.alias
> +    local id = server.id
> +    local pid = server.process.pid
> +
> +    local deadline = clock.time() + WAIT_TIMEOUT
> +    while true do
> +        if func(...) then
> +            return
> +        end
> +        if clock.time() > deadline then
> +            error(('Waiting for "%s" on server %s-%s (PID %d) timed out')
> +                  :format(cond_name, alias, id, pid))
> +        end
> +        fiber.sleep(WAIT_DELAY)
> +    end
> +end
> +
> +function Server:wait_for_readiness()
> +    return wait_cond('readiness', self, function()
> +        local ok, is_ready = pcall(function()
> +            self:connect_net_box()
> +            return self.net_box:eval('return _G.ready') == true
> +        end)
> +        return ok and is_ready
> +    end)
> +end
> +
> +function Server:wait_election_leader()
> +    -- Include read-only property too because if an instance is a leader, it
> +    -- does not mean it finished the synchro queue ownership transition. It is
> +    -- read-only until that happens. But in tests usually the leader is needed
> +    -- as a writable node.
> +    return wait_cond('election leader', self, self.exec, self, function()
> +        return box.info.election.state == 'leader' and not box.info.ro
> +    end)
> +end
> +
> +function Server:wait_election_leader_found()
> +    return wait_cond('election leader is found', self, self.exec, self,
> +                     function() return box.info.election.leader ~= 0 end)
> +end
> +
> +-- Unlike the original luatest.Server function it waits for
> +-- starting the server.
> +function Server:start(opts)
> +    checks('table', {
> +        wait_for_readiness = '?boolean',
> +    })
> +    getmetatable(getmetatable(self)).start(self)
> +
> +    -- The option is true by default.
> +    local wait_for_readiness = true
> +    if opts ~= nil and opts.wait_for_readiness ~= nil then
> +        wait_for_readiness = opts.wait_for_readiness
> +    end
> +
> +    if wait_for_readiness then
> +        self:wait_for_readiness()
> +    end
> +end
> +
> +function Server:instance_id()
> +    -- Cache the value when found it first time.
> +    if self.instance_id_value then
> +        return self.instance_id_value
> +    end
> +    local id = self:exec(function() return box.info.id end)
> +    -- But do not cache 0 - it is an anon instance, its ID might change.
> +    if id ~= 0 then
> +        self.instance_id_value = id
> +    end
> +    return id
> +end
> +
> +function Server:instance_uuid()
> +    -- Cache the value when found it first time.
> +    if self.instance_uuid_value then
> +        return self.instance_uuid_value
> +    end
> +    local uuid = self:exec(function() return box.info.uuid end)
> +    self.instance_uuid_value = uuid
> +    return uuid
> +end
> +
> +-- TODO: Add the 'wait_for_readiness' parameter for the restart()
> +-- method.
> +
> +-- Unlike the original luatest.Server function it waits until
> +-- the server will stop.
> +function Server:stop()
> +    local alias = self.alias
> +    local id = self.id
> +    if self.process then
> +        local pid = self.process.pid
> +        getmetatable(getmetatable(self)).stop(self)
> +
> +        local deadline = clock.time() + WAIT_TIMEOUT
> +        while true do
> +            if ffi.C.kill(pid, 0) ~= 0 then
> +                break
> +            end
> +            if clock.time() > deadline then
> +                error(('Stopping of server %s-%s (PID %d) was timed out'):format(
> +                    alias, id, pid))
> +            end
> +            fiber.sleep(WAIT_DELAY)
> +        end
> +    end
> +end
> +
> +function Server:cleanup()
> +    for _, pattern in ipairs(DEFAULT_CHECKPOINT_PATTERNS) do
> +        fio.rmtree(('%s/%s'):format(self.workdir, pattern))
> +    end
> +    self.instance_id_value = nil
> +    self.instance_uuid_value = nil
> +end
> +
> +function Server:drop()
> +    self:stop()
> +    self:cleanup()
> +end
> +
> +-- A copy of test_run:grep_log.
> +function Server:grep_log(what, bytes, opts)
> +    local opts = opts or {}
> +    local noreset = opts.noreset or false
> +    -- if instance has crashed provide filename to use grep_log
> +    local filename = opts.filename or self:eval('return box.cfg.log')
> +    local file = fio.open(filename, {'O_RDONLY', 'O_NONBLOCK'})
> +
> +    local function fail(msg)
> +        local err = errno.strerror()
> +        if file ~= nil then
> +file:close()
> +        end
> +        error(string.format("%s: %s: %s", msg, filename, err))
> +    end
> +
> +    if file == nil then
> +        fail("Failed to open log file")
> +    end
> +    io.flush() -- attempt to flush stdout == log fd
> +    local filesize =file:seek(0, 'SEEK_END')
> +    if filesize == nil then
> +        fail("Failed to get log file size")
> +    end
> +    local bytes = bytes or 65536 -- don't read whole log - it can be huge
> +    bytes = bytes > filesize and filesize or bytes
> +    iffile:seek(-bytes, 'SEEK_END') == nil then
> +        fail("Failed to seek log file")
> +    end
> +    local found, buf
> +    repeat -- read file in chunks
> +        local s =file:read(2048)
> +        if s == nil then
> +            fail("Failed to read log file")
> +        end
> +        local pos = 1
> +        repeat -- split read string in lines
> +            local endpos = string.find(s, '\n', pos)
> +            endpos = endpos and endpos - 1 -- strip terminating \n
> +            local line = string.sub(s, pos, endpos)
> +            if endpos == nil and s ~= '' then
> +                -- line doesn't end with \n or eof, append it to buffer
> +                -- to be checked on next iteration
> +                buf = buf or {}
> +                table.insert(buf, line)
> +            else
> +                if buf ~= nil then -- prepend line with buffered data
> +                    table.insert(buf, line)
> +                    line = table.concat(buf)
> +                    buf = nil
> +                end
> +                if string.match(line, "Starting instance") and not noreset then
> +                    found = nil -- server was restarted, reset search
> +                else
> +                    found = string.match(line, what) or found
> +                end
> +            end
> +            pos = endpos and endpos + 2 -- jump to char after \n
> +        until pos == nil
> +    until s == ''
> +file:close()
> +    return found
> +end
> +
> +return Server
> diff --git a/test/luatest_helpers/vtest.lua b/test/luatest_helpers/vtest.lua
> new file mode 100644
> index 0000000..affc008
> --- /dev/null
> +++ b/test/luatest_helpers/vtest.lua
> @@ -0,0 +1,135 @@
> +local helpers = require('test.luatest_helpers')
> +local cluster = require('test.luatest_helpers.cluster')
> +
> +local uuid_idx = 1
> +
> +--
> +-- New UUID unique per this process. Generation is not random - for simplicity
> +-- and reproducibility.
> +--
> +local function uuid_next()
> +    local last = tostring(uuid_idx)
> +    uuid_idx = uuid_idx + 1
> +    assert(#last <= 12)
> +    return '00000000-0000-0000-0000-'..string.rep('0', 12 - #last)..last
> +end
> +
> +--
> +-- Build a valid vshard config by a template. A template does not specify
> +-- anything volatile such as URIs, UUIDs - these are installed at runtime.
> +--
> +local function config_new(templ)
> +    local res = table.deepcopy(templ)
> +    local sharding = {}
> +    res.sharding = sharding
> +    for _, replicaset_templ in pairs(templ.sharding) do
> +        local replicaset_uuid = uuid_next()
> +        local replicas = {}
> +        local replicaset = table.deepcopy(replicaset_templ)
> +        replicaset.replicas = replicas
> +        for replica_name, replica_templ in pairs(replicaset_templ.replicas) do
> +            local replica_uuid = uuid_next()
> +            local replica = table.deepcopy(replica_templ)
> +            replica.name = replica_name
> +            replica.uri = 'storage:storage@'..helpers.instance_uri(replica_name)
> +            replicas[replica_uuid] = replica
> +        end
> +        sharding[replicaset_uuid] = replicaset
> +    end
> +    return res
> +end
> +
> +--
> +-- Build new cluster by a given config.
> +--
> +local function storage_new(g, cfg)
> +    if not g.cluster then
> +        g.cluster = cluster:new({})
> +    end
> +    local all_servers = {}
> +    local masters = {}
> +    local replicas = {}
> +    for replicaset_uuid, replicaset in pairs(cfg.sharding) do
> +        -- Luatest depends on box.cfg being ready and listening. Need to
> +        -- configure it before vshard.storage.cfg().
> +        local box_repl = {}
> +        for _, replica in pairs(replicaset.replicas) do
> +            table.insert(box_repl, replica.uri)
> +        end
> +        local box_cfg = {
> +            replication = box_repl,
> +            -- Speed retries up.
> +            replication_timeout = 0.1,
> +        }
> +        for replica_uuid, replica in pairs(replicaset.replicas) do
> +            local name = replica.name
> +            box_cfg.instance_uuid = replica_uuid
> +            box_cfg.replicaset_uuid = replicaset_uuid
> +            box_cfg.listen = helpers.instance_uri(replica.name)
> +            -- Need to specify read-only explicitly to know how is master.
> +            box_cfg.read_only = not replica.master
> +            local server = g.cluster:build_server({
> +                alias = name,
> +                box_cfg = box_cfg,
> +            }, 'storage.lua')
> +            g[name] = server
> +            g.cluster:add_server(server)
> +
> +            table.insert(all_servers, server)
> +            if replica.master then
> +                table.insert(masters, server)
> +            else
> +                table.insert(replicas, server)
> +            end
> +        end
> +    end
> +    for _, replica in pairs(all_servers) do
> +        replica:start({wait_for_readiness = false})
> +    end
> +    for _, master in pairs(masters) do
> +        master:wait_for_readiness()
> +        master:exec(function(cfg)
> +            -- Logged in as guest with 'super' access rights. Yet 'super' is not
> +            -- enough to grant 'replication' privilege. The simplest way - login
> +            -- as admin for that temporary.
> +            local user = box.session.user()
> +            box.session.su('admin')
> +
> +            vshard.storage.cfg(cfg, box.info.uuid)
> +            box.schema.user.grant('storage', 'super')
> +
> +            box.session.su(user)
> +        end, {cfg})
> +    end
> +    for _, replica in pairs(replicas) do
> +        replica:wait_for_readiness()
> +        replica:exec(function(cfg)
> +            vshard.storage.cfg(cfg, box.info.uuid)
> +        end, {cfg})
> +    end
> +end
> +
> +--
> +-- Create a new router in the cluster.
> +--
> +local function router_new(g, name, cfg)
> +    if not g.cluster then
> +        g.cluster = cluster:new({})
> +    end
> +    local server = g.cluster:build_server({
> +        alias = name,
> +    }, 'router.lua')
> +    g[name] = server
> +    g.cluster:add_server(server)
> +    server:start()
> +    server:exec(function(cfg)
> +        vshard.router.cfg(cfg)
> +    end, {cfg})
> +    return server
> +end
> +
> +return {
> +    config_new = config_new,
> +    storage_new = storage_new,
> +    router_new = router_new,
> +}
> diff --git a/test/router-luatest/router_test.lua b/test/router-luatest/router_test.lua
> new file mode 100644
> index 0000000..621794a
> --- /dev/null
> +++ b/test/router-luatest/router_test.lua
> @@ -0,0 +1,54 @@
> +local t = require('luatest')
> +local vtest = require('test.luatest_helpers.vtest')
> +local wait_timeout = 120
> +
> +local g = t.group('router')
> +local cluster_cfg = vtest.config_new({
> +    sharding = {
> +        {
> +            replicas = {
> +                replica_1_a = {
> +                    master = true,
> +                },
> +                replica_1_b = {},
> +            },
> +        },
> +        {
> +            replicas = {
> +                replica_2_a = {
> +                    master = true,
> +                },
> +                replica_2_b = {},
> +            },
> +        },
> +    },
> +    bucket_count = 100
> +})
> +
> +g.before_all(function()
> +    vtest.storage_new(g, cluster_cfg)
> +
> +    t.assert_equals(g.replica_1_a:exec(function()
> +        return #vshard.storage.info().alerts
> +    end), 0, 'no alerts after boot')
> +
> +    local router = vtest.router_new(g, 'router', cluster_cfg)
> +    g.router = router
> +    local res, err = router:exec(function(timeout)
> +        return vshard.router.bootstrap({timeout = timeout})
> +    end, {wait_timeout})
> +    t.assert(res and not err, 'bootstrap buckets')
> +end)
> +
> +g.after_all(function()
> +    g.cluster:drop()
> +end)
> +
> +g.test_basic = function(g)
> +    local router = g.router
> +    local res, err = router:exec(function(timeout)
> +        return vshard.router.callrw(1, 'echo', {1}, {timeout = timeout})
> +    end, {wait_timeout})
> +    t.assert(not err, 'no error')
> +    t.assert_equals(res, 1, 'good result')
> +end
> diff --git a/test/router-luatest/suite.ini b/test/router-luatest/suite.ini
> new file mode 100644
> index 0000000..ae79147
> --- /dev/null
> +++ b/test/router-luatest/suite.ini
> @@ -0,0 +1,5 @@
> +[default]
> +core = luatest
> +description = Router tests
> +is_parallel = True
> +release_disabled =

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

* Re: [Tarantool-patches] [PATCH vshard 2/4] util: introduce Tarantool's semver parser
  2022-02-10 22:33     ` Vladislav Shpilevoy via Tarantool-patches
@ 2022-02-11 16:38       ` Oleg Babin via Tarantool-patches
  0 siblings, 0 replies; 20+ messages in thread
From: Oleg Babin via Tarantool-patches @ 2022-02-11 16:38 UTC (permalink / raw)
  To: Vladislav Shpilevoy, tarantool-patches

Thanks for your answers and changes. LGTM.

On 11.02.2022 01:33, Vladislav Shpilevoy wrote:
> Thanks for the review!
>
> On 09.02.2022 18:53, Oleg Babin via Tarantool-patches wrote:
>> Thanks for your patch.
>>
>>
>> Looks good. Probably it's worth to move this code into separate module.
>>
>> There are several modules in Tarantool ecosystem that implement its own parsers.
> I don't mind if somebody would move it. And heap.lua too. But I don't really
> want to do that myself since it requires too much work not related to
> programming. Just the necessity to install CI in a new repository is enough
> for me to give that up.
>
>>> +
>>> +local function version_new(id_major, id_middle, id_minor, rel_type, rel_num,
>>> +                           id_commit)
>> I think we could validate that at least id_major is not nil.
> Yes, we can.
>
> ====================
> @@ -76,7 +76,8 @@ local version_mt = {
>   
>   local function version_new(id_major, id_middle, id_minor, rel_type, rel_num,
>                              id_commit)
> -    -- There is no any validation - the API is not public.
> +    -- There is no any proper validation - the API is not public.
> +    assert(id_major and id_middle and id_minor)
>       return setmetatable({
>           id_major = id_major,
>           id_middle = id_middle,
> ====================
>
>
> New patch:
>
> ====================
> util: introduce Tarantool's semver parser
>
> Tarantool's version since recently has semver-like format. It
> made impossible to check for features existence if they were
> introduced not in the first release of one x.x.x triplet.
>
> An alternative would be to test for feature existence by its
> usage, but it is not always possible with ease.
>
> This patch improves version parser in vshard to understand new
> version names.
>
> It will be used by a future commit about netbox's return_raw
> feature adoption to skip its tests for old versions
> (<= 2.10.0-beta2).
>
> Needed for #312
> ---
>   test/unit-luatest/suite.ini        |   5 +
>   test/unit-luatest/version_test.lua | 179 +++++++++++++++++++++++++++++
>   vshard/CMakeLists.txt              |   2 +-
>   vshard/router/init.lua             |   2 +-
>   vshard/storage/init.lua            |   2 +-
>   vshard/util.lua                    |  17 +--
>   vshard/version.lua                 | 149 ++++++++++++++++++++++++
>   7 files changed, 340 insertions(+), 16 deletions(-)
>   create mode 100644 test/unit-luatest/suite.ini
>   create mode 100644 test/unit-luatest/version_test.lua
>   create mode 100644 vshard/version.lua
>
> diff --git a/test/unit-luatest/suite.ini b/test/unit-luatest/suite.ini
> new file mode 100644
> index 0000000..303032c
> --- /dev/null
> +++ b/test/unit-luatest/suite.ini
> @@ -0,0 +1,5 @@
> +[default]
> +core = luatest
> +description = Unit tests
> +is_parallel = True
> +release_disabled =
> diff --git a/test/unit-luatest/version_test.lua b/test/unit-luatest/version_test.lua
> new file mode 100644
> index 0000000..905d36e
> --- /dev/null
> +++ b/test/unit-luatest/version_test.lua
> @@ -0,0 +1,179 @@
> +local t = require('luatest')
> +local lversion = require('vshard.version')
> +
> +local g = t.group('version')
> +
> +g.test_order = function(g)
> +    -- Example of a full version: 2.10.0-beta2-86-gc9981a567.
> +    local versions = {
> +        {
> +            str = '1.2.3-alpha',
> +            ver = lversion.new(1, 2, 3, 'alpha', 0, 0),
> +        },
> +        {
> +            str = '1.2.3-alpha-30',
> +            ver = lversion.new(1, 2, 3, 'alpha', 0, 30),
> +        },
> +        {
> +            str = '1.2.3-alpha-45',
> +            ver = lversion.new(1, 2, 3, 'alpha', 0, 45),
> +        },
> +        {
> +            str = '1.2.3-alpha1',
> +            ver = lversion.new(1, 2, 3, 'alpha', 1, 0),
> +        },
> +        {
> +            str = '1.2.3-alpha1-45',
> +            ver = lversion.new(1, 2, 3, 'alpha', 1, 45),
> +        },
> +        {
> +            str = '1.2.3-alpha2',
> +            ver = lversion.new(1, 2, 3, 'alpha', 2, 0),
> +        },
> +        {
> +            str = '1.2.3-alpha2-45',
> +            ver = lversion.new(1, 2, 3, 'alpha', 2, 45),
> +        },
> +        {
> +            str = '1.2.3-beta',
> +            ver = lversion.new(1, 2, 3, 'beta', 0, 0),
> +        },
> +        {
> +            str = '1.2.3-beta-45',
> +            ver = lversion.new(1, 2, 3, 'beta', 0, 45),
> +        },
> +        {
> +            str = '1.2.3-beta1',
> +            ver = lversion.new(1, 2, 3, 'beta', 1, 0),
> +        },
> +        {
> +            str = '1.2.3-beta1-45',
> +            ver = lversion.new(1, 2, 3, 'beta', 1, 45),
> +        },
> +        {
> +            str = '1.2.3-beta2',
> +            ver = lversion.new(1, 2, 3, 'beta', 2, 0),
> +        },
> +        {
> +            str = '1.2.3-beta2-45',
> +            ver = lversion.new(1, 2, 3, 'beta', 2, 45),
> +        },
> +        {
> +            str = '1.2.3-rc',
> +            ver = lversion.new(1, 2, 3, 'rc', 0, 0),
> +        },
> +        {
> +            str = '1.2.3-rc-45',
> +            ver = lversion.new(1, 2, 3, 'rc', 0, 45),
> +        },
> +        {
> +            str = '1.2.3-rc1',
> +            ver = lversion.new(1, 2, 3, 'rc', 1, 0),
> +        },
> +        {
> +            str = '1.2.3-rc1-45',
> +            ver = lversion.new(1, 2, 3, 'rc', 1, 45),
> +        },
> +        {
> +            str = '1.2.3-rc2',
> +            ver = lversion.new(1, 2, 3, 'rc', 2, 0),
> +        },
> +        {
> +            str = '1.2.3-rc2-45',
> +            ver = lversion.new(1, 2, 3, 'rc', 2, 45),
> +        },
> +        {
> +            str = '1.2.3-rc3',
> +            ver = lversion.new(1, 2, 3, 'rc', 3, 0),
> +        },
> +        {
> +            str = '1.2.3-rc4',
> +            ver = lversion.new(1, 2, 3, 'rc', 4, 0),
> +        },
> +        {
> +            str = '1.2.3',
> +            ver = lversion.new(1, 2, 3, nil, 0, 0),
> +        },
> +        {
> +            str = '1.2.4',
> +            ver = lversion.new(1, 2, 4, nil, 0, 0),
> +        },
> +        {
> +            str = '1.2.5-alpha',
> +            ver = lversion.new(1, 2, 5, 'alpha', 0, 0),
> +        },
> +        {
> +            str = '1.2.5-alpha1-45-gc9981a567',
> +            ver = lversion.new(1, 2, 5, 'alpha', 1, 45),
> +        },
> +        {
> +            str = '1.2.6-',
> +            ver = lversion.new(1, 2, 6, nil, 0, 0),
> +        },
> +        {
> +            str = '1.2.7-alpha-',
> +            ver = lversion.new(1, 2, 7, 'alpha', 0, 0),
> +        },
> +        {
> +            str = '1.2.7-alpha1-',
> +            ver = lversion.new(1, 2, 7, 'alpha', 1, 0),
> +        },
> +        {
> +            str = '1.2.7-alpha1-45',
> +            ver = lversion.new(1, 2, 7, 'alpha', 1, 45),
> +        },
> +        {
> +            str = '1.2.7-alpha1-46-',
> +            ver = lversion.new(1, 2, 7, 'alpha', 1, 46),
> +        },
> +        {
> +            str = '1.2.8-alpha',
> +            ver = lversion.new(1, 2, 8, 'alpha', 0, 0),
> +        },
> +        {
> +            str = '1.2.8-beta',
> +            ver = lversion.new(1, 2, 8, 'beta', 0, 0),
> +        },
> +        {
> +            str = '1.2.8-rc',
> +            ver = lversion.new(1, 2, 8, 'rc', 0, 0),
> +        },
> +        {
> +            str = '1.2.9',
> +            ver = lversion.new(1, 2, 9, nil, 0, 0),
> +        },
> +    }
> +    for i, v in pairs(versions) do
> +        local ver = lversion.parse(v.str)
> +        t.assert(ver == v.ver, ('versions ==, %d'):format(i))
> +        t.assert(not (ver ~= v.ver), ('versions not ~=, %d'):format(i))
> +        t.assert(not (ver < v.ver), ('versions not <, %d'):format(i))
> +        t.assert(not (ver > v.ver), ('versions not >, %d'):format(i))
> +        t.assert(ver <= v.ver, ('versions <=, %d'):format(i))
> +        t.assert(ver >= v.ver, ('versions <=, %d'):format(i))
> +        if i > 1 then
> +            local prev = versions[i - 1].ver
> +            t.assert(prev < ver, ('versions <, %d'):format(i))
> +            t.assert(prev <= ver, ('versions <=, %d'):format(i))
> +            t.assert(not (prev > ver), ('versions not >, %d'):format(i))
> +            t.assert(not (prev >= ver), ('versions not >=, %d'):format(i))
> +
> +            t.assert(not (ver < prev), ('versions not <, %d'):format(i))
> +            t.assert(not (ver <= prev), ('versions not <=, %d'):format(i))
> +            t.assert(ver > prev, ('versions >, %d'):format(i))
> +            t.assert(ver >= prev, ('versions >=, %d'):format(i))
> +
> +            t.assert(ver ~= prev, ('versions ~=, %d'):format(i))
> +            t.assert(not (ver == prev), ('versions not ==, %d'):format(i))
> +        end
> +    end
> +end
> +
> +g.test_error = function(g)
> +    t.assert_error_msg_contains('Could not parse version', lversion.parse,
> +                                'bad version')
> +    t.assert_error_msg_contains('Could not parse version', lversion.parse,
> +                                '1.x.x')
> +    t.assert_error_msg_contains('Could not parse version', lversion.parse,
> +                                '1.2.x')
> +end
> diff --git a/vshard/CMakeLists.txt b/vshard/CMakeLists.txt
> index 2d9be25..29500f6 100644
> --- a/vshard/CMakeLists.txt
> +++ b/vshard/CMakeLists.txt
> @@ -7,5 +7,5 @@ add_subdirectory(router)
>   
>   # Install module
>   install(FILES cfg.lua error.lua consts.lua hash.lua init.lua replicaset.lua
> -        util.lua rlist.lua heap.lua registry.lua
> +        util.lua rlist.lua heap.lua registry.lua version.lua
>           DESTINATION ${TARANTOOL_INSTALL_LUADIR}/vshard)
> diff --git a/vshard/router/init.lua b/vshard/router/init.lua
> index 064bd5a..44ed801 100644
> --- a/vshard/router/init.lua
> +++ b/vshard/router/init.lua
> @@ -7,7 +7,7 @@ local MODULE_INTERNALS = '__module_vshard_router'
>   -- Reload requirements, in case this module is reloaded manually.
>   if rawget(_G, MODULE_INTERNALS) then
>       local vshard_modules = {
> -        'vshard.consts', 'vshard.error', 'vshard.cfg',
> +        'vshard.consts', 'vshard.error', 'vshard.cfg', 'vshard.version',
>           'vshard.hash', 'vshard.replicaset', 'vshard.util'
>       }
>       for _, module in pairs(vshard_modules) do
> diff --git a/vshard/storage/init.lua b/vshard/storage/init.lua
> index 1642609..6820ad0 100644
> --- a/vshard/storage/init.lua
> +++ b/vshard/storage/init.lua
> @@ -13,7 +13,7 @@ local MODULE_INTERNALS = '__module_vshard_storage'
>   -- Reload requirements, in case this module is reloaded manually.
>   if rawget(_G, MODULE_INTERNALS) then
>       local vshard_modules = {
> -        'vshard.consts', 'vshard.error', 'vshard.cfg',
> +        'vshard.consts', 'vshard.error', 'vshard.cfg', 'vshard.version',
>           'vshard.replicaset', 'vshard.util',
>           'vshard.storage.reload_evolution', 'vshard.rlist', 'vshard.registry',
>           'vshard.heap', 'vshard.storage.ref', 'vshard.storage.sched',
> diff --git a/vshard/util.lua b/vshard/util.lua
> index 366fdea..9e0212a 100644
> --- a/vshard/util.lua
> +++ b/vshard/util.lua
> @@ -2,6 +2,7 @@
>   local log = require('log')
>   local fiber = require('fiber')
>   local lerror = require('vshard.error')
> +local lversion = require('vshard.version')
>   
>   local MODULE_INTERNALS = '__module_vshard_util'
>   local M = rawget(_G, MODULE_INTERNALS)
> @@ -16,13 +17,7 @@ if not M then
>       rawset(_G, MODULE_INTERNALS, M)
>   end
>   
> -local version_str = _TARANTOOL:sub(1, _TARANTOOL:find('-')-1)
> -local dot = version_str:find('%.')
> -local major = tonumber(version_str:sub(1, dot - 1))
> -version_str = version_str:sub(dot + 1)
> -dot = version_str:find('%.')
> -local middle = tonumber(version_str:sub(1, dot - 1))
> -local minor = tonumber(version_str:sub(dot + 1))
> +local tnt_version = lversion.parse(_TARANTOOL)
>   
>   --
>   -- Extract parts of a tuple.
> @@ -146,12 +141,8 @@ end
>   --
>   -- Check if Tarantool version is >= that a specified one.
>   --
> -local function version_is_at_least(major_need, middle_need, minor_need)
> -    if major > major_need then return true end
> -    if major < major_need then return false end
> -    if middle > middle_need then return true end
> -    if middle < middle_need then return false end
> -    return minor >= minor_need
> +local function version_is_at_least(...)
> +    return tnt_version >= lversion.new(...)
>   end
>   
>   if not version_is_at_least(1, 10, 1) then
> diff --git a/vshard/version.lua b/vshard/version.lua
> new file mode 100644
> index 0000000..a53eb60
> --- /dev/null
> +++ b/vshard/version.lua
> @@ -0,0 +1,149 @@
> +--
> +-- Semver parser adopted to Tarantool's versions.
> +-- Almost everything is the same as inhttps://semver.org.
> +--
> +-- Tarantool's version has format:
> +--
> +--     x.x.x-typen-commit-ghash
> +--
> +-- * x.x.x - major, middle, minor release numbers;
> +-- * typen - release type and its optional number: alpha1, beta5, rc10.
> +--   Optional;
> +-- * commit - commit count since the latest release. Optional;
> +-- * ghash - latest commit hash in format g<hash>. Optional.
> +--
> +-- Differences with the semver docs:
> +--
> +-- * No support for nested releases like x.x.x-alpha.beta. Only x.x.x-alpha.
> +-- * Release number is written right after its type. Not 'alpha.1' but 'alpha1'.
> +--
> +
> +local release_type_weight = {
> +    alpha = 0,
> +    beta = 1,
> +    rc = 2,
> +}
> +
> +local function release_type_cmp(t1, t2)
> +    t1 = release_type_weight[t1]
> +    t2 = release_type_weight[t2]
> +    -- 'No release type' means the greatest.
> +    if not t1 then
> +        if not t2 then
> +            return 0
> +        end
> +        return 1
> +    end
> +    if not t2 then
> +        return -1
> +    end
> +    return t1 - t2
> +end
> +
> +local function version_cmp(ver1, ver2)
> +    if ver1.id_major ~= ver2.id_major then
> +        return ver1.id_major - ver2.id_major
> +    end
> +    if ver1.id_middle ~= ver2.id_middle then
> +        return ver1.id_middle - ver2.id_middle
> +    end
> +    if ver1.id_minor ~= ver2.id_minor then
> +        return ver1.id_minor - ver2.id_minor
> +    end
> +    if ver1.rel_type ~= ver2.rel_type then
> +        return release_type_cmp(ver1.rel_type, ver2.rel_type)
> +    end
> +    if ver1.rel_num ~= ver2.rel_num then
> +        return ver1.rel_num - ver2.rel_num
> +    end
> +    if ver1.id_commit ~= ver2.id_commit then
> +        return ver1.id_commit - ver2.id_commit
> +    end
> +    return 0
> +end
> +
> +local version_mt = {
> +    __eq = function(l, r)
> +        return version_cmp(l, r) == 0
> +    end,
> +    __lt = function(l, r)
> +        return version_cmp(l, r) < 0
> +    end,
> +    __le = function(l, r)
> +        return version_cmp(l, r) <= 0
> +    end,
> +}
> +
> +local function version_new(id_major, id_middle, id_minor, rel_type, rel_num,
> +                           id_commit)
> +    -- There is no any proper validation - the API is not public.
> +    assert(id_major and id_middle and id_minor)
> +    return setmetatable({
> +        id_major = id_major,
> +        id_middle = id_middle,
> +        id_minor = id_minor,
> +        rel_type = rel_type,
> +        rel_num = rel_num,
> +        id_commit = id_commit,
> +    }, version_mt)
> +end
> +
> +local function version_parse(version_str)
> +    --  x.x.x-name<num>-<num>-g<commit>
> +    -- \____/\___/\___/\_____/
> +    --   P1   P2   P3    P4
> +    local id_major, id_middle, id_minor
> +    local rel_type
> +    local rel_num = 0
> +    local id_commit = 0
> +    local pos
> +
> +    -- Part 1 - version ID triplet.
> +    id_major, id_middle, id_minor = version_str:match('^(%d+)%.(%d+)%.(%d+)')
> +    if not id_major or not id_middle or not id_minor then
> +        error(('Could not parse version: %s'):format(version_str))
> +    end
> +    id_major = tonumber(id_major)
> +    id_middle = tonumber(id_middle)
> +    id_minor = tonumber(id_minor)
> +
> +    -- Cut to 'name<num>-<num>-g<commit>'.
> +    pos = version_str:find('-')
> +    if not pos then
> +        goto finish
> +    end
> +    version_str = version_str:sub(pos + 1)
> +
> +    -- Part 2 and 3 - release name, might be absent.
> +    rel_type, rel_num = version_str:match('^(%a+)(%d+)')
> +    if not rel_type then
> +        rel_type = version_str:match('^(%a+)')
> +        rel_num = 0
> +    else
> +        rel_num = tonumber(rel_num)
> +    end
> +
> +    -- Cut to '<num>-g<commit>'.
> +    pos = version_str:find('-')
> +    if not pos then
> +        goto finish
> +    end
> +    version_str = version_str:sub(pos + 1)
> +
> +    -- Part 4 - commit count since latest release, might be absent.
> +    id_commit = version_str:match('^(%d+)')
> +    if not id_commit then
> +        id_commit = 0
> +    else
> +        id_commit = tonumber(id_commit)
> +    end
> +
> +::finish::
> +    return version_new(id_major, id_middle, id_minor, rel_type, rel_num,
> +                       id_commit)
> +end
> +
> +return {
> +    parse = version_parse,
> +    new = version_new,
> +}

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

* Re: [Tarantool-patches] [PATCH vshard 3/4] router: support msgpack object args
  2022-02-10 22:33     ` Vladislav Shpilevoy via Tarantool-patches
@ 2022-02-11 16:38       ` Oleg Babin via Tarantool-patches
  0 siblings, 0 replies; 20+ messages in thread
From: Oleg Babin via Tarantool-patches @ 2022-02-11 16:38 UTC (permalink / raw)
  To: Vladislav Shpilevoy, 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,
>   }

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

* Re: [Tarantool-patches] [PATCH vshard 4/4] router: support netbox return_raw
  2022-02-10 22:34     ` Vladislav Shpilevoy via Tarantool-patches
@ 2022-02-11 16:38       ` Oleg Babin via Tarantool-patches
  0 siblings, 0 replies; 20+ messages in thread
From: Oleg Babin via Tarantool-patches @ 2022-02-11 16:38 UTC (permalink / raw)
  To: Vladislav Shpilevoy, tarantool-patches

Thanks for your fixes. LGTM.

On 11.02.2022 01:34, Vladislav Shpilevoy wrote:
> Thanks for the review!
>
>>> diff --git a/test/router-luatest/router_test.lua b/test/router-luatest/router_test.lua
>>> index 13ab74d..23929dc 100644
>>> --- a/test/router-luatest/router_test.lua
>>> +++ b/test/router-luatest/router_test.lua
> <...>
>
>>> +    --
>>> +    -- Route error.
>>> +    --
>>> +    res = router:exec(function(timeout, mode)
>>> +        local route = vshard.router.route(1)
>>> +        return add_details(route[mode](route, 'box_error', {1, 2, 3},
>>> +                           {timeout = timeout}))
>>> +    end, {wait_timeout, mode})
>>> +    t.assert(not res.val, 'no value')
>>> +    t.assert_equals(res.err_type, 'table', 'error type')
>>> +    t.assert_covers(res.err, {type = 'ClientError', code = box.error.PROC_LUA},
>>> +                    'error value')
>>> +end
>>> +
>>> +g.test_return_raw = function(g)
>> g.skip_if could be used
> Done during interactive rebase, diff wasn't saved.
>
>> Also probably you could use parametrization (however I won't insist):
>>
>> ```
>>
>> local pg = t.group('pgroup', {{mode = 'callrw'}, {engine = 'callro'}})
>> pg.test = function(cg)
>>     ...
>> end
>>
>> ```
> I tried, but then realized it will affect all the tests in the group.
> If I add in the future more tests which won't need both callro and callrw,
> they will run 2 times without need.
>
> If I make 2 groups - one with 2 modes and other without a config, then
> AFAIU I will need to recreate the cluster when the first group ends and the
> second one starts. I am trying to avoid that because it takes too a lot of
> time (compared to running the test cases).
>
>>> diff --git a/vshard/router/init.lua b/vshard/router/init.lua
>>> index 44ed801..d39f489 100644
>>> --- a/vshard/router/init.lua
>>> +++ b/vshard/router/init.lua
>>> @@ -1,7 +1,10 @@
>>>    local log = require('log')
>>>    local lfiber = require('fiber')
>>> +local lmsgpack = require('msgpack')
>>>    local table_new = require('table.new')
>>>    local fiber_clock = lfiber.clock
>>> +local msgpack_is_object = lmsgpack.is_object
>>> +local msgpack_object = lmsgpack.object
>> I suggest to add something like
>>
>> ```
>>
>> if msgpack_is_object == nil then
>>
>>      msgpack_is_object = function() error("Msgpack object feature is not supported by current Tarantool version") end
>>
>> end
>>
>> ```
> Looks good to me:
>
> ====================
> @@ -3,8 +3,6 @@ local lfiber = require('fiber')
>   local lmsgpack = require('msgpack')
>   local table_new = require('table.new')
>   local fiber_clock = lfiber.clock
> -local msgpack_is_object = lmsgpack.is_object
> -local msgpack_object = lmsgpack.object
>   
>   local MODULE_INTERNALS = '__module_vshard_router'
>   -- Reload requirements, in case this module is reloaded manually.
> @@ -25,6 +23,20 @@ local lreplicaset = require('vshard.replicaset')
>   local util = require('vshard.util')
>   local seq_serializer = { __serialize = 'seq' }
>   
> +local msgpack_is_object = lmsgpack.is_object
> +local msgpack_object = lmsgpack.object
> +
> +if not util.feature.msgpack_object then
> +    local msg = 'Msgpack object feature is not supported by current '..
> +                'Tarantool version'
> +    msgpack_is_object = function()
> +        error(msg)
> +    end
> +    msgpack_object = function()
> +        error(msg)
> +    end
> +end
> +
>   local M = rawget(_G, MODULE_INTERNALS)
> ====================
>
>
> New patch:
>
> ====================
> router: support netbox return_raw
>
> Netbox return_raw option allows to return remote call result
> without decoding, as an msgpack object. It will be introduced
> after 2.10.0-beta2.
>
> It can be used with router working as a proxy. Pass arguments to
> it in msgpack object and get back another msgpack object by usage
> of return_raw option. This way the router won't decode nor encode
> user's function arguments at all.
>
> Part of #312
> ---
>   test/instances/storage.lua           |  5 ++
>   test/reload_evolution/storage.result |  4 --
>   test/router-luatest/router_test.lua  | 84 ++++++++++++++++++++++++++++
>   test/storage/storage.result          |  2 -
>   vshard/router/init.lua               | 40 ++++++++++++-
>   vshard/storage/init.lua              | 15 +++++
>   vshard/util.lua                      |  1 +
>   7 files changed, 144 insertions(+), 7 deletions(-)
>
> diff --git a/test/instances/storage.lua b/test/instances/storage.lua
> index 7ad2af3..2afc942 100755
> --- a/test/instances/storage.lua
> +++ b/test/instances/storage.lua
> @@ -14,10 +14,15 @@ end
>   box.cfg(helpers.box_cfg())
>   box.schema.user.grant('guest', 'super', nil, nil, {if_not_exists = true})
>   
> +local function box_error()
> +    box.error(box.error.PROC_LUA, 'box_error')
> +end
> +
>   local function echo(...)
>       return ...
>   end
>   
> +_G.box_error = box_error
>   _G.echo = echo
>   
>   _G.ready = true
> diff --git a/test/reload_evolution/storage.result b/test/reload_evolution/storage.result
> index 77010a2..a282bba 100644
> --- a/test/reload_evolution/storage.result
> +++ b/test/reload_evolution/storage.result
> @@ -122,8 +122,6 @@ vshard.storage.call(bucket_id_to_move, 'read', 'do_select', {42})
>   ---
>   - true
>   - - [42, 3000]
> -- null
> -- null
>   ...
>   vshard.storage.bucket_send(bucket_id_to_move, util.replicasets[1])
>   ---
> @@ -155,8 +153,6 @@ vshard.storage.call(bucket_id_to_move, 'read', 'do_select', {42})
>   ---
>   - true
>   - - [42, 3000]
> -- null
> -- null
>   ...
>   -- Check info() does not fail.
>   vshard.storage.info() ~= nil
> diff --git a/test/router-luatest/router_test.lua b/test/router-luatest/router_test.lua
> index b7aeea9..39dedcb 100644
> --- a/test/router-luatest/router_test.lua
> +++ b/test/router-luatest/router_test.lua
> @@ -96,3 +96,87 @@ g.test_msgpack_args = function(g)
>       t.assert(err == nil, 'no error')
>       t.assert_equals(res, 100, 'good result')
>   end
> +
> +local function test_return_raw_template(g, mode)
> +    --
> +    -- Normal call.
> +    --
> +    local router = g.router
> +    local res = router:exec(function(timeout, mode)
> +        return add_details(vshard.router[mode](1, 'echo', {1, 2, 3},
> +                           {timeout = timeout, return_raw = true}))
> +    end, {wait_timeout, mode})
> +    t.assert_equals(res.val, {1, 2, 3}, 'value value')
> +    t.assert_equals(res.val_type, 'userdata', 'value type')
> +    t.assert(not res.err, 'no error')
> +
> +    --
> +    -- Route call.
> +    --
> +    res = router:exec(function(timeout, mode)
> +        local route = vshard.router.route(1)
> +        return add_details(route[mode](route, 'echo', {1, 2, 3},
> +                           {timeout = timeout, return_raw = true}))
> +    end, {wait_timeout, mode})
> +    t.assert_equals(res.val, {1, 2, 3}, 'value value')
> +    t.assert_equals(res.val_type, 'userdata', 'value type')
> +    t.assert(not res.err, 'no error')
> +
> +    --
> +    -- Empty result set.
> +    --
> +    res = router:exec(function(timeout, mode)
> +        return add_details(vshard.router[mode](1, 'echo', {},
> +                           {timeout = timeout, return_raw = true}))
> +    end, {wait_timeout, mode})
> +    t.assert(not res.val, 'no value')
> +    t.assert(not res.err, 'no error')
> +
> +    --
> +    -- Error.
> +    --
> +    res = router:exec(function(timeout, mode)
> +        return add_details(vshard.router[mode](1, 'box_error', {1, 2, 3},
> +                           {timeout = timeout}))
> +    end, {wait_timeout, mode})
> +    t.assert(not res.val, 'no value')
> +    t.assert_equals(res.err_type, 'table', 'error type')
> +    t.assert_covers(res.err, {type = 'ClientError', code = box.error.PROC_LUA},
> +                    'error value')
> +
> +    --
> +    -- Route error.
> +    --
> +    res = router:exec(function(timeout, mode)
> +        local route = vshard.router.route(1)
> +        return add_details(route[mode](route, 'box_error', {1, 2, 3},
> +                           {timeout = timeout}))
> +    end, {wait_timeout, mode})
> +    t.assert(not res.val, 'no value')
> +    t.assert_equals(res.err_type, 'table', 'error type')
> +    t.assert_covers(res.err, {type = 'ClientError', code = box.error.PROC_LUA},
> +                    'error value')
> +end
> +
> +g.test_return_raw = function(g)
> +    t.run_only_if(vutil.feature.netbox_return_raw)
> +
> +    g.router:exec(function()
> +        rawset(_G, 'add_details', function(val, err)
> +            -- Direct return would turn nils into box.NULLs. The tests want to
> +            -- ensure it doesn't happen. Table wrap makes the actual nils
> +            -- eliminate themselves.
> +            return {
> +                val = val,
> +                val_type = type(val),
> +                err = err,
> +                err_type = type(err),
> +            }
> +        end)
> +    end)
> +    test_return_raw_template(g, 'callrw')
> +    test_return_raw_template(g, 'callro')
> +    g.router:exec(function()
> +        _G.add_details = nil
> +    end)
> +end
> diff --git a/test/storage/storage.result b/test/storage/storage.result
> index dcd1c1f..73c171a 100644
> --- a/test/storage/storage.result
> +++ b/test/storage/storage.result
> @@ -480,8 +480,6 @@ vshard.storage.call(1, 'read', 'space_get', {'test', {1}})
>   ---
>   - true
>   - [1, 1]
> -- null
> -- null
>   ...
>   vshard.storage.call(100500, 'read', 'space_get', {'test', {1}})
>   ---
> diff --git a/vshard/router/init.lua b/vshard/router/init.lua
> index 44ed801..8f0f489 100644
> --- a/vshard/router/init.lua
> +++ b/vshard/router/init.lua
> @@ -1,5 +1,6 @@
>   local log = require('log')
>   local lfiber = require('fiber')
> +local lmsgpack = require('msgpack')
>   local table_new = require('table.new')
>   local fiber_clock = lfiber.clock
>   
> @@ -22,6 +23,20 @@ local lreplicaset = require('vshard.replicaset')
>   local util = require('vshard.util')
>   local seq_serializer = { __serialize = 'seq' }
>   
> +local msgpack_is_object = lmsgpack.is_object
> +local msgpack_object = lmsgpack.object
> +
> +if not util.feature.msgpack_object then
> +    local msg = 'Msgpack object feature is not supported by current '..
> +                'Tarantool version'
> +    msgpack_is_object = function()
> +        error(msg)
> +    end
> +    msgpack_object = function()
> +        error(msg)
> +    end
> +end
> +
>   local M = rawget(_G, MODULE_INTERNALS)
>   if not M then
>       M = {
> @@ -530,14 +545,17 @@ end
>   --
>   local function router_call_impl(router, bucket_id, mode, prefer_replica,
>                                   balance, func, args, opts)
> +    local do_return_raw
>       if opts then
>           if type(opts) ~= 'table' or
>              (opts.timeout and type(opts.timeout) ~= 'number') then
>               error('Usage: call(bucket_id, mode, func, args, opts)')
>           end
>           opts = table.copy(opts)
> -    elseif not opts then
> +        do_return_raw = opts.return_raw
> +    else
>           opts = {}
> +        do_return_raw = false
>       end
>       local timeout = opts.timeout or consts.CALL_TIMEOUT_MIN
>       local replicaset, err
> @@ -569,6 +587,26 @@ local function router_call_impl(router, bucket_id, mode, prefer_replica,
>               local storage_call_status, call_status, call_error =
>                   replicaset[call](replicaset, 'vshard.storage.call',
>                                    {bucket_id, mode, func, args}, opts)
> +            if do_return_raw and msgpack_is_object(storage_call_status) then
> +                -- Storage.call returns in the first value a flag whether user's
> +                -- function threw an exception or not. Need to extract it.
> +                -- Unfortunately, it forces to repack the rest of values into a
> +                -- new array. But the values themselves are not decoded.
> +                local it = storage_call_status:iterator()
> +                local count = it:decode_array_header()
> +                storage_call_status = it:decode()
> +                -- When no values, nil is not packed into msgpack object. Same
> +                -- as in raw netbox.
> +                if count > 1 then
> +                    count = count - 1
> +                    local res = table_new(count, 0)
> +                    for i = 1, count do
> +                        res[i] = it:take()
> +                    end
> +                    call_status = msgpack_object(res)
> +                end
> +                call_error = nil
> +            end
>               if storage_call_status then
>                   if call_status == nil and call_error ~= nil then
>                       return call_status, call_error
> diff --git a/vshard/storage/init.lua b/vshard/storage/init.lua
> index 6820ad0..5083911 100644
> --- a/vshard/storage/init.lua
> +++ b/vshard/storage/init.lua
> @@ -2512,6 +2512,21 @@ local function storage_call(bucket_id, mode, name, args)
>       if not ok then
>           ret1 = lerror.make(ret1)
>       end
> +    -- Truncate nil values. Can't return them all because empty values turn into
> +    -- box.NULL. Even if user's function actually returned just 1 value, this
> +    -- would lead to '1, box.NULL, box.NULL' on the client. Not visible on the
> +    -- router when called normally, but won't help if the router did
> +    -- 'return_raw' and just forwards everything as is without truncation.
> +    -- Although this solution truncates really returned nils.
> +    if ret3 == nil then
> +        if ret2 == nil then
> +            if ret1 == nil then
> +                return ok
> +            end
> +            return ok, ret1
> +        end
> +        return ok, ret1, ret2
> +    end
>       return ok, ret1, ret2, ret3
>   end
>   
> diff --git a/vshard/util.lua b/vshard/util.lua
> index 354727d..c6975cf 100644
> --- a/vshard/util.lua
> +++ b/vshard/util.lua
> @@ -237,6 +237,7 @@ end
>   --
>   local feature = {
>       msgpack_object = lmsgpack.object ~= nil,
> +    netbox_return_raw = version_is_at_least(2, 10, 0, 'beta', 2, 86),
>   }
>   
>   return {

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

* Re: [Tarantool-patches] [PATCH vshard 0/4] Router msgpack object and netbox return_raw
  2022-02-09  0:32 [Tarantool-patches] [PATCH vshard 0/4] Router msgpack object and netbox return_raw Vladislav Shpilevoy via Tarantool-patches
                   ` (3 preceding siblings ...)
  2022-02-09  0:32 ` [Tarantool-patches] [PATCH vshard 4/4] router: support netbox return_raw Vladislav Shpilevoy via Tarantool-patches
@ 2022-02-11 23:05 ` Vladislav Shpilevoy via Tarantool-patches
  2022-02-15 16:55   ` Oleg Babin via Tarantool-patches
  4 siblings, 1 reply; 20+ messages in thread
From: Vladislav Shpilevoy via Tarantool-patches @ 2022-02-11 23:05 UTC (permalink / raw)
  To: tarantool-patches, olegrok

Pushed to master.

But the ticket isn't finished yet. I need to measure performance of return_raw
vs normal call with full encode/decode of all values. It is in progress.

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

* Re: [Tarantool-patches] [PATCH vshard 0/4] Router msgpack object and netbox return_raw
  2022-02-11 23:05 ` [Tarantool-patches] [PATCH vshard 0/4] Router msgpack object and " Vladislav Shpilevoy via Tarantool-patches
@ 2022-02-15 16:55   ` Oleg Babin via Tarantool-patches
  2022-02-15 21:16     ` Vladislav Shpilevoy via Tarantool-patches
  0 siblings, 1 reply; 20+ messages in thread
From: Oleg Babin via Tarantool-patches @ 2022-02-15 16:55 UTC (permalink / raw)
  To: Vladislav Shpilevoy, tarantool-patches

Hi! I remembered one more moment. Could we support return_raw for 
vshard.router.map_callrw?


On 12.02.2022 02:05, Vladislav Shpilevoy wrote:
> Pushed to master.
>
> But the ticket isn't finished yet. I need to measure performance of return_raw
> vs normal call with full encode/decode of all values. It is in progress.

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

* Re: [Tarantool-patches] [PATCH vshard 0/4] Router msgpack object and netbox return_raw
  2022-02-15 16:55   ` Oleg Babin via Tarantool-patches
@ 2022-02-15 21:16     ` Vladislav Shpilevoy via Tarantool-patches
  0 siblings, 0 replies; 20+ messages in thread
From: Vladislav Shpilevoy via Tarantool-patches @ 2022-02-15 21:16 UTC (permalink / raw)
  To: Oleg Babin, tarantool-patches

Shouldn't be a problem. I will send a separate patch.

On 15.02.2022 17:55, Oleg Babin wrote:
> Hi! I remembered one more moment. Could we support return_raw for vshard.router.map_callrw?
> 
> 
> On 12.02.2022 02:05, Vladislav Shpilevoy wrote:
>> Pushed to master.
>>
>> But the ticket isn't finished yet. I need to measure performance of return_raw
>> vs normal call with full encode/decode of all values. It is in progress.

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

end of thread, other threads:[~2022-02-15 21:16 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-09  0:32 [Tarantool-patches] [PATCH vshard 0/4] Router msgpack object and netbox return_raw Vladislav Shpilevoy via Tarantool-patches
2022-02-09  0:32 ` [Tarantool-patches] [PATCH vshard 1/4] test: support luatest Vladislav Shpilevoy via Tarantool-patches
2022-02-09 17:53   ` Oleg Babin via Tarantool-patches
2022-02-10 22:32     ` Vladislav Shpilevoy via Tarantool-patches
2022-02-11 16:38       ` Oleg Babin via Tarantool-patches
2022-02-09  0:32 ` [Tarantool-patches] [PATCH vshard 2/4] util: introduce Tarantool's semver parser Vladislav Shpilevoy via Tarantool-patches
2022-02-09 17:53   ` Oleg Babin via Tarantool-patches
2022-02-10 22:33     ` Vladislav Shpilevoy via Tarantool-patches
2022-02-11 16:38       ` Oleg Babin via Tarantool-patches
2022-02-09  0:32 ` [Tarantool-patches] [PATCH vshard 3/4] router: support msgpack object args Vladislav Shpilevoy via Tarantool-patches
2022-02-09 17:53   ` Oleg Babin via Tarantool-patches
2022-02-10 22:33     ` Vladislav Shpilevoy via Tarantool-patches
2022-02-11 16:38       ` Oleg Babin via Tarantool-patches
2022-02-09  0:32 ` [Tarantool-patches] [PATCH vshard 4/4] router: support netbox return_raw Vladislav Shpilevoy via Tarantool-patches
2022-02-09 17:53   ` Oleg Babin via Tarantool-patches
2022-02-10 22:34     ` Vladislav Shpilevoy via Tarantool-patches
2022-02-11 16:38       ` Oleg Babin via Tarantool-patches
2022-02-11 23:05 ` [Tarantool-patches] [PATCH vshard 0/4] Router msgpack object and " Vladislav Shpilevoy via Tarantool-patches
2022-02-15 16:55   ` Oleg Babin via Tarantool-patches
2022-02-15 21:16     ` Vladislav Shpilevoy via Tarantool-patches

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