[Tarantool-patches] [PATCH vshard 1/2] router: wrap is_async futures completely
Vladislav Shpilevoy
v.shpilevoy at tarantool.org
Fri Oct 1 01:39:22 MSK 2021
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)
More information about the Tarantool-patches
mailing list