From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp3.mail.ru (smtp3.mail.ru [94.100.179.58]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 269F3469719 for ; Fri, 2 Oct 2020 15:23:19 +0300 (MSK) From: "Timur Safin" References: In-Reply-To: Date: Fri, 2 Oct 2020 15:23:17 +0300 Message-ID: <09de01d698b6$cf00bc10$6d023430$@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Content-Language: ru Subject: Re: [Tarantool-patches] [PATCH 2.X 0/7] RFC: module api: extend for external merger Lua module List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: v.shpilevoy@tarantool.org, alexander.turenko@tarantool.org, Igor Munkin , Aleksandr Lyapunov Cc: tarantool-patches@dev.tarantool.org Before getting into all gory details and start to actively participate = in discussion=20 I want to highlight some basic principles which we used while preparing = these simple=20 patches (some are obvious, but some not that much): - I try to offload functionality to the code existing inside of = Tarantool as much as=20 possible. If there are some structures or functions - refer to them. And = this principle=20 better be applied not only to Tarantool core specific functionality, but = also to all 3rd=20 party functions it uses. E.g. small, msgpuck, or even heap. - And current principle to compile against them is simple - we use these = 3rd party headers=20 for compilation of functions in the module, but ignore libraries for a = moment. We delay=20 binding till run-time where system loader would substitute symbol from = Tarantool executable=20 which eventually called us as external module. If we could use msgpuck = or small in this=20 manner then we almost entirely avoiding most ABI incompatibility = problems.=20 - The question is: where to get those 3rd party headers? We may use = several approaches=20 in modules: 1. via cmake ExternalProject_Add to github repo; 2. via git submodule to github repo; 3. via systems package manager dependency and binary dependency (e.g. = apt or rpm). So happened, that in merger we used several of them, e.g. submodule for = small https://github.com/tsafin/tarantool-merge/tree/master/src tarantool-dev system package for msgpuck. And simply copy-pasted heap.h = from salad. [IMVHO, salad worth to be treated similarly as small. For better reuse = we may export It to external repository] 3 different approaches for quite similar cases of code reuse. :) = Eventually we may=20 develop the consistent approach here - how to reuse 3rd party Tarantool = library=20 in ABI stable way, which may be originating of different versions of = Tarantool?=20 (e.g. small in 2 its incarnations from 2.x and 1.10) Would it be always enough to use older version of headers, and run-time = linking with=20 parent executable? At the moment I see no reason to not do so. As far as = I can see=20 small is quite similar from ABI prospective between 1.10 and 2.x at the = moment,=20 we could start away as is, but certainly should add binary compatibility = check somewhere=20 into CI (to small repo, to Tarantool repo, and/or to merger repo) for = future=20 compatibility guarantees. I do assume that small repository was created = just for such usage scenarios like here (module reusing some useful functionality), = and it's=20 correct way to use it as submodule or ExternalProject_ in cmake. We just = need to make sure it will not broken later. Same for msgpuck - it's already created for better code reuse - we just = need to=20 Make sure it's possible to be used in future compatible way. (Via extra = CI here=20 and there). Having said all that, now we could return to patches discussions - see = my=20 other emails... Regards, Timur : -----Original Message----- : From: Timur Safin : Sent: Thursday, September 24, 2020 8:00 PM : To: v.shpilevoy@tarantool.org; alexander.turenko@tarantool.org : Cc: Timur Safin ; tarantool- : patches@dev.tarantool.org : Subject: [PATCH 2.X 0/7] RFC: module api: extend for external merger = Lua : module :=20 : This patchset is a continuation of patch series which Alexander = Turenko : has sent : earlier on. It extends module api with the necessary wrappers we = should : have : in api to make external merger possible. Some internal functionality = which : was acessible to internal merger, should be wrapped via accessors if : merger : is becoming external. :=20 : - most of introduced functions are simple public wrappers around of : internal : functions, e.g. `box_tuple_validate` is a wrapper around : `tuple_validate_raw`, or `bax_key_def_dup` is a wrapper for : `key_def_dup`. : They were necessary to not reveal implementation specific details : (i.e. `tuple_data` needed in `tuple_validate_raw` should know : `box_tuple_t` : layout); : - wherever we used to use `struct tuple *` we replacing them with = public : alias : `box_tuple_t`. Same is for `struct tuple_format` -> = `box_tuple_format_t` : and : other typedefs. :=20 : - eventually merger in Tarantool core might get deprecated and all new : development for merger will continue in version agnostic external = module : thus we decided that common utility functions (e.g. : `luaT_temp_luastate`) : which used to reside in merger to better be sitting in = `src/lua/utils.c` : instead (in addition to becoming public as described above). :=20 : NB! You could observe, that this part of changes for merger in = module_api : is : much, much easier than those for key_def as sent by Sasha Turenko = before. :=20 : Issue: : * https://github.com/tarantool/tarantool/issues/5273 : ('module api: expose everything that is needed for external key_def : module') :=20 : Branches: : * https://github.com/tarantool/tarantool/tree/tsafin/gh-5273-expand- : module-api : (top 7 commits above of 14 @Totktonada's commits) : * https://github.com/tarantool/tarantool/tree/tsafin/gh-5273-expand- : module-api-1.10 : (last 9 commits above of 16 @Totktonada's commits) :=20 : =3D=3D External merger module :=20 : Currently external merger is residing here : https://github.com/tsafin/tarantool-merge : and it uses key_def from https://github.com/Totktonada/key_def = repository : via simple luarocks dependency (we had to introduce debugging = rock-server : to : make this working). :=20 : CI is installed and is running on GitHub Actions, thus we know that = it's : sort of working together, but more extensive testing of merger and = key_def : yet to be proceeded. Though it might be done independently of current : kernel patches. :=20 :=20 : Timur Safin (7): : module api: export box_tuple_validate : module api: export box_key_def_dup : module api: luaT_newthread : module api: luaL_register_module & luaL_register_type : module api: luaT_temp_luastate & luaT_release_temp_luastate : module api: luaL_checkibuf & luaL_checkconstchar : module api: luaL_cdata_iscallable :=20 : src/box/key_def_api.c | 6 +++ : src/box/key_def_api.h | 10 +++++ : src/box/lua/merger.c | 78 ------------------------------------- : src/box/tuple.c | 6 +++ : src/box/tuple.h | 11 ++++++ : src/exports.h | 10 +++++ : src/lua/utils.c | 51 ++++++++++++++++++++++++- : src/lua/utils.h | 89 = ++++++++++++++++++++++++++++++++++++------- : 8 files changed, 169 insertions(+), 92 deletions(-) :=20 : -- : 2.20.1