From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <korablev@tarantool.org>
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 dev.tarantool.org (Postfix) with ESMTPS id 58F0346970E
 for <tarantool-patches@dev.tarantool.org>;
 Mon, 13 Jan 2020 16:51:16 +0300 (MSK)
Date: Mon, 13 Jan 2020 16:51:14 +0300
From: Nikita Pettik <korablev@tarantool.org>
Message-ID: <20200113135114.GC7851@tarantool.org>
References: <20191225143557.4002-1-arkholga@tarantool.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <20191225143557.4002-1-arkholga@tarantool.org>
Subject: Re: [Tarantool-patches] [PATCH v5] memtx: fix out of memory
	handling for rtree
List-Id: Tarantool development patches <tarantool-patches.dev.tarantool.org>
List-Unsubscribe: <https://lists.tarantool.org/mailman/options/tarantool-patches>, 
 <mailto:tarantool-patches-request@dev.tarantool.org?subject=unsubscribe>
List-Archive: <https://lists.tarantool.org/pipermail/tarantool-patches/>
List-Post: <mailto:tarantool-patches@dev.tarantool.org>
List-Help: <mailto:tarantool-patches-request@dev.tarantool.org?subject=help>
List-Subscribe: <https://lists.tarantool.org/mailman/listinfo/tarantool-patches>, 
 <mailto:tarantool-patches-request@dev.tarantool.org?subject=subscribe>
To: Olga Arkhangelskaia <arkholga@tarantool.org>
Cc: tarantool-patches@dev.tarantool.org

On 25 Dec 17:35, Olga Arkhangelskaia wrote:
> 
> diff --git a/src/box/index.cc b/src/box/index.cc
> index 4e4867118..62cb2ab96 100644
> --- a/src/box/index.cc
> +++ b/src/box/index.cc
> @@ -733,6 +733,8 @@ int
>  generic_index_build_next(struct index *index, struct tuple *tuple)
>  {
>  	struct tuple *unused;
> +	if (index_reserve(index, 0) != 0)
> +        return -1;

Nit: still looks like broken indentation. Make sure that tab in your
editor occupies 8 spaces.

>  	return index_replace(index, NULL, tuple, DUP_INSERT, &unused);
>  }
>  
> diff --git a/src/box/memtx_rtree.c b/src/box/memtx_rtree.c
> index 8badad797..a5cae8f45 100644
> --- a/src/box/memtx_rtree.c
> +++ b/src/box/memtx_rtree.c
> @@ -242,6 +242,23 @@ memtx_rtree_index_replace(struct index *base, struct tuple *old_tuple,
>  	return 0;
>  }
>  
> +static int
> +memtx_rtree_index_reserve(struct index *base, uint32_t size_hint)
> +{
> +	/*
> +	 * In case of rtree we use reserve to make sure that memory allocation
> +	 * will not fail durig any operationin rtree, because there is no
> +	 * error handling in rtree lib.
> +	 */
> +	(void)size_hint;
> +	ERROR_INJECT(ERRINJ_INDEX_RESERVE, {
> +		diag_set(OutOfMemory, MEMTX_EXTENT_SIZE, "slab allocator", "memtx extent");

Nit: out of 80 chars.

> +		return -1;
> +	});
> +	struct memtx_engine *memtx = (struct memtx_engine *)base->engine;
> +	return memtx_index_extent_reserve(memtx, RESERVE_EXTENTS_BEFORE_REPLACE);
> +}
> +
>  static struct iterator *
>  memtx_rtree_index_create_iterator(struct index *base,  enum iterator_type type,
>  				  const char *key, uint32_t part_count)
> index babe36b1b..4c5a56ee3 100644
> --- a/test/box/errinj.result
> +++ b/test/box/errinj.result
> @@ -1,1783 +1,1983 @@
> +-- test-run result file version 2
>  -- Test that recovery had been completed without errors
>  test_run = require('test_run').new()
> ----
> -...
> + | ---
> + | ...
>  test_run:cmd("restart server default")
> + | 
>  box.error.last() == nil
> ----
> -- true
> -...
> + | ---
> + | - true
> + | ...
> +
>  errinj = box.error.injection

Something wrong with this result file change (looks like it is not
related to the patch at all - 2.7k lines are changed).

Otherwise LGTM.