From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from [87.239.111.99] (localhost [127.0.0.1]) by dev.tarantool.org (Postfix) with ESMTP id 680027030B; Tue, 26 Jan 2021 00:56:33 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 680027030B DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1611611793; bh=43KD7D9kRmzFdXy4LV0b4iMEmw//OjeZnCdXUsI9CBw=; h=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=opEan+GTYY2f7qiK58iS1PA08IE+laq1WH5G73VyHf5Dg2/YC0XYMHhBzy8fNvHZ+ l4uK2AOlP8MqElOvzcAOdH10qZLXOgJnmi+j03LcVrm9KUz8wE/6wk2Tdpl1emBxix SIfCsPPOaTPz2bciBHVIT+VI2/KpypIXM7j5lm+U= Received: from mail-lj1-f175.google.com (mail-lj1-f175.google.com [209.85.208.175]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 4AC917030B for ; Tue, 26 Jan 2021 00:56:32 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 4AC917030B Received: by mail-lj1-f175.google.com with SMTP id f11so17194406ljm.8 for ; Mon, 25 Jan 2021 13:56:32 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=Q9ut9nURL/YzPwi9oIMr4HdA+DygNnj6Ovc4ahY+JPY=; b=KwMiV4s33KSJCtxfV8vRzRQ0/u6y9kPezTk+atamevOKXLmXW0oD1EAJ8kxdbaLONO RriyyxWxilR7K5cNbApMIoYsaqV8LnY/voLTUoyXQLD/Y4tqQdPX895m6V1voyW23F+u Kx2JAo25zaP93ldSuWN+xln4xouvacgG8+T5Sm3A2Q0Li0DspCOMqysBBpVB+gspEyCx LTOYQbCWE41+XZiitC2k9tVeK/+jC3T6mhsVCLrpDL/Fc7H8ShSYUq8V5632lg0WZcio KTQToU+m5o34twr5LVYsRHDpzDhdCCqBV9AaC0bYjN2GgD0S24TRsZc1C+DgxHbNiNUe WuJw== X-Gm-Message-State: AOAM532nivPnX+ApVvsXKkdOjr0ncGvNN9fs0WcKve/rhQyh9ZAXiH8+ fUEcVqW1jmeaJgo1wGHdplQ= X-Google-Smtp-Source: ABdhPJxSh1istYuFOrcHMjsm5d+s/p41Zb4MTXALMcTDZxgcHWGG+3dYE6prZpw7wmTBhZf0UOXJZQ== X-Received: by 2002:a2e:7f1a:: with SMTP id a26mr1227731ljd.202.1611611791699; Mon, 25 Jan 2021 13:56:31 -0800 (PST) Received: from grain.localdomain ([5.18.91.94]) by smtp.gmail.com with ESMTPSA id e20sm2146931lfn.221.2021.01.25.13.56.30 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 25 Jan 2021 13:56:30 -0800 (PST) Received: by grain.localdomain (Postfix, from userid 1000) id 6228A56011F; Tue, 26 Jan 2021 00:56:29 +0300 (MSK) Date: Tue, 26 Jan 2021 00:56:29 +0300 To: Vladislav Shpilevoy Message-ID: <20210125215629.GI2174@grain> References: <20210125203550.GH2174@grain> <9beb6862-b21a-a054-e66d-b744152c1be0@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <9beb6862-b21a-a054-e66d-b744152c1be0@tarantool.org> User-Agent: Mutt/1.14.6 (2020-07-11) Subject: Re: [Tarantool-patches] [RFC] cmod user api X-BeenThere: tarantool-patches@dev.tarantool.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Cyrill Gorcunov via Tarantool-patches Reply-To: Cyrill Gorcunov Cc: Mons Anderson , TML Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" On Mon, Jan 25, 2021 at 10:08:33PM +0100, Vladislav Shpilevoy wrote: > > > > 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. We've been discussing the methods with you and Mons long ago, I even have a draft https://gist.github.com/cyrillos/a0eaa178bfc0d22c1810c59a951421c8 and I agree that RFC should come first but you know in most cases plain RFC is not enough, one have to implement a draft working version which would reveal all corner cases and problems. For example when you've been reviewing one of mine early version you pointed about function duplicates (ie same function loaded twise and we use load_count for this). IOW I mean that RFC is a good pont to start but without draft working version it not that better because implementation details might be a stopper. I already been there - can't reveal the details but idea was brilliant and... uncodable: the problems of implementation ruined everything. That said I think our iterations via implementations here is a very good approach. > > 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. OK. Not critical issue, we even would be able to add such support in future by request. > > 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. All you said is correct the problem is -- existing code, ie a we already have this weird reload() functionality, for functions which are created via box.schema.func. I simply provided a hand for this api. We can hide it and require explicit unload+load pair instead.