From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org> To: Cyrill Gorcunov <gorcunov@gmail.com>, tml <tarantool-patches@dev.tarantool.org> Subject: Re: [Tarantool-patches] [PATCH v10 3/4] box/cbox: implement cbox Lua module Date: Thu, 12 Nov 2020 23:53:45 +0100 [thread overview] Message-ID: <e186c454-6765-4776-6433-f3f791ff4c27@tarantool.org> (raw) In-Reply-To: <20201105151808.456573-4-gorcunov@gmail.com> Thanks for the patch! See 3 comments below. On 05.11.2020 16:18, Cyrill Gorcunov wrote: > Currently to run "C" function from some external module > one have to register it first in "_func" system space. This > is a problem if node is in read-only mode (replica). > > Still people would like to have a way to run such functions > even in ro mode. For this sake we implement "cbox" lua module. > > Fixes #4692 1. Wrong issue number. > Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com> > > @TarantoolBot document > Title: cbox module > > Overview > ======== > > `cbox` module provides a way to create, delete and execute > `C` procedures. Unlinke `box.schema.func` functionality this > the functions created with `cbox` help are not persistent and 2. "functionality this the functions created"? Couldn't parse. "this" seems to be unnecessary. > live purely in memory. Once a node get turned off they are > vanished. An initial purpose for them is to execute them on > nodes which are running in read-only mode. > > Module functions > ================ > > `cbox.func.load([dso.]name) -> obj | nil, err` > ---------------------------------------------- > > Loads a new function with name `name` from module `dso.`. > The module name is optional and if not provided implies > to be the same as `name`. > > The `load` call must be paired with `unload` and these > calls are accounted. Until coupled `unload` is called > the instance is present in memory. Any `load` calls > followed by another `load` with same name simply > increase a reference to the existing function. 3. I thought about it more, and I am not sure unload should be called only manually. Because Lua has exceptions. So the user need to wrap everything into pcall between he called load() and the code calling unload(). Otherwise an exception may arise somewhere, and the user won't get to unload(). As a result, he won't decrease the reference counter. For example: local f = cbox.load('func') box.space.test:insert({f()}) cbox.unload('func') If the insert will throw, unload is never called. Moreover, if the code is called repeatedly, and often fails on insertion, then the reference counter will constantly grow, and number of leaked unload() calls will grow as well. Also it looks ugly that 'unload' is not bound anyhow to the function objects. In Lua we usually provide a method to destroy cdata object content in the object itself. Not in the module function-set. For example, look at fio. When we create a file handle via fh = fio.open('file_name') , we then call fh:close() , not fio.close('file_name') Also if :close() is not called, and 'fh' is garbage-collected, it is closed automatically. Your API for cbox looks exactly like the second case. While IMO it should look like the first one. Otherwise the users will leak the modules, 100%. On the other hand, if we will unload the function when its last Lua object is deleted, then we may get oscillation in case the user frequently gets the function object using cbox.load(). So maybe even if the last function is unloaded, we must keep the module loaded, but with 0 reference counter. This, in turn, may lead to the module never being deleted, if the user will not ever use it or reload. I don't think I have a ready to use solution for that. I have a raw idea how to resolve that. We can separate function load and function get from each other. So cbox.load() only does the load and returns nothing. And to actually get the function we add something like cbox.get/fetch. Or we can remove the load reference counting. So the users will be forced not to call it too often and track the usages more carefully. Or load the functions one time in a global namespace, and unload them also only one time when the script is re-loaded, for example. I advise to ask somebody who uses C stored functions in real code, how is it usually handled. Solution team should have experience, I hear they discuss C functions sometimes. Also Mons and Igor should now good patterns for this, because it is about Lua as well.
next prev parent reply other threads:[~2020-11-12 22:53 UTC|newest] Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-11-05 15:18 [Tarantool-patches] [PATCH v10 0/4] box/cbox: implement cfunc " Cyrill Gorcunov 2020-11-05 15:18 ` [Tarantool-patches] [PATCH v10 1/4] box/func: factor out c function entry structure Cyrill Gorcunov 2020-11-12 22:53 ` Vladislav Shpilevoy 2020-11-13 17:56 ` Cyrill Gorcunov 2020-11-05 15:18 ` [Tarantool-patches] [PATCH v10 2/4] module_cache: move module handling into own subsystem Cyrill Gorcunov 2020-11-12 22:54 ` Vladislav Shpilevoy 2020-11-16 9:54 ` Cyrill Gorcunov 2020-11-16 14:41 ` Cyrill Gorcunov 2020-11-05 15:18 ` [Tarantool-patches] [PATCH v10 3/4] box/cbox: implement cbox Lua module Cyrill Gorcunov 2020-11-12 22:53 ` Vladislav Shpilevoy [this message] 2020-11-16 20:26 ` Cyrill Gorcunov 2020-11-05 15:18 ` [Tarantool-patches] [PATCH v10 4/4] test: box/cfunc -- add simple module test Cyrill Gorcunov 2020-11-12 22:53 ` [Tarantool-patches] [PATCH v10 0/4] box/cbox: implement cfunc Lua module Vladislav Shpilevoy 2020-11-13 17:54 ` Cyrill Gorcunov
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=e186c454-6765-4776-6433-f3f791ff4c27@tarantool.org \ --to=v.shpilevoy@tarantool.org \ --cc=gorcunov@gmail.com \ --cc=tarantool-patches@dev.tarantool.org \ --subject='Re: [Tarantool-patches] [PATCH v10 3/4] box/cbox: implement cbox Lua module' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox