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 0C48F21E29 for ; Tue, 26 Jun 2018 07:11:54 -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 xM800D98vJhj for ; Tue, 26 Jun 2018 07:11:53 -0400 (EDT) Received: from smtp48.i.mail.ru (smtp48.i.mail.ru [94.100.177.108]) (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 BB22921D69 for ; Tue, 26 Jun 2018 07:11:53 -0400 (EDT) Subject: [tarantool-patches] Re: [PATCH 3/3] Introduce destroy module feature References: <1ebeee48-5204-f1c4-0775-bc12276bfcdf@tarantool.org> From: Vladislav Shpilevoy Message-ID: <4769d4ef-ab88-561a-72b0-3722180e13ef@tarantool.org> Date: Tue, 26 Jun 2018 14:11:51 +0300 MIME-Version: 1.0 In-Reply-To: <1ebeee48-5204-f1c4-0775-bc12276bfcdf@tarantool.org> Content-Type: text/plain; charset="utf-8"; format="flowed" Content-Language: en-US Content-Transfer-Encoding: 8bit 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: Alex Khatskevich , tarantool-patches@freelists.org Thanks for the fixes! See 3 comments below. On 26/06/2018 00:54, Alex Khatskevich wrote: > >>> --- a/test/lua_libs/util.lua >>> +++ b/test/lua_libs/util.lua >>> @@ -69,9 +69,43 @@ local function wait_master(test_run, replicaset, master) >>>       log.info('Slaves are connected to a master "%s"', master) >>>   end >>>   +function vshard_fiber_list() >>> +    -- Flush jit traces to prevent them from >>> +    -- keeping its upvalues in memory. >>> +    jit.flush() >>> +    collectgarbage() >>> +    -- Give a fiber time to clean itself. >>> +    fiber.sleep(0.05) >> >> 1. Why do you need this sleep? As far as I know >> collectgarbage just cleans all the garbage in the same >> fiber with no yields or async tasks. > netbox: If `collect garbage` has collected a connection, the fiber need a time to wakeup after > fiber:cancel() to process it and exit. > another module: the same possible behavior. 1. I removed this sleep and nothing happened. See below comment 5. > commit b0cddc2a3ba30d3ecf57e71ce8d095bf2f093a5e > Author: AKhatskevich > Date:   Wed Jun 20 00:31:25 2018 +0300 > >     Introduce destroy module feature > >     Introduce functions: >      * vshard.router.destroy() >      * vshard.storage.destroy() > >     Those functions: >      * close connections >      * stop background fibers >      * delete vshard spaces >      * delete vshard funcitons 2. Typo. >      * delete `once` metadate 3. Typo. > >     After the destroy, module can be configured as it was just loaded. > >     Extra changes: >      * introduce fiber_list function which returns names of non-tarantool >        fibers >      * introduce update_M function, which updates M (module internals) with >        values defined in the module > >     Closes #121 > > diff --git a/test/lua_libs/util.lua b/test/lua_libs/util.lua > index f2d3b48..ce0ea67 100644 > --- a/test/lua_libs/util.lua > +++ b/test/lua_libs/util.lua > @@ -69,9 +69,43 @@ local function wait_master(test_run, replicaset, master) >      log.info('Slaves are connected to a master "%s"', master) >  end > > +function vshard_fiber_list() > +    -- Flush jit traces to prevent them from > +    -- keeping its upvalues in memory. > +    jit.flush() > +    collectgarbage() > +    -- Give a fiber time to clean itself. > +    fiber.sleep(0.05) 4. It makes no sense not only because it changes nothing in the test output. Even if it could affect active fiber count because of fibers that had not managed to exit, you call this vshard_fiber_list() in the while cycles until its output matches your expectations. So please, remove the sleep. 5. Since we do not know what to do with spaces cleaning, I will review the next part of the patch later, when we will know.