[Tarantool-patches] [PATCH 1/1] box: export box_session_push to the public C API

Alexander Turenko alexander.turenko at tarantool.org
Wed Jan 22 05:25:38 MSK 2020


> > I see that box_return_tuple() receives (box_function_ctx_t *) as the
> > first argument and wonder why box_session_push() does not. If we able to
> > determine where to and how to send a push w/o the context, then the
> > context is not necessary in box_return_tuple() too? I don't very
> > familiar with this part of codebase, so it is just question for now.
> > I'll look around soon if time permits.
> 
> Context of session push is a session. It is available in fiber, which
> is a global variable.
> 
> Context of a function is its port - a storage for returned values. Port
> is not available in the fiber, and therefore is not stored anywhere
> globally. It needs to be passed explicitly. We pass it as an opaque
> pointer box_function_ctx_t*.
> 
> We can add a context to the push, but then we need to create a public
> version of struct session. Something like 'box_session_t', which would
> be an alias for 'struct session'. And have a function like
> 'box_session_current()', which in turn won't have a context.
> 
> I think we need to introduce box_session_t only if we are going to
> allow users to access foreign sessions. Otherwise it is always the
> user's own session and is available in the fiber anyway.

But box_return_tuple() can create a port on demand, just like you do in
box_session_push(), right? I don't see any difference between those
functions in context of the question.

If box_return_tuple() could be implemented with or without explicit
(box_function_ctx_t *) argument in the past, then someone made the
decision to implement it with the argument and there is (hopefully) some
reasoning under this decision. If so, it may be applicable to
box_session_push(). Or there is a reason why it is not applicable here.

> +	/*
> +	 * Create a new thread and pop it immediately from the
> +	 * main stack. So as it would not stay there in case
> +	 * something would throw in this function.
> +	 */
> +	int coro_ref = luaL_ref(tarantool_L, LUA_REGISTRYINDEX);
> +	/*
> +	 * MessagePack -> Lua -> YAML. The middle is not really
> +	 * needed here, but there is no MessagePack -> YAML
> +	 * encoder yet.
> +	 */
> +	const char *data = port->data;
> +	luamp_decode(L, luaL_msgpack_default, &data);
> +	int rc = lua_yaml_encode(L, luaL_yaml_default, "!push!",
> +				 "tag:tarantool.io/push,2018");
> ================================================================================
> 
> luamp_decode and lua_yaml_encode can throw a Lua error. Don't know how to
> fix that except introduction of a MessagePack -> YAML converter, which seems
> a pretty big and independent task.
> 
> ================================================================================

It seems that it is usually handled with lua_pushcfunction() /
lua_pushcclosure() + lua_pcall().


More information about the Tarantool-patches mailing list