[Tarantool-patches] [RFC] cmod user api

[Vladislav Shpilevoy]

Hi!

On 25.01.2021 21:35, Cyrill Gorcunov wrote:
> Hi guys! Recently I've sent a series which implements that
> named "cmod" module which allows to load and run stored C
> procedures similar to 'box.func' except they allows to run
> such functions even on nodes in read-only mode.
> 
> The overall picture of api was not clear from changelog and
> here I provide a short description to gather comments.

It is clear from the changelog. The problem was that the API was
*implemented* before getting approves on it. For big public
features like this it usually leads to huge waste of time and
disappointment of the author when patches get rejected due to
implementing something in a wrong way.

Which is not the case for this version I hope. For me all looks
sane in the API except reload.

> Entry point of any function comes from "module" instance.
> To load a module one have to
> 
>     module = require('cmod').load('path-to-module')
> 
> Note the path to the module should be shipped without .so
> extension (or dylib for macos). I suspect we should support
> both variants with and without file extension.

Nevermind, it is platform-dependent. I think it is fine just like
it works in _func now - file name without an extension.

> Once module is loaded we can associate a function from
> this module with some Lua variable.
> 
>     foo = module:load('name-of-function')
> 
> Then we can execute it
> 
>     res = foo(arguments, ...)
> 
> Both function and module supports explicit unloading via
> unload method.
> 
>     foo:unload()
>     module:unload()
> 
> When function/module is unloaded any attempt to use this
> variable will produce an error. If they are not explicitly
> unloaded then GC will reap them.
> 
> Module Lua object provides :reload() method which re-reads
> the shared library and updates associated functions such than
> any new call will be executed via new code. If there are some
> already executing functions which are in yield state then such
> functions gonna finish execution of old code first and only
> next calls will be pacing the new code.
> 
> Does such api looks sane or there some other ideas?

I will copy-paste here what I said about reloading the existing
function objects under the hood.

Reason for not changing the existing function objects was
that the reload is supposed to happen when the whole
application reloads. It means there can be some fibers
running the old code for a while after reload until they
notice that they must be restarted. If you do reload like
you did in this patch, the old fibers will get new functions
immediately right under their feet, and this may result
into unexpected behaviour for code in these fibers.

For instance, you did this code:

	local func = mod:load('do_replace_and_yield')
	for i, obj in pairs(objs) do
		func(obj)
	end
	func:unload()

Assume 'do_replace_and_yield' yields inside. Also assume that
the loop is started in a separate fiber, and func is called
first time. It yields, and now the whole Lua module, having this
code, is reloaded. This old code still runs in the fiber, created
in the beginning before reload.

During Lua application reload the shared module was also reloaded,
and now 'do_replace_and_yield' expects two arguments instead of 1.
So on the next iteration of the loop 'func' will fail.

The user didn't do anything with 'func' object, but it was changed
inside, and the user couldn't do anything about it. Calling 'load()'
on each iteration is not an option if the loop iterates thousands
of times and more.

All becomes even worse if 'do_replace_and_yield' does accept the
arguments, but its behaviour is changed. For instance, its result
format.

Another way to get the desired behaviour - use unload() + load()
- I assume in this case the old function objects will remain
unchanged, right? But do we really need such 'reload' behaviour
then?

I suggest at least to ask Mons what to do with this.