From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id 13EC726E27 for ; Tue, 20 Aug 2019 19:15:49 -0400 (EDT) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id FU3Eg6UyTkxj for ; Tue, 20 Aug 2019 19:15:49 -0400 (EDT) Received: from smtp16.mail.ru (smtp16.mail.ru [94.100.176.153]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id 630AF26DE1 for ; Tue, 20 Aug 2019 19:15:48 -0400 (EDT) Date: Wed, 21 Aug 2019 02:15:28 +0300 From: Alexander Turenko Subject: [tarantool-patches] Re: [PATCH] tarantoolctl doesn't fail when box.cfg is delayed in init script Message-ID: <20190820231528.bqfpccu334bfxkbs@tkn_work_nb> References: <20190819073936.37388-1-m.melentiev@corp.mail.ru> <20190819075457.GA18442@atlas> <59464C78-29E6-4F91-B8E9-CA01CB63FE65@corp.mail.ru> <20190819150532.gh25tyc73mcfazeg@tkn_work_nb> MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-Help: List-Unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-Subscribe: List-Owner: List-post: List-Archive: To: Maxim Melentiev Cc: tarantool-patches@freelists.org, Maxim Melentiev , Konstantin Osipov , Kirill Yukhin On Tue, Aug 20, 2019 at 02:24:23PM +0300, Maxim Melentiev wrote: > > BTW, is this is enough for you in the scope of your task or we need to > > boost other tarantoolctl discussions very soon? > This should be enough to complete our task (with some limitations). > > However we decided not to proceed with patching tarantoolctl right now > and provide start/stop features in rock’s binary replacing ctl for cluster apps. > Thank you for review. Please, ignore this patch request. The patch looks good for me as well. Thanks! I think that it is valuable even if your team does not needed it right now. CCed Kirill to decide about branches to which it should land. Max, I guess a mainainer may ask you to provide a branch in tarantool/tarantool repository with your commit at top of current master. Kirill, the patch has LGTMs from Kostya O. and me. WBR, Alexander Turenko. > Best regards, > Max > > From 1f53e9582ecf311323f88f9ac9b23a329072f901 Mon Sep 17 00:00:00 2001 > From: Max Melentiev > Date: Mon, 19 Aug 2019 10:35:55 +0300 > Subject: [PATCH] tarantoolctl: allow to start instances with delayed box.cfg{} > > `tarantoolctl start` patches box.cfg two times: > 1) before the init script to set default values and enforce some others, > 2) after the init script to prevent changing a pid_file in runtime. > > The second patching fails if an init file does not call > box.cfg{} before it's finished. This can take a place in apps with > managed instances which receive configuration from external server. > > This patch moves the second patching into the box.cfg > wrapper created during the first patching. So the second patching > is performed only after box.cfg{} was invoked, and it does not fail anymore. > > However there is relatively minor flaw for applications that > invoke box.cfg{} after init script is finished: > `tarantoolctl start` goes to background only when box.cfg{} is called. > Though this is not the case for daemon management systems like systemd, > as they handle backgrounding on their side. > > Fixes #4435 > > @TarantoolBot document > Title: tarantoolctl allows to start instances without a box.cfg{} > > tarantoolctl now works for instances without box.cfg{} or > with delayed box.cfg{} call. This can be managed instances which receive > configuration from external server. > > For such instances `tarantoolctl start` goes to background when > box.cfg{} is called, so it will wait until options for box.cfg are received. > However this is not the case for daemon management systems like systemd, > as they handle backgrounding on their side. > --- > extra/dist/tarantoolctl.in | 22 ++++++++++------------ > 1 file changed, 10 insertions(+), 12 deletions(-) > > diff --git a/extra/dist/tarantoolctl.in b/extra/dist/tarantoolctl.in > index 8adb57533..6daf866ac 100755 > --- a/extra/dist/tarantoolctl.in > +++ b/extra/dist/tarantoolctl.in > @@ -483,6 +483,16 @@ local function wrapper_cfg(cfg) > os.exit(1) > end > > + -- Prevent overwriting pid_file in subsequent box.cfg calls. > + local box_cfg_mt = getmetatable(box.cfg) > + local orig_cfg_call = box_cfg_mt.__call > + box_cfg_mt.__call = function(old_cfg, new_cfg) > + if old_cfg.pid_file ~= nil and new_cfg ~= nil and new_cfg.pid_file ~= nil then > + new_cfg.pid_file = old_cfg.pid_file > + end > + orig_cfg_call(old_cfg, new_cfg) > + end > + > return data > end > > @@ -547,18 +557,6 @@ local function start() > end > os.exit(1) > end > - local box_cfg_mt = getmetatable(box.cfg) > - if box_cfg_mt == nil then > - log.error('box.cfg() is not called in an instance file') > - os.exit(1) > - end > - local old_call = box_cfg_mt.__call > - box_cfg_mt.__call = function(old_cfg, cfg) > - if old_cfg.pid_file ~= nil and cfg ~= nil and cfg.pid_file ~= nil then > - cfg.pid_file = old_cfg.pid_file > - end > - old_call(old_cfg, cfg) > - end > return 0 > end > > -- > 2.21.0