Tarantool development patches archive
 help / color / mirror / Atom feed
* [Tarantool-patches] [PATCH vshard 0/2] VShard new netbox
@ 2021-09-28 23:08 Vladislav Shpilevoy via Tarantool-patches
  2021-09-28 23:08 ` [Tarantool-patches] [PATCH vshard 1/2] router: wrap is_async futures completely Vladislav Shpilevoy via Tarantool-patches
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Vladislav Shpilevoy via Tarantool-patches @ 2021-09-28 23:08 UTC (permalink / raw)
  To: tarantool-patches, olegrok

The patch makes vshard support new netbox: is_async and proper errors which are
now returned from the server as MP_ERROR by default (in netbox at least).

Branch: http://github.com/tarantool/vshard/tree/gerold103/gh-294-is_async
Issue: https://github.com/tarantool/vshard/issues/294

Vladislav Shpilevoy (2):
  router: wrap is_async futures completely
  test: drop error codes from test output

 test/lua_libs/storage_template.lua      |  11 +
 test/lua_libs/util.lua                  |   2 +-
 test/misc/check_uuid_on_connect.result  |   2 -
 test/rebalancer/bucket_ref.result       |   1 -
 test/rebalancer/receiving_bucket.result |   3 -
 test/router/retry_reads.result          |   2 -
 test/router/router.result               | 304 +++++++++++++++++++++++-
 test/router/router.test.lua             | 120 +++++++++-
 test/router/sync.result                 |   2 -
 test/storage/storage.result             |   1 -
 test/unit/error.result                  |   2 -
 vshard/router/init.lua                  | 148 +++++++++---
 12 files changed, 543 insertions(+), 55 deletions(-)

-- 
2.24.3 (Apple Git-128)


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

* [Tarantool-patches] [PATCH vshard 1/2] router: wrap is_async futures completely
  2021-09-28 23:08 [Tarantool-patches] [PATCH vshard 0/2] VShard new netbox Vladislav Shpilevoy via Tarantool-patches
@ 2021-09-28 23:08 ` Vladislav Shpilevoy via Tarantool-patches
  2021-09-29  6:18   ` Oleg Babin via Tarantool-patches
  2021-09-28 23:08 ` [Tarantool-patches] [PATCH vshard 2/2] test: drop error codes from test output Vladislav Shpilevoy via Tarantool-patches
  2021-10-01 21:14 ` [Tarantool-patches] [PATCH vshard 0/2] VShard new netbox Vladislav Shpilevoy via Tarantool-patches
  2 siblings, 1 reply; 8+ messages in thread
From: Vladislav Shpilevoy via Tarantool-patches @ 2021-09-28 23:08 UTC (permalink / raw)
  To: tarantool-patches, olegrok

Router used to override :result() method of netbox futures. It is
needed because user functions are called via vshard.storage.call()
which returns some metadata - it must be truncated before
returning the user's data.

It worked fine while netbox futures were implemented as tables.
But in the newest Tarantool most of netbox state machine code is
moved into C. The futures now are cdata.

They allow to add new members, but can't override their methods.
As a result, on the newest Tarantool is_async in
vshard.router.call() simply didn't work.

The patch wraps netbox futures completely with a Lua table, not
just overrides one method. Now it works the same on all Tarantool
versions starting from 1.10.

Closes #294
---
 test/lua_libs/storage_template.lua |  11 ++
 test/router/router.result          | 301 ++++++++++++++++++++++++++++-
 test/router/router.test.lua        | 120 +++++++++++-
 vshard/router/init.lua             | 148 ++++++++++----
 4 files changed, 542 insertions(+), 38 deletions(-)

diff --git a/test/lua_libs/storage_template.lua b/test/lua_libs/storage_template.lua
index 8df89f6..83ea710 100644
--- a/test/lua_libs/storage_template.lua
+++ b/test/lua_libs/storage_template.lua
@@ -101,6 +101,8 @@ function bootstrap_storage(engine)
         box.schema.role.grant('public', 'execute', 'function', 'raise_client_error')
         box.schema.func.create('do_push')
         box.schema.role.grant('public', 'execute', 'function', 'do_push')
+        box.schema.func.create('do_push_wait')
+        box.schema.role.grant('public', 'execute', 'function', 'do_push_wait')
         box.snapshot()
     end)
 end
@@ -152,6 +154,15 @@ function do_push(push, retval)
     return retval
 end
 
+is_push_wait_blocked = true
+function do_push_wait(push, retval_arr)
+    box.session.push(push)
+    while is_push_wait_blocked do
+        fiber.sleep(0.001)
+    end
+    return unpack(retval_arr)
+end
+
 --
 -- Wait a specified log message.
 -- Requirements:
diff --git a/test/router/router.result b/test/router/router.result
index 8a0e30d..8ddbe6d 100644
--- a/test/router/router.result
+++ b/test/router/router.result
@@ -613,7 +613,7 @@ messages
 - - 100
 ...
 --
--- gh-171: support is_async.
+-- gh-171, gh-294: support is_async.
 --
 future = vshard.router.callro(bucket_id, 'space_get', {'test', {1}}, {is_async = true})
 ---
@@ -632,6 +632,11 @@ future = vshard.router.callrw(bucket_id, 'raise_client_error', {}, {is_async = t
 res, err = future:wait_result()
 ---
 ...
+-- VShard wraps all errors.
+assert(type(err) == 'table')
+---
+- true
+...
 util.portable_error(err)
 ---
 - type: ClientError
@@ -690,6 +695,300 @@ future:wait_result()
 - [[1, 1]]
 ...
 --
+-- Error as a result of discard.
+--
+future = vshard.router.callrw(bucket_id, 'do_push_wait', {10, {20}},            \
+                              {is_async = true})
+---
+...
+future:discard()
+---
+...
+res, err = future:result()
+---
+...
+assert(not res and err.message:match('discarded') ~= nil)
+---
+- true
+...
+assert(type(err) == 'table')
+---
+- true
+...
+res, err = future:wait_result()
+---
+...
+assert(not res and err.message:match('discarded') ~= nil)
+---
+- true
+...
+assert(type(err) == 'table')
+---
+- true
+...
+--
+-- See how pairs behaves when the final result is not immediately ready.
+--
+future = vshard.router.callrw(bucket_id, 'do_push_wait', {10, {20}},            \
+                              {is_async = true})
+---
+...
+assert(not future:is_ready())
+---
+- true
+...
+-- Get the push successfully.
+func, iter, i = future:pairs()
+---
+...
+i, res = func(iter, i)
+---
+...
+assert(i == 1)
+---
+- true
+...
+assert(res == 10)
+---
+- true
+...
+-- Fail to get the final result during the timeout. It is supposed to test how
+-- the router knows which result is final and which is just a push. Even before
+-- the request ends.
+func, iter, i = future:pairs(0.001)
+---
+...
+i, res = func(iter, i)
+---
+...
+i, res = func(iter, i)
+---
+...
+assert(not i and res.code == box.error.TIMEOUT)
+---
+- true
+...
+assert(type(res) == 'table')
+---
+- true
+...
+res, err = future:wait_result(0.001)
+---
+...
+assert(not res and err.code == box.error.TIMEOUT)
+---
+- true
+...
+assert(type(err) == 'table')
+---
+- true
+...
+test_run:switch('storage_1_a')
+---
+- true
+...
+is_push_wait_blocked = false
+---
+...
+test_run:switch('storage_2_a')
+---
+- true
+...
+is_push_wait_blocked = false
+---
+...
+test_run:switch('router_1')
+---
+- true
+...
+func, iter, i = future:pairs()
+---
+...
+i, res = func(iter, i)
+---
+...
+assert(i == 1)
+---
+- true
+...
+assert(res == 10)
+---
+- true
+...
+i, res = func(iter, i)
+---
+...
+assert(i == 2)
+---
+- true
+...
+assert(res[1] == 20 and not res[2])
+---
+- true
+...
+assert(future:is_ready())
+---
+- true
+...
+i, res = func(iter, i)
+---
+...
+assert(not i)
+---
+- true
+...
+assert(not res)
+---
+- true
+...
+-- Repeat the same to ensure it returns the same.
+i, res = func(iter, 1)
+---
+...
+assert(i == 2)
+---
+- true
+...
+assert(res[1] == 20 and not res[2])
+---
+- true
+...
+-- Non-pairs functions return correctly unpacked successful results.
+res, err = future:wait_result()
+---
+...
+assert(res[1] == 20 and not res[2] and not err)
+---
+- true
+...
+res, err = future:result()
+---
+...
+assert(res[1] == 20 and not res[2] and not err)
+---
+- true
+...
+-- Return 2 nils - shouldn't be treated as an error.
+future = vshard.router.callrw(bucket_id, 'do_push_wait',                        \
+                              {10, {nil, nil}}, {is_async = true})
+---
+...
+res, err = future:wait_result()
+---
+...
+assert(res[1] == nil and res[2] == nil and not err)
+---
+- true
+...
+res, err = future:result()
+---
+...
+assert(res[1] == nil and res[2] == nil and not err)
+---
+- true
+...
+func, iter, i = future:pairs()
+---
+...
+i, res = func(iter, i)
+---
+...
+i, res = func(iter, i)
+---
+...
+assert(res[1] == nil and res[2] == nil and not err)
+---
+- true
+...
+-- Serialize and tostring.
+future
+---
+- []
+...
+future.key = 'value'
+---
+...
+future
+---
+- key: value
+...
+tostring(future)
+---
+- vshard.net.box.request
+...
+--
+-- The same, but the push function returns an error.
+--
+future = vshard.router.callrw(bucket_id, 'do_push_wait', {10, {nil, 'err'}},    \
+                              {is_async = true})
+---
+...
+func, iter, i = future:pairs()
+---
+...
+i, res = func(iter, i)
+---
+...
+assert(i == 1)
+---
+- true
+...
+assert(res == 10)
+---
+- true
+...
+i, res = func(iter, i)
+---
+...
+-- This test is for the sake of checking how the async request handles nil,err
+-- result.
+assert(i == 2)
+---
+- true
+...
+assert(not res[1] and res[2].message == 'err')
+---
+- true
+...
+assert(type(res[2]) == 'table')
+---
+- true
+...
+i, res = func(iter, i)
+---
+...
+assert(not i)
+---
+- true
+...
+assert(not res)
+---
+- true
+...
+-- Non-pairs getting of an error.
+res, err = future:wait_result()
+---
+...
+assert(not res and err.message == 'err')
+---
+- true
+...
+assert(type(err) == 'table')
+---
+- true
+...
+res, err = future:result()
+---
+...
+assert(not res and err.message == 'err')
+---
+- true
+...
+assert(type(err) == 'table')
+---
+- true
+...
+--
 -- Test errors from router call.
 --
 new_bid = vshard.consts.DEFAULT_BUCKET_COUNT + 1
diff --git a/test/router/router.test.lua b/test/router/router.test.lua
index 0017111..0f3854a 100644
--- a/test/router/router.test.lua
+++ b/test/router/router.test.lua
@@ -216,13 +216,15 @@ vshard.router.route(bucket_id):callrw('do_push', args, opts)
 messages
 
 --
--- gh-171: support is_async.
+-- gh-171, gh-294: support is_async.
 --
 future = vshard.router.callro(bucket_id, 'space_get', {'test', {1}}, {is_async = true})
 future:wait_result()
 future:is_ready()
 future = vshard.router.callrw(bucket_id, 'raise_client_error', {}, {is_async = true})
 res, err = future:wait_result()
+-- VShard wraps all errors.
+assert(type(err) == 'table')
 util.portable_error(err)
 future:is_ready()
 future = vshard.router.callrw(bucket_id, 'do_push', args, {is_async = true})
@@ -240,6 +242,122 @@ future:wait_result()
 future = vshard.router.route(bucket_id):callrw('space_get', {'test', {1}}, {is_async = true})
 future:wait_result()
 
+--
+-- Error as a result of discard.
+--
+future = vshard.router.callrw(bucket_id, 'do_push_wait', {10, {20}},            \
+                              {is_async = true})
+future:discard()
+res, err = future:result()
+assert(not res and err.message:match('discarded') ~= nil)
+assert(type(err) == 'table')
+res, err = future:wait_result()
+assert(not res and err.message:match('discarded') ~= nil)
+assert(type(err) == 'table')
+
+--
+-- See how pairs behaves when the final result is not immediately ready.
+--
+future = vshard.router.callrw(bucket_id, 'do_push_wait', {10, {20}},            \
+                              {is_async = true})
+assert(not future:is_ready())
+-- Get the push successfully.
+func, iter, i = future:pairs()
+i, res = func(iter, i)
+assert(i == 1)
+assert(res == 10)
+
+-- Fail to get the final result during the timeout. It is supposed to test how
+-- the router knows which result is final and which is just a push. Even before
+-- the request ends.
+func, iter, i = future:pairs(0.001)
+i, res = func(iter, i)
+i, res = func(iter, i)
+assert(not i and res.code == box.error.TIMEOUT)
+assert(type(res) == 'table')
+
+res, err = future:wait_result(0.001)
+assert(not res and err.code == box.error.TIMEOUT)
+assert(type(err) == 'table')
+
+test_run:switch('storage_1_a')
+is_push_wait_blocked = false
+test_run:switch('storage_2_a')
+is_push_wait_blocked = false
+test_run:switch('router_1')
+
+func, iter, i = future:pairs()
+i, res = func(iter, i)
+assert(i == 1)
+assert(res == 10)
+
+i, res = func(iter, i)
+assert(i == 2)
+assert(res[1] == 20 and not res[2])
+
+assert(future:is_ready())
+
+i, res = func(iter, i)
+assert(not i)
+assert(not res)
+
+-- Repeat the same to ensure it returns the same.
+i, res = func(iter, 1)
+assert(i == 2)
+assert(res[1] == 20 and not res[2])
+
+-- Non-pairs functions return correctly unpacked successful results.
+res, err = future:wait_result()
+assert(res[1] == 20 and not res[2] and not err)
+res, err = future:result()
+assert(res[1] == 20 and not res[2] and not err)
+
+-- Return 2 nils - shouldn't be treated as an error.
+future = vshard.router.callrw(bucket_id, 'do_push_wait',                        \
+                              {10, {nil, nil}}, {is_async = true})
+res, err = future:wait_result()
+assert(res[1] == nil and res[2] == nil and not err)
+res, err = future:result()
+assert(res[1] == nil and res[2] == nil and not err)
+func, iter, i = future:pairs()
+i, res = func(iter, i)
+i, res = func(iter, i)
+assert(res[1] == nil and res[2] == nil and not err)
+
+-- Serialize and tostring.
+future
+future.key = 'value'
+future
+tostring(future)
+
+--
+-- The same, but the push function returns an error.
+--
+future = vshard.router.callrw(bucket_id, 'do_push_wait', {10, {nil, 'err'}},    \
+                              {is_async = true})
+func, iter, i = future:pairs()
+i, res = func(iter, i)
+assert(i == 1)
+assert(res == 10)
+i, res = func(iter, i)
+-- This test is for the sake of checking how the async request handles nil,err
+-- result.
+assert(i == 2)
+assert(not res[1] and res[2].message == 'err')
+assert(type(res[2]) == 'table')
+i, res = func(iter, i)
+assert(not i)
+assert(not res)
+
+-- Non-pairs getting of an error.
+res, err = future:wait_result()
+assert(not res and err.message == 'err')
+assert(type(err) == 'table')
+
+res, err = future:result()
+assert(not res and err.message == 'err')
+assert(type(err) == 'table')
+
 --
 -- Test errors from router call.
 --
diff --git a/vshard/router/init.lua b/vshard/router/init.lua
index 3d468f3..42de740 100644
--- a/vshard/router/init.lua
+++ b/vshard/router/init.lua
@@ -470,6 +470,114 @@ end
 -- API
 --------------------------------------------------------------------------------
 
+local function vshard_future_tostring(self)
+    return 'vshard.net.box.request'
+end
+
+local function vshard_future_serialize(self)
+    -- Drop the metatable. It is also copied and if returned as is leads to
+    -- recursive serialization.
+    local s = setmetatable(table.deepcopy(self), {})
+    s._base = nil
+    return s
+end
+
+local function vshard_future_is_ready(self)
+    return self._base:is_ready()
+end
+
+local function vshard_future_wrap_result(res)
+    local storage_ok, res, err = unpack(res)
+    if storage_ok then
+        if res == nil and err ~= nil then
+            return nil, lerror.make(err)
+        end
+        return setmetatable({res}, seq_serializer)
+    end
+    return nil, lerror.make(res)
+end
+
+local function vshard_future_result(self)
+    local res, err = self._base:result()
+    if res == nil then
+        return nil, lerror.make(err)
+    end
+    return vshard_future_wrap_result(res)
+end
+
+local function vshard_future_wait_result(self, timeout)
+    local res, err = self._base:wait_result(timeout)
+    if res == nil then
+        return nil, lerror.make(err)
+    end
+    return vshard_future_wrap_result(res)
+end
+
+local function vshard_future_discard(self)
+    return self._base:discard()
+end
+
+local function vshard_future_iter_next(iter, i)
+    local res, err
+    local base_next = iter.base_next
+    local base_req = iter.base_req
+    local base = iter.base
+    -- Need to distinguish the last response from the pushes. Because the former
+    -- has metadata returned by vshard.storage.call().
+    -- At the same time there is no way to check if the base pairs() did its
+    -- last iteration except calling its next() function again.
+    -- This, in turn, might lead to a block if the result is not ready yet.
+    i, res = base_next(base, i)
+    -- To avoid that there is a 2-phase check.
+    -- If the request isn't finished after first next(), it means the result is
+    -- not received. This is a push. Return as is.
+    -- If the request is finished, it is safe to call next() again to check if
+    -- it ended. It won't block.
+    local is_done = base_req:is_ready()
+
+    if not is_done then
+        -- Definitely a push. It would be finished if the final result was
+        -- received.
+        if i == nil then
+            return nil, lerror.make(res)
+        end
+        return i, res
+    end
+    if i == nil then
+        if res ~= nil then
+            return i, lerror.make(res)
+        end
+        return nil, nil
+    end
+    -- Will not block because the request is already finished.
+    if base_next(base, i) == nil then
+        res, err = vshard_future_wrap_result(res)
+        if res ~= nil then
+            return i, res
+        end
+        return i, {nil, lerror.make(err)}
+    end
+    return i, res
+end
+
+local function vshard_future_pairs(self, timeout)
+    local next_f, iter, i = self._base:pairs(timeout)
+    return vshard_future_iter_next,
+           {base = iter, base_req = self, base_next = next_f}, i
+end
+
+local vshard_future_mt = {
+    __tostring = vshard_future_tostring,
+    __serialize = vshard_future_serialize,
+    __index = {
+        is_ready = vshard_future_is_ready,
+        result = vshard_future_result,
+        wait_result = vshard_future_wait_result,
+        discard = vshard_future_discard,
+        pairs = vshard_future_pairs,
+    }
+}
+
 --
 -- Since 1.10 netbox supports flag 'is_async'. Given this flag, a
 -- request result is returned immediately in a form of a future
@@ -482,41 +590,9 @@ end
 -- So vshard.router.call should wrap a future object with its own
 -- unpacker of result.
 --
--- Unpack a result given from a future object from
--- vshard.storage.call. If future returns [status, result, ...]
--- this function returns [result]. Or a classical couple
--- nil, error.
---
-function future_storage_call_result(self)
-    local res, err = self:base_result()
-    if not res then
-        return nil, err
-    end
-    local storage_call_status, call_status, call_error = unpack(res)
-    if storage_call_status then
-        if call_status == nil and call_error ~= nil then
-            return call_status, call_error
-        else
-            return setmetatable({call_status}, seq_serializer)
-        end
-    end
-    return nil, call_status
-end
-
---
--- Given a netbox future object, redefine its 'result' method.
--- It is impossible to just create a new signle metatable per
--- the module as a copy of original future's one because it has
--- some upvalues related to the netbox connection.
---
-local function wrap_storage_call_future(future)
-    -- Base 'result' below is got from __index metatable under the
-    -- hood. But __index is used only when a table has no such a
-    -- member in itself. So via adding 'result' as a member to a
-    -- future object its __index.result can be redefined.
-    future.base_result = future.result
-    future.result = future_storage_call_result
-    return future
+local function vshard_future_new(future)
+    -- Use '_' as a prefix so as users could use all normal names.
+    return setmetatable({_base = future}, vshard_future_mt)
 end
 
 -- Perform shard operation
@@ -574,7 +650,7 @@ local function router_call_impl(router, bucket_id, mode, prefer_replica,
                     -- values: true/false and func result. But
                     -- async returns future object. No true/false
                     -- nor func result. So return the first value.
-                    return wrap_storage_call_future(storage_call_status)
+                    return vshard_future_new(storage_call_status)
                 end
             end
             err = lerror.make(call_status)
-- 
2.24.3 (Apple Git-128)


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

* [Tarantool-patches] [PATCH vshard 2/2] test: drop error codes from test output
  2021-09-28 23:08 [Tarantool-patches] [PATCH vshard 0/2] VShard new netbox Vladislav Shpilevoy via Tarantool-patches
  2021-09-28 23:08 ` [Tarantool-patches] [PATCH vshard 1/2] router: wrap is_async futures completely Vladislav Shpilevoy via Tarantool-patches
