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 55D8B2F2F9 for ; Sat, 3 Nov 2018 10:30:03 -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 7gG2eU-oPab6 for ; Sat, 3 Nov 2018 10:30:03 -0400 (EDT) Received: from smtpng1.m.smailru.net (smtpng1.m.smailru.net [94.100.181.251]) (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 106622F2F2 for ; Sat, 3 Nov 2018 10:30:02 -0400 (EDT) Subject: [tarantool-patches] Re: [PATCH v1 1/1] box: restore region after box.snapshot() References: From: Vladislav Shpilevoy Message-ID: <10ec393e-1d7a-29b2-2b0d-a860d4b126e5@tarantool.org> Date: Sat, 3 Nov 2018 17:29:58 +0300 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit 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: imeevma@tarantool.org, tarantool-patches@freelists.org Hi! Thanks for the patch! On 03/11/2018 17:11, imeevma@tarantool.org wrote: > Before this patch region wasn't restored after box.snapshot() so > some tests fail due to "memory leak". After this patch region will > be restored using region_truncate(). It could be done by cleaning > memory in case there wasn't active transaction but it seems like > region_truncate() is more universal. > > Closes #3732 LGTM. > --- > Issue: https://github.com/tarantool/tarantool/issues/3732 > Branch: https://github.com/tarantool/tarantool/tree/imeevma/gh-3732-restore_region_after_box_snapshot > > src/box/box.cc | 2 ++ > test/engine/snapshot.result | 35 +++++++++++++++++++++++++++++++++++ > test/engine/snapshot.test.lua | 17 +++++++++++++++++ > 3 files changed, 54 insertions(+) > > diff --git a/src/box/box.cc b/src/box/box.cc > index c4b4fb4..0d27aa2 100644 > --- a/src/box/box.cc > +++ b/src/box/box.cc > @@ -2157,6 +2157,7 @@ box_checkpoint() > if (! is_box_configured) > return 0; > int rc = 0; > + size_t used = region_used(&fiber()->gc); > if (box_checkpoint_is_in_progress) { > diag_set(ClientError, ER_CHECKPOINT_IN_PROGRESS); > return -1; > @@ -2181,6 +2182,7 @@ end: > > latch_unlock(&schema_lock); > box_checkpoint_is_in_progress = false; > + region_truncate(&fiber()->gc, used); > return rc; > } > > diff --git a/test/engine/snapshot.result b/test/engine/snapshot.result > index a5aa08d..42009c2 100644 > --- a/test/engine/snapshot.result > +++ b/test/engine/snapshot.result > @@ -238,3 +238,38 @@ box.space.test.index.secondary:select{} > box.space.test:drop() > --- > ... > +-- > +-- sql: fix sql/gh-3199-no-mem-leaks.test.lua > +-- One of the reason why this test failed - box.snapshot didn't > +-- free region. > +-- > +fiber = require('fiber') > +--- > +... > +-- Should be 0. > +fiber.info()[fiber.self().id()].memory.used > +--- > +- 0 > +... > +box.snapshot() > +--- > +- ok > +... > +-- Should be 0. > +fiber.info()[fiber.self().id()].memory.used > +--- > +- 0 > +... > +box.snapshot() > +--- > +- ok > +... > +box.snapshot() > +--- > +- ok > +... > +-- Should be 0. > +fiber.info()[fiber.self().id()].memory.used > +--- > +- 0 > +... > diff --git a/test/engine/snapshot.test.lua b/test/engine/snapshot.test.lua > index 4bbbe40..2a32280 100644 > --- a/test/engine/snapshot.test.lua > +++ b/test/engine/snapshot.test.lua > @@ -41,3 +41,20 @@ box.space.test:select{} > box.space.test.index.primary:select{} > box.space.test.index.secondary:select{} > box.space.test:drop() > + > +-- > +-- sql: fix sql/gh-3199-no-mem-leaks.test.lua > +-- One of the reason why this test failed - box.snapshot didn't > +-- free region. > +-- > + > +fiber = require('fiber') > +-- Should be 0. > +fiber.info()[fiber.self().id()].memory.used > +box.snapshot() > +-- Should be 0. > +fiber.info()[fiber.self().id()].memory.used > +box.snapshot() > +box.snapshot() > +-- Should be 0. > +fiber.info()[fiber.self().id()].memory.used >