Re: [Tarantool-patches] [PATCH v5] memtx: fix out of memory handling for rtree

[Nikita Pettik <korablev@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.