tarantool / vshard

The new generation of sharding based on virtual buckets
Other
100 stars 30 forks source link

router: support return_raw in map_callrw() #318

Closed Gerold103 closed 2 years ago

Gerold103 commented 2 years ago

Closes #312

@TarantoolBot document Title: vshard.router.map_callrw() format of return_raw result

vshard.router.map_callrw() supports return_raw option (in the version of Tarantool which have it at all).

When there was an error, the result is the same as always: nil, err, err_uuid. No changes.

When the call was a success, the result is a map of the format {[replicaset_uuid] = msgpack.object}. The msgpack.object here stores a MessagePack array with the results returned from the storage's map-function.

The usecase is the same as for when bare netbox is used - to avoid decoding of the results into Lua land. Helpful when the router is used as a proxy and the results received from the storage are big.

Example:

local res = vshard.router.map_callrw('my_func', args, {..., return_raw = true})

for replicaset_uuid, msgpack_value in pairs(res) do
    log.info('Replicaset %s returned %s', replicaset_uuid,
             msgpack_value:decode())
end

However this is just an example. Normally you don't need return_raw if you are calling :decode() anyway.

The family of vshard.router.call functions also supports return_raw, but their behaviour is the same as with bare netbox - the entire result is just an msgpack.object. The same as if direct netbox call would be done. Nothing to document here, I think.

R-omk commented 2 years ago

Does it really should close an issue? Are there any plans to implement callraw API?

@olegrok , for the most part, this ticket was discussed here

olegrok commented 2 years ago

Does it really should close an issue? Are there any plans to implement callraw API?

@olegrok , for the most part, this ticket was discussed here

I read this discussion. But anyway there should be a way to take raw args on storage side. It could be useful for modules such a crud. Since box API allows to insert msgpack objects into spaces.

R-omk commented 2 years ago

to take raw args on storage side

But this has nothing to do with vshard. Vshard simply calls functions, while the user himself can configure the function options (a function can be registered in box.schema.func with takes_raw_args = true)

olegrok commented 2 years ago

to take raw args on storage side

But this has nothing to do with vshard. Vshard simply calls functions, while the user himself can configure the function options (a function can be registered in box.schema.func with takes_raw_args = true)

Hmm... I was sure that vshard proxies all calls from "vshard.storage.*" function. To be able to call ref on storage.

https://github.com/tarantool/vshard/blob/4962609d52d4a8935407cef75024f93051ea122b/vshard/storage/init.lua#L2504-L2514

Even I create a function with "takes_raw_args = true" it shouldn't have any effect.

R-omk commented 2 years ago

@olegrok, you're right.

Gerold103 commented 2 years ago

Thanks for your patch. LGTM.

Single question:

Closes #312

Should it really close an issue? Are there any plans to implement callraw API?

312 was about the router only. The callraw mentioned in the discussion was about having it on the router. vshard.router.callraw would still call vshard.storage.call on the storage with normal arguments, but callraw itself would take an msgpack object with an array of the same arguments as vshard.router.call except that the function arguments would be in the end.

This idea was discarded, because it would require you to either grant universe execution rights to connections to the router (unsafe) or call box.schema.func.create('vshard.router.callraw', {takes_raw_args = true}) on the router. Router can't do the former action itself, because it should not depend on schema anyhow. Thus you either way need to do actions on the router. Meaning you can simply implement callraw as well as register it. Moreover, it can be done more optimal when you make it for a specific application.

As for having msgpack object on the storage - TBH I am quite disappointed in msgpack object performance when it comes to not so big number of values in it. Nonetheless we can support it on the storage too if you really need it. In a separate ticket, lets say. Implementation would be rather straight - pass an option to the router like return_raw for getting msgpack object (like done now), and pass_raw to call a new function vshard.storage.callraw. It would call your function with msgpack object as argument.

Totktonada commented 2 years ago

Regarding granting permissions: I found that a module-provided function like module.grant(user_or_role) is quite useful in terms of convenience. One don't need to dig into internals to make things just work.

In this particular case (where we need to register a function and optionally grant permission for it to given user), it can be vshard.router.func.create('callraw'[, user]), where 'callraw' is a predefined name. Or vshard.router.func.callraw.register([user]). Something like this.

Gerold103 commented 2 years ago

https://github.com/tarantool/vshard/issues/319