Re: [Tarantool-patches] [PATCH] memtx: allow quota overuse for truncation

[Vladislav Shpilevoy <v.shpilevoy@tarantool.org>]

Thanks for the patch!

See 2 comments below.

On 11.12.2020 16:37, Ilya Kosarev wrote:
> Trying to perform space:truncate() while reaching memtx_memory limit
> we could experience slab allocator failure. This behavior seems to be
> quite surprising for users. Now we are allowing to overuse memtx quota
> for tuples in space _truncate using flag in struct quota.
> Truncate tuples are only being allocated with large slabs using malloc
> so that the quota can shrink back when they are freed.

1. 3807 is also about delete. Why didn't you patch it too? AFAIR, the
fix I proposed was easy - in case something inside memtx_space_execute_delete()
fails due to OOM, we disable quota, try again, and enable quota. In fact,
it would be even simpler than the truncation fix, I suppose.

> Closes #3807
> ---
> Branch: https://github.com/tarantool/tarantool/tree/i.kosarev/gh-3807-safe-alloc-on-truncation
> Issue: https://github.com/tarantool/tarantool/issues/3807
> 
> diff --git a/src/box/memtx_space.c b/src/box/memtx_space.c
> index 73b4c450eb..cc431ea816 100644
> --- a/src/box/memtx_space.c
> +++ b/src/box/memtx_space.c
> @@ -327,8 +327,9 @@ memtx_space_execute_replace(struct space *space, struct txn *txn,
>  	struct memtx_space *memtx_space = (struct memtx_space *)space;
>  	struct txn_stmt *stmt = txn_current_stmt(txn);
>  	enum dup_replace_mode mode = dup_replace_mode(request->type);
> -	stmt->new_tuple = memtx_tuple_new(space->format, request->tuple,
> -					  request->tuple_end);
> +	stmt->new_tuple = space->format->vtab.tuple_new(space->format,
> +							request->tuple,
> +							request->tuple_end);

2. Seems like an expensive change. You added +2 pointer
dereferences to a hot path.

Last time when you worked on that I proposed to make space_truncate
use box.begin + quota disable + box_upsert + quota enable + commit.
So there are no yields between quota enable and disable. And no changes
in the code not related to truncation. Why didn't it work?

>  	if (stmt->new_tuple == NULL)
>  		return -1;
>  	tuple_ref(stmt->new_tuple);