From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp43.i.mail.ru (smtp43.i.mail.ru [94.100.177.103]) (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 8613A4696C3 for ; Thu, 19 Dec 2019 03:32:00 +0300 (MSK) References: <95c49cb56ed3eb7f2051ee1690f9b28a45210367.1575627361.git.i.kosarev@tarantool.org> From: Vladislav Shpilevoy Message-ID: <984f5fda-48d1-ce5d-4c69-2e784955bc10@tarantool.org> Date: Thu, 19 Dec 2019 01:31:58 +0100 MIME-Version: 1.0 In-Reply-To: <95c49cb56ed3eb7f2051ee1690f9b28a45210367.1575627361.git.i.kosarev@tarantool.org> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [Tarantool-patches] [PATCH 2/3] memtx: don't reserve extra memory if not needed List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Ilya Kosarev , tarantool-patches@dev.tarantool.org Thanks for the patch! On 13/12/2019 08:05, Ilya Kosarev wrote: > memtx_index_extent_reserve was always being called from > memtx_space_replace_all_keys. It could lead to slab allocator failure > while reaching memtx_memory limit, which is especially distressing on > space:truncate() and space:delete() invocations. we Now it is not > being called if not needed, including space:truncate() and > space:delete() cases. The thing is that delete also can cause tree rebalancing. Maybe that is why reservation was done for all cases. > > Part of #3807 > --- > src/box/memtx_space.c | 14 +++++++++++--- > 1 file changed, 11 insertions(+), 3 deletions(-) > > diff --git a/src/box/memtx_space.c b/src/box/memtx_space.c > index 6ef84e04564..1e8759778e7 100644 > --- a/src/box/memtx_space.c > +++ b/src/box/memtx_space.c > @@ -260,10 +260,18 @@ memtx_space_replace_all_keys(struct space *space, struct tuple *old_tuple, > * Ensure we have enough slack memory to guarantee > * successful statement-level rollback. > */ > - if (memtx_index_extent_reserve(memtx, new_tuple != NULL ? > - RESERVE_EXTENTS_BEFORE_REPLACE : > - RESERVE_EXTENTS_BEFORE_DELETE) != 0) > + if (space->index_count > 1 && new_tuple != NULL) { > + if (memtx_index_extent_reserve(memtx, new_tuple != NULL ? > + RESERVE_EXTENTS_BEFORE_REPLACE : > + RESERVE_EXTENTS_BEFORE_DELETE) != 0) > + return -1; Why does it work only when index count > 1? Why delete don't need a reservation (I pointed out above that it needs)? And why do you check for new_tuple != NULL second time after having it already checked in the if's condition? > + } > + ERROR_INJECT(ERRINJ_INDEX_ALLOC, { > + /* same error as in mempool_alloc */ > + diag_set(OutOfMemory, MEMTX_EXTENT_SIZE, > + "mempool", "new slab"); > return -1; > + }); > > uint32_t i = 0; > >