@ 2021-09-28 23:08 ` Vladislav Shpilevoy via Tarantool-patches
  2021-09-29  6:18   ` Oleg Babin via Tarantool-patches
  2021-10-01 21:14 ` [Tarantool-patches] [PATCH vshard 0/2] VShard new netbox Vladislav Shpilevoy via Tarantool-patches
  2 siblings, 1 reply; 8+ messages in thread
From: Vladislav Shpilevoy via Tarantool-patches @ 2021-09-28 23:08 UTC (permalink / raw)
  To: tarantool-patches, olegrok

In the newest Tarantool the error objects are encoded as MP_ERROR
in netbox connections by default. This allows to transfer their
full data including correct error code, even when returned as
nil,err instead of throwing the error as an exception.

Because of that some vshard tests failed - they expected the error
codes returned from the server to be wrong.

On the other hand, the codes can't be fixed, because then the
tests wouldn't work on 1.10.

The patch just drops the codes. At least while need to support
Tarantool versions not using MP_ERROR in netbox by default.
---
 test/lua_libs/util.lua                  | 2 +-
 test/misc/check_uuid_on_connect.result  | 2 --
 test/rebalancer/bucket_ref.result       | 1 -
 test/rebalancer/receiving_bucket.result | 3 ---
 test/router/retry_reads.result          | 2 --
 test/router/router.result               | 3 ---
 test/router/sync.result                 | 2 --
 test/storage/storage.result             | 1 -
 test/unit/error.result                  | 2 --
 9 files changed, 1 insertion(+), 17 deletions(-)

diff --git a/test/lua_libs/util.lua b/test/lua_libs/util.lua
index 9c2e667..fe23065 100644
--- a/test/lua_libs/util.lua
+++ b/test/lua_libs/util.lua
@@ -198,7 +198,7 @@ end
 -- trimmed in order for the tests not to depend on line numbers of
 -- the source files, which may slip into a .result file.
 local function portable_error(err)
-    return {code = err.code, type = err.type, message = err.message}
+    return {type = err.type, message = err.message}
 end
 
 return {
diff --git a/test/misc/check_uuid_on_connect.result b/test/misc/check_uuid_on_connect.result
index 6ebc5d0..8862e62 100644
--- a/test/misc/check_uuid_on_connect.result
+++ b/test/misc/check_uuid_on_connect.result
@@ -45,7 +45,6 @@ res, util.portable_error(err)
 ---
 - null
 - type: ClientError
-  code: 77
   message: Connection closed
 ...
 test_run:grep_log('bad_uuid_1_a', 'Mismatch server UUID on replica bad_uuid_2_a%(storage%@')
@@ -174,7 +173,6 @@ res, util.portable_error(err)
 ---
 - null
 - type: ClientError
-  code: 77
   message: Connection closed
 ...
 -- Close existing connection on a first error and log it.
diff --git a/test/rebalancer/bucket_ref.result b/test/rebalancer/bucket_ref.result
index 9df7480..67ae5bc 100644
--- a/test/rebalancer/bucket_ref.result
+++ b/test/rebalancer/bucket_ref.result
@@ -144,7 +144,6 @@ res, util.portable_error(err)
 ---
 - null
 - type: ClientError
-  code: 32
   message: Timeout exceeded
 ...
 vshard.storage.buckets_info(1)
diff --git a/test/rebalancer/receiving_bucket.result b/test/rebalancer/receiving_bucket.result
index ad93445..ae1da58 100644
--- a/test/rebalancer/receiving_bucket.result
+++ b/test/rebalancer/receiving_bucket.result
@@ -167,7 +167,6 @@ res, util.portable_error(err)
 ---
 - null
 - type: ClientError
-  code: 32
   message: Error injection 'the bucket is received partially'
 ...
 box.space._bucket:get{1}
@@ -225,7 +224,6 @@ _, err = vshard.storage.bucket_send(101, util.replicasets[1], {timeout = 0.1})
 util.portable_error(err)
 ---
 - type: ClientError
-  code: 78
   message: Timeout exceeded
 ...
 box.space._bucket:get{101}
@@ -332,7 +330,6 @@ ret, util.portable_error(err)
 ---
 - null
 - type: ClientError
-  code: 78
   message: Timeout exceeded
 ...
 finish_long_thing = true
diff --git a/test/router/retry_reads.result b/test/router/retry_reads.result
index fa38541..e5e721a 100644
--- a/test/router/retry_reads.result
+++ b/test/router/retry_reads.result
@@ -118,7 +118,6 @@ fiber.time() - start < 1
 util.portable_error(e)
 ---
 - type: ClientError
-  code: 0
   message: Unknown error
 ...
 _, e = rs1:callro('sleep', {1}, {timeout = 0.0001})
@@ -127,7 +126,6 @@ _, e = rs1:callro('sleep', {1}, {timeout = 0.0001})
 util.portable_error(e)
 ---
 - type: ClientError
-  code: 78
   message: Timeout exceeded
 ...
 --
diff --git a/test/router/router.result b/test/router/router.result
index 8ddbe6d..f9bf649 100644
--- a/test/router/router.result
+++ b/test/router/router.result
@@ -259,7 +259,6 @@ _, e = vshard.router.callro(1, 'raise_client_error', {}, {})
 util.portable_error(e)
 ---
 - type: ClientError
-  code: 32
   message: Unknown error
 ...
 _, e = vshard.router.route(1):callro('raise_client_error', {})
@@ -268,7 +267,6 @@ _, e = vshard.router.route(1):callro('raise_client_error', {})
 util.portable_error(e)
 ---
 - type: ClientError
-  code: 0
   message: Unknown error
 ...
 -- Ensure, that despite not working multi-return, it is allowed
@@ -640,7 +638,6 @@ assert(type(err) == 'table')
 util.portable_error(err)
 ---
 - type: ClientError
-  code: 32
   message: Unknown error
 ...
 future:is_ready()
diff --git a/test/router/sync.result b/test/router/sync.result
index 040d611..164861e 100644
--- a/test/router/sync.result
+++ b/test/router/sync.result
@@ -51,7 +51,6 @@ res, err = vshard.router.sync(-1)
 util.portable_error(err)
 ---
 - type: ClientError
-  code: 78
   message: Timeout exceeded
 ...
 res, err = vshard.router.sync(0)
@@ -60,7 +59,6 @@ res, err = vshard.router.sync(0)
 util.portable_error(err)
 ---
 - type: ClientError
-  code: 78
   message: Timeout exceeded
 ...
 --
diff --git a/test/storage/storage.result b/test/storage/storage.result
index acae98f..af48a13 100644
--- a/test/storage/storage.result
+++ b/test/storage/storage.result
@@ -514,7 +514,6 @@ res, err = vshard.storage.bucket_recv(4, util.replicasets[2], {{1000, {{1}}}})
 util.portable_error(err)
 ---
 - type: ClientError
-  code: 36
   message: Space '1000' does not exist
 ...
 while box.space._bucket:get{4} do vshard.storage.recovery_wakeup() fiber.sleep(0.01) end
diff --git a/test/unit/error.result b/test/unit/error.result
index 738cfeb..bb4e0cc 100644
--- a/test/unit/error.result
+++ b/test/unit/error.result
@@ -28,7 +28,6 @@ str = tostring(box_error)
 util.portable_error(json.decode(str))
 ---
 - type: ClientError
-  code: 78
   message: Timeout exceeded
 ...
 vshard_error = lerror.vshard(lerror.code.UNREACHABLE_MASTER, 'uuid', 'reason')
@@ -94,7 +93,6 @@ err = lerror.make(err)
 util.portable_error(err)
 ---
 - type: ClientError
-  code: 32
   message: '[string "function raise_lua_err() assert(false) end "]:1: assertion failed!'
 ...
 --
-- 
2.24.3 (Apple Git-128)


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

* Re: [Tarantool-patches] [PATCH vshard 1/2] router: wrap is_async futures completely
  2021-09-28 23:08 ` [Tarantool-patches] [PATCH vshard 1/2] router: wrap is_async futures completely Vladislav Shpilevoy via Tarantool-patches
