[Tarantool-patches] [PATCH vshard 1/2] router: wrap is_async futures completely

Oleg Babin olegrok at tarantool.org
Fri Oct 1 09:38:58 MSK 2021

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.

More information about the Tarantool-patches mailing list