Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Router is_async is broken on the latest tarantool #294

Closed
Gerold103 opened this issue Sep 16, 2021 · 0 comments
Closed

Router is_async is broken on the latest tarantool #294

Gerold103 opened this issue Sep 16, 2021 · 0 comments
Assignees
Labels
bug Something isn't working router teamS Scaling
Milestone

Comments

@Gerold103
Copy link
Collaborator

After netbox was reworked, vshard router's future:wait_result() got broken. This is because the router tried to replace the result method of future with a wrapper, which would remove the service values internal to the router.

It seems the new netbox future objects do not check their __index for methods, so the methods can't be overridden.

This is my fault, because I relied on a non-documented feature of being able to add stuff to the future object. A fix would be to wrap netbox futures with a Lua table in vshard, which would have special methods. At least the ones which return the request result.

It seems to be affecting only router's public API. Internal code like discovery used raw netbox futures and does not look broken. At least the tests pass. The now broken tests are router/router.test.lua and failover/failover.test.lua.

@Gerold103 Gerold103 added bug Something isn't working router 3sp labels Sep 16, 2021
@Gerold103 Gerold103 added this to the 0.2 milestone Sep 16, 2021
@kyukhin kyukhin added the teamS Scaling label Sep 17, 2021
@Gerold103 Gerold103 self-assigned this Sep 27, 2021
Gerold103 added a commit that referenced this issue Sep 28, 2021
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
ylobankov added a commit that referenced this issue Oct 11, 2021
Issue #294 is fixed and it's time to enable tarantool 2.9 testing.
Gerold103 pushed a commit that referenced this issue Oct 11, 2021
Issue #294 is fixed and it's time to enable tarantool 2.9 testing.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working router teamS Scaling
Projects
None yet
Development

No branches or pull requests

2 participants