@ 2021-09-29  6:18   ` Oleg Babin via Tarantool-patches
  2021-09-30 22:39     ` Vladislav Shpilevoy via Tarantool-patches
  0 siblings, 1 reply; 8+ messages in thread
From: Oleg Babin via Tarantool-patches @ 2021-09-29  6:18 UTC (permalink / raw)
  To: Vladislav Shpilevoy, tarantool-patches

Thanks for your patch! I left three comments below.

On 29.09.2021 02:08, Vladislav Shpilevoy wrote:
> Router used to override :result() method of netbox futures. It is
> needed because user functions are called via vshard.storage.call()
> which returns some metadata - it must be truncated before
> returning the user's data.
>
> It worked fine while netbox futures were implemented as tables.
> But in the newest Tarantool most of netbox state machine code is
> moved into C. The futures now are cdata.
AFAIK userdata (not cdata) if it makes sense.
> They allow to add new members, but can't override their methods.
> As a result, on the newest Tarantool is_async in
> vshard.router.call() simply didn't work.
>
> The patch wraps netbox futures completely with a Lua table, not
> just overrides one method. Now it works the same on all Tarantool
> versions starting from 1.10.
>
> Closes #294
> ---
>   test/lua_libs/storage_template.lua |  11 ++
>   test/router/router.result          | 301 ++++++++++++++++++++++++++++-
>   test/router/router.test.lua        | 120 +++++++++++-
>   vshard/router/init.lua             | 148 ++++++++++----
>   4 files changed, 542 insertions(+), 38 deletions(-)
>
> diff --git a/test/lua_libs/storage_template.lua b/test/lua_libs/storage_template.lua
> index 8df89f6..83ea710 100644
> --- a/test/lua_libs/storage_template.lua
...
> +
>   --
>   -- Test errors from router call.
>   --
> diff --git a/vshard/router/init.lua b/vshard/router/init.lua
> index 3d468f3..42de740 100644
> --- a/vshard/router/init.lua
> +++ b/vshard/router/init.lua
> @@ -470,6 +470,114 @@ end
>   -- API
>   --------------------------------------------------------------------------------
>   

