[Tarantool-patches] [PATCH 2/3] memtx: don't reserve extra memory if not needed

[Vladislav Shpilevoy]

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;
>  
>