I thing following changes could be moved into sepearate file.

> +local function vshard_future_tostring(self)
> +    return 'vshard.net.box.request'
> +end
> +
> +local function vshard_future_serialize(self)
> +    -- Drop the metatable. It is also copied and if returned as is leads to
> +    -- recursive serialization.
> +    local s = setmetatable(table.deepcopy(self), {})
> +    s._base = nil
> +    return s
> +end
> +
> +local function vshard_future_is_ready(self)
> +    return self._base:is_ready()
> +end
> +
> +local function vshard_future_wrap_result(res)
> +    local storage_ok, res, err = unpack(res)

Unpack could be replaced with ... = res[1], res[2], res[3].

Should be a bit faster since unpack can't be jitted.

> +    if storage_ok then
> +        if res == nil and err ~= nil then
> +            return nil, lerror.make(err)
> +        end
> +        return setmetatable({res}, seq_serializer)
> +    end
> +    return nil, lerror.make(res)
> +end
> +
> +local function vshard_future_result(self)
> +    local res, err = self._base:result()
> +    if res == nil then
> +        return nil, lerror.make(err)
> +    end
> +    return vshard_future_wrap_result(res)
> +end
> +
> +local function vshard_future_wait_result(self, timeout)
> +    local res, err = self._base:wait_result(timeout)
> +    if res == nil then
> +        return nil, lerror.make(err)
> +    end
> +    return vshard_future_wrap_result(res)
> +end
> +
> +local function vshard_future_discard(self)
> +    return self._base:discard()
> +end
> +
> +local function vshard_future_iter_next(iter, i)
> +    local res, err
> +    local base_next = iter.base_next
> +    local base_req = iter.base_req
> +    local base = iter.base
> +    -- Need to distinguish the last response from the pushes. Because the former
> +    -- has metadata returned by vshard.storage.call().
> +    -- At the same time there is no way to check if the base pairs() did its
> +    -- last iteration except calling its next() function again.
> +    -- This, in turn, might lead to a block if the result is not ready yet.
> +    i, res = base_next(base, i)
> +    -- To avoid that there is a 2-phase check.
> +    -- If the request isn't finished after first next(), it means the result is
> +    -- not received. This is a push. Return as is.
> +    -- If the request is finished, it is safe to call next() again to check if
> +    -- it ended. It won't block.
> +    local is_done = base_req:is_ready()
> +
> +    if not is_done then
> +        -- Definitely a push. It would be finished if the final result was
> +        -- received.
> +        if i == nil then
> +            return nil, lerror.make(res)
> +        end
> +        return i, res
> +    end
> +    if i == nil then
> +        if res ~= nil then
> +            return i, lerror.make(res)
> +        end
> +        return nil, nil
> +    end
> +    -- Will not block because the request is already finished.
> +    if base_next(base, i) == nil then
> +        res, err = vshard_future_wrap_result(res)
> +        if res ~= nil then
> +            return i, res
> +        end
> +        return i, {nil, lerror.make(err)}
> +    end
> +    return i, res
> +end
> +
> +local function vshard_future_pairs(self, timeout)
> +    local next_f, iter, i = self._base:pairs(timeout)
> +    return vshard_future_iter_next,
> +           {base = iter, base_req = self, base_next = next_f}, i
> +end
> +
> +local vshard_future_mt = {
> +    __tostring = vshard_future_tostring,
> +    __serialize = vshard_future_serialize,
> +    __index = {
> +        is_ready = vshard_future_is_ready,
> +        result = vshard_future_result,
> +        wait_result = vshard_future_wait_result,
> +        discard = vshard_future_discard,
> +        pairs = vshard_future_pairs,
> +    }
> +}
> +
>   --
>   -- Since 1.10 netbox supports flag 'is_async'. Given this flag, a
>   -- request result is returned immediately in a form of a future
> @@ -482,41 +590,9 @@ end
>   -- So vshard.router.call should wrap a future object with its own
>   -- unpacker of result.
>   --
> --- Unpack a result given from a future object from
> --- vshard.storage.call. If future returns [status, result, ...]
> --- this function returns [result]. Or a classical couple
> --- nil, error.
> ---
> -function future_storage_call_result(self)
> -    local res, err = self:base_result()
> -    if not res then
> -        return nil, err
> -    end
> -    local storage_call_status, call_status, call_error = unpack(res)
> -    if storage_call_status then
> -        if call_status == nil and call_error ~= nil then
> -            return call_status, call_error
> -        else
> -            return setmetatable({call_status}, seq_serializer)
> -        end
> -    end
> -    return nil, call_status
> -end
> -
> ---
> --- Given a netbox future object, redefine its 'result' method.
> --- It is impossible to just create a new signle metatable per
> --- the module as a copy of original future's one because it has
> --- some upvalues related to the netbox connection.
> ---
> -local function wrap_storage_call_future(future)
> -    -- Base 'result' below is got from __index metatable under the
> -    -- hood. But __index is used only when a table has no such a
> -    -- member in itself. So via adding 'result' as a member to a
> -    -- future object its __index.result can be redefined.
> -    future.base_result = future.result
> -    future.result = future_storage_call_result
> -    return future
> +local function vshard_future_new(future)
> +    -- Use '_' as a prefix so as users could use all normal names.
> +    return setmetatable({_base = future}, vshard_future_mt)
>   end
>   
>   -- Perform shard operation
> @@ -574,7 +650,7 @@ local function router_call_impl(router, bucket_id, mode, prefer_replica,
>                       -- values: true/false and func result. But
>                       -- async returns future object. No true/false
>                       -- nor func result. So return the first value.
> -                    return wrap_storage_call_future(storage_call_status)
> +                    return vshard_future_new(storage_call_status)
>                   end
>               end
>               err = lerror.make(call_status)

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

* Re: [Tarantool-patches] [PATCH vshard 2/2] test: drop error codes from test output
  2021-09-28 23:08 ` [Tarantool-patches] [PATCH vshard 2/2] test: drop error codes from test output Vladislav Shpilevoy via Tarantool-patches
@ 2021-09-29  6:18   ` Oleg Babin via Tarantool-patches
  0 siblings, 0 replies; 8+ messages in thread
From: Oleg Babin via Tarantool-patches @ 2021-09-29  6:18 UTC (permalink / raw)
  To: Vladislav Shpilevoy, tarantool-patches

Thanks for your patch. LGTM.

On 29.09.2021 02:08, Vladislav Shpilevoy wrote:
> In the newest Tarantool the error objects are encoded as MP_ERROR
> in netbox connections by default. This allows to transfer their
> full data including correct error code, even when returned as
> nil,err instead of throwing the error as an exception.
>
> Because of that some vshard tests failed - they expected the error
> codes returned from the server to be wrong.
>
> On the other hand, the codes can't be fixed, because then the
> tests wouldn't work on 1.10.
>
> The patch just drops the codes. At least while need to support
> Tarantool versions not using MP_ERROR in netbox by default.
> ---
>   test/lua_libs/util.lua                  | 2 +-
>   test/misc/check_uuid_on_connect.result  | 2 --
>   test/rebalancer/bucket_ref.result       | 1 -
>   test/rebalancer/receiving_bucket.result | 3 ---
>   test/router/retry_reads.result          | 2 --
>   test/router/router.result               | 3 ---
>   test/router/sync.result                 | 2 --
>   test/storage/storage.result             | 1 -
>   test/unit/error.result                  | 2 --
>   9 files changed, 1 insertion(+), 17 deletions(-)
>
> diff --git a/test/lua_libs/util.lua b/test/lua_libs/util.lua
> index 9c2e667..fe23065 100644
> --- a/test/lua_libs/util.lua
> +++ b/test/lua_libs/util.lua
> @@ -198,7 +198,7 @@ end
>   -- trimmed in order for the tests not to depend on line numbers of
>   -- the source files, which may slip into a .result file.
>   local function portable_error(err)
> -    return {code = err.code, type = err.type, message = err.message}
> +    return {type = err.type, message = err.message}
>   end
>   
>   return {
> diff --git a/test/misc/check_uuid_on_connect.result b/test/misc/check_uuid_on_connect.result
> index 6ebc5d0..8862e62 100644
> --- a/test/misc/check_uuid_on_connect.result
> +++ b/test/misc/check_uuid_on_connect.result
> @@ -45,7 +45,6 @@ res, util.portable_error(err)
>   ---
>   - null
>   - type: ClientError
> -  code: 77
>     message: Connection closed
>   ...
>   test_run:grep_log('bad_uuid_1_a', 'Mismatch server UUID on replica bad_uuid_2_a%(storage%@')
> @@ -174,7 +173,6 @@ res, util.portable_error(err)
>   ---
>   - null
>   - type: ClientError
> -  code: 77
>     message: Connection closed
>   ...
>   -- Close existing connection on a first error and log it.
> diff --git a/test/rebalancer/bucket_ref.result b/test/rebalancer/bucket_ref.result
> index 9df7480..67ae5bc 100644
> --- a/test/rebalancer/bucket_ref.result
> +++ b/test/rebalancer/bucket_ref.result
> @@ -144,7 +144,6 @@ res, util.portable_error(err)
>   ---
>   - null
>   - type: ClientError
> -  code: 32
>     message: Timeout exceeded
>   ...
>   vshard.storage.buckets_info(1)
> diff --git a/test/rebalancer/receiving_bucket.result b/test/rebalancer/receiving_bucket.result
> index ad93445..ae1da58 100644
> --- a/test/rebalancer/receiving_bucket.result
> +++ b/test/rebalancer/receiving_bucket.result
> @@ -167,7 +167,6 @@ res, util.portable_error(err)
>   ---
>   - null
>   - type: ClientError
> -  code: 32
>     message: Error injection 'the bucket is received partially'
>   ...
>   box.space._bucket:get{1}
> @@ -225,7 +224,6 @@ _, err = vshard.storage.bucket_send(101, util.replicasets[1], {timeout = 0.1})
>   util.portable_error(err)
>   ---
>   - type: ClientError
> -  code: 78
>     message: Timeout exceeded
>   ...
>   box.space._bucket:get{101}
> @@ -332,7 +330,6 @@ ret, util.portable_error(err)
>   ---
>   - null
>   - type: ClientError
> -  code: 78
>     message: Timeout exceeded
>   ...
>   finish_long_thing = true
> diff --git a/test/router/retry_reads.result b/test/router/retry_reads.result
> index fa38541..e5e721a 100644
> --- a/test/router/retry_reads.result
> +++ b/test/router/retry_reads.result
> @@ -118,7 +118,6 @@ fiber.time() - start < 1
>   util.portable_error(e)
>   ---
>   - type: ClientError
> -  code: 0
>     message: Unknown error
>   ...
>   _, e = rs1:callro('sleep', {1}, {timeout = 0.0001})
> @@ -127,7 +126,6 @@ _, e = rs1:callro('sleep', {1}, {timeout = 0.0001})
>   util.portable_error(e)
>   ---
>   - type: ClientError
> -  code: 78
>     message: Timeout exceeded
>   ...
>   --
> diff --git a/test/router/router.result b/test/router/router.result
> index 8ddbe6d..f9bf649 100644
> --- a/test/router/router.result
> +++ b/test/router/router.result
> @@ -259,7 +259,6 @@ _, e = vshard.router.callro(1, 'raise_client_error', {}, {})
>   util.portable_error(e)
>   ---
>   - type: ClientError
> -  code: 32
>     message: Unknown error
>   ...
>   _, e = vshard.router.route(1):callro('raise_client_error', {})
> @@ -268,7 +267,6 @@ _, e = vshard.router.route(1):callro('raise_client_error', {})
>   util.portable_error(e)
>   ---
>   - type: ClientError
> -  code: 0
>     message: Unknown error
>   ...
>   -- Ensure, that despite not working multi-return, it is allowed
> @@ -640,7 +638,6 @@ assert(type(err) == 'table')
>   util.portable_error(err)
>   ---
>   - type: ClientError
> -  code: 32
>     message: Unknown error
>   ...
>   future:is_ready()
> diff --git a/test/router/sync.result b/test/router/sync.result
> index 040d611..164861e 100644
> --- a/test/router/sync.result
> +++ b/test/router/sync.result
> @@ -51,7 +51,6 @@ res, err = vshard.router.sync(-1)
>   util.portable_error(err)
>   ---
>   - type: ClientError
> -  code: 78
>     message: Timeout exceeded
>   ...
>   res, err = vshard.router.sync(0)
> @@ -60,7 +59,6 @@ res, err = vshard.router.sync(0)
>   util.portable_error(err)
>   ---
>   - type: ClientError
> -  code: 78
>     message: Timeout exceeded
>   ...
>   --
> diff --git a/test/storage/storage.result b/test/storage/storage.result
> index acae98f..af48a13 100644
> --- a/test/storage/storage.result
> +++ b/test/storage/storage.result
> @@ -514,7 +514,6 @@ res, err = vshard.storage.bucket_recv(4, util.replicasets[2], {{1000, {{1}}}})
>   util.portable_error(err)
>   ---
>   - type: ClientError
> -  code: 36
>     message: Space '1000' does not exist
>   ...
>   while box.space._bucket:get{4} do vshard.storage.recovery_wakeup() fiber.sleep(0.01) end
> diff --git a/test/unit/error.result b/test/unit/error.result
> index 738cfeb..bb4e0cc 100644
> --- a/test/unit/error.result
> +++ b/test/unit/error.result
> @@ -28,7 +28,6 @@ str = tostring(box_error)
>   util.portable_error(json.decode(str))
>   ---
>   - type: ClientError
> -  code: 78
>     message: Timeout exceeded
>   ...
>   vshard_error = lerror.vshard(lerror.code.UNREACHABLE_MASTER, 'uuid', 'reason')
> @@ -94,7 +93,6 @@ err = lerror.make(err)
>   util.portable_error(err)
>   ---
>   - type: ClientError
> -  code: 32
>     message: '[string "function raise_lua_err() assert(false) end "]:1: assertion failed!'
>   ...
>   --

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

* Re: [Tarantool-patches] [PATCH vshard 1/2] router: wrap is_async futures completely
  2021-09-29  6:18   ` Oleg Babin via Tarantool-patches
@ 2021-09-30 22:39     ` Vladislav Shpilevoy via Tarantool-patches
  2021-10-01  6:38       ` Oleg Babin via Tarantool-patches
  0 siblings, 1 reply; 8+ messages in thread
From: Vladislav Shpilevoy via Tarantool-patches @ 2021-09-30 22:39 UTC (permalink / raw)
  To: Oleg Babin, tarantool-patches

Hi! Thanks for the review!

On 29.09.2021 08:18, Oleg Babin wrote:
> Thanks for your patch! I left three comments below.
> 
> On 29.09.2021 02:08, Vladislav Shpilevoy wrote:
>> Router used to override :result() method of netbox futures. It is
>> needed because user functions are called via vshard.storage.call()
>> which returns some metadata - it must be truncated before
>> returning the user's data.
>>
>> It worked fine while netbox futures were implemented as tables.
>> But in the newest Tarantool most of netbox state machine code is
>> moved into C. The futures now are cdata.
> AFAIK userdata (not cdata) if it makes sense.

Changed to 'C structs'.

>> diff --git a/vshard/router/init.lua b/vshard/router/init.lua
>> index 3d468f3..42de740 100644
>> --- a/vshard/router/init.lua
>> +++ b/vshard/router/init.lua
>> @@ -470,6 +470,114 @@ end
>>   -- API
>>   --------------------------------------------------------------------------------
>>   
> 
> I thing following changes could be moved into sepearate file.

I thought of that, but decided it won't improve much. These futures are
a part of router_call, they can't be used for anything else. Because
are specific to how the storage answers to router_call.

They won't work for replicaset:call() nor for router_map_callrw()

So by moving them out I won't make it easier to reuse them or make
them more independent. I will just reduce init.lua file size, which
is not a primary goal.

I was rather hoping to move the entire router_call() into its own file
eventually with all its satellite structures and functions used only
by it.

Does it make sense?

>> +local function vshard_future_tostring(self)
>> +    return 'vshard.net.box.request'
>> +end
>> +
>> +local function vshard_future_serialize(self)
>> +    -- Drop the metatable. It is also copied and if returned as is leads to
>> +    -- recursive serialization.
>> +    local s = setmetatable(table.deepcopy(self), {})
>> +    s._base = nil
>> +    return s
>> +end
>> +
>> +local function vshard_future_is_ready(self)
>> +    return self._base:is_ready()
>> +end
>> +
>> +local function vshard_future_wrap_result(res)
>> +    local storage_ok, res, err = unpack(res)
> 
> Unpack could be replaced with ... = res[1], res[2], res[3].
> 
> Should be a bit faster since unpack can't be jitted.

Nice, didn't know that.

====================
diff --git a/vshard/router/init.lua b/vshard/router/init.lua
index 42de740..4748398 100644
--- a/vshard/router/init.lua
+++ b/vshard/router/init.lua
@@ -487,7 +487,7 @@ local function vshard_future_is_ready(self)
 end
 
 local function vshard_future_wrap_result(res)
-    local storage_ok, res, err = unpack(res)
+    local storage_ok, res, err = res[1], res[2], res[3]
     if storage_ok then
         if res == nil and err ~= nil then
             return nil, lerror.make(err)

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

* Re: [Tarantool-patches] [PATCH vshard 1/2] router: wrap is_async futures completely
  2021-09-30 22:39     ` Vladislav Shpilevoy via Tarantool-patches
@ 2021-10-01  6:38       ` Oleg Babin via Tarantool-patches
  0 siblings, 0 replies; 8+ messages in thread
From: Oleg Babin via Tarantool-patches @ 2021-10-01  6:38 UTC (permalink / raw)
  To: Vladislav Shpilevoy, tarantool-patches

Hi! Thanks for your changes. LGTM.

On 01.10.2021 01:39, Vladislav Shpilevoy wrote:
> Hi! Thanks for the review!
>
> On 29.09.2021 08:18, Oleg Babin wrote:
>> Thanks for your patch! I left three comments below.
>>
>> On 29.09.2021 02:08, Vladislav Shpilevoy wrote:
>>> Router used to override :result() method of netbox futures. It is
>>> needed because user functions are called via vshard.storage.call()
>>> which returns some metadata - it must be truncated before
>>> returning the user's data.
>>>
>>> It worked fine while netbox futures were implemented as tables.
>>> But in the newest Tarantool most of netbox state machine code is
>>> moved into C. The futures now are cdata.
>> AFAIK userdata (not cdata) if it makes sense.
> Changed to 'C structs'.
>
>>> diff --git a/vshard/router/init.lua b/vshard/router/init.lua
>>> index 3d468f3..42de740 100644
>>> --- a/vshard/router/init.lua
>>> +++ b/vshard/router/init.lua
>>> @@ -470,6 +470,114 @@ end
>>>    -- API
>>>    --------------------------------------------------------------------------------
>>>    
>> I thing following changes could be moved into sepearate file.
> I thought of that, but decided it won't improve much. These futures are
> a part of router_call, they can't be used for anything else. Because
> are specific to how the storage answers to router_call.
>
> They won't work for replicaset:call() nor for router_map_callrw()
>
> So by moving them out I won't make it easier to reuse them or make
> them more independent. I will just reduce init.lua file size, which
> is not a primary goal.
>
> I was rather hoping to move the entire router_call() into its own file
> eventually with all its satellite structures and functions used only
> by it.
>
> Does it make sense?

I thought about simplification, not about reusing. As for me it's simpler to

keep some logic into separate file. But I won't insist.


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

* Re: [Tarantool-patches] [PATCH vshard 0/2] VShard new netbox
  2021-09-28 23:08 [Tarantool-patches] [PATCH vshard 0/2] VShard new netbox Vladislav Shpilevoy via Tarantool-patches
  2021-09-28 23:08 ` [Tarantool-patches] [PATCH vshard 1/2] router: wrap is_async futures completely Vladislav Shpilevoy via Tarantool-patches
  2021-09-28 23:08 ` [Tarantool-patches] [PATCH vshard 2/2] test: drop error codes from test output Vladislav Shpilevoy via Tarantool-patches
@ 2021-10-01 21:14 ` Vladislav Shpilevoy via Tarantool-patches
  2 siblings, 0 replies; 8+ messages in thread
From: Vladislav Shpilevoy via Tarantool-patches @ 2021-10-01 21:14 UTC (permalink / raw)
  To: tarantool-patches, olegrok

Pushed to master.

On 29.09.2021 01:08, Vladislav Shpilevoy via Tarantool-patches wrote:
> The patch makes vshard support new netbox: is_async and proper errors which are
> now returned from the server as MP_ERROR by default (in netbox at least).
> 
> Branch: http://github.com/tarantool/vshard/tree/gerold103/gh-294-is_async
> Issue: https://github.com/tarantool/vshard/issues/294
> 
> Vladislav Shpilevoy (2):
>   router: wrap is_async futures completely
>   test: drop error codes from test output
> 
>  test/lua_libs/storage_template.lua      |  11 +
>  test/lua_libs/util.lua                  |   2 +-
>  test/misc/check_uuid_on_connect.result  |   2 -
>  test/rebalancer/bucket_ref.result       |   1 -
>  test/rebalancer/receiving_bucket.result |   3 -
>  test/router/retry_reads.result          |   2 -
>  test/router/router.result               | 304 +++++++++++++++++++++++-
>  test/router/router.test.lua             | 120 +++++++++-
>  test/router/sync.result                 |   2 -
>  test/storage/storage.result             |   1 -
>  test/unit/error.result                  |   2 -
>  vshard/router/init.lua                  | 148 +++++++++---
>  12 files changed, 543 insertions(+), 55 deletions(-)
> 

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

end of thread, other threads:[~2021-10-01 21:14 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-28 23:08 [Tarantool-patches] [PATCH vshard 0/2] VShard new netbox Vladislav Shpilevoy via Tarantool-patches
2021-09-28 23:08 ` [Tarantool-patches] [PATCH vshard 1/2] router: wrap is_async futures completely Vladislav Shpilevoy via Tarantool-patches
2021-09-29  6:18   ` Oleg Babin via Tarantool-patches
2021-09-30 22:39     ` Vladislav Shpilevoy via Tarantool-patches
2021-10-01  6:38       ` Oleg Babin via Tarantool-patches
2021-09-28 23:08 ` [Tarantool-patches] [PATCH vshard 2/2] test: drop error codes from test output Vladislav Shpilevoy via Tarantool-patches
2021-09-29  6:18   ` Oleg Babin via Tarantool-patches
2021-10-01 21:14 ` [Tarantool-patches] [PATCH vshard 0/2] VShard new netbox 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