Re: [Tarantool-patches] [PATCH v3 2/2] memtx: allow quota overuse for truncation and deletion

[Nikita Pettik <korablev@tarantool.org>]

On 20 Jan 21:13, Ilya Kosarev wrote:
> diff --git a/src/box/box.cc b/src/box/box.cc
> index 1b2b27d61..678c9ffe3 100644
> --- a/src/box/box.cc
> +++ b/src/box/box.cc
> @@ -1321,9 +1321,14 @@ space_truncate(struct space *space)
>  	ops_buf_end = mp_encode_uint(ops_buf_end, 1);
>  	assert(ops_buf_end < buf + buf_size);
>  
> +	struct space *truncate_space = space_cache_find_xc(BOX_TRUNCATE_ID);
> +	memtx_engine_set_quota_strictness(truncate_space->engine, false);

I wouldn't call it strictness. In fact you simply turn quota on/off.
So I'd better call it quota_enable/disable.

Nit: please accompany your code with (at least brief) comments.

> +
>  	if (box_upsert(BOX_TRUNCATE_ID, 0, tuple_buf, tuple_buf_end,
>  		       ops_buf, ops_buf_end, 0, NULL) != 0)
>  		diag_raise();

If box_upsert() fails, quota is not re-enabled, isn't it?

> +
> +	memtx_engine_set_quota_strictness(truncate_space->engine, true);
>  }
>  
>  int
> diff --git a/src/box/memtx_engine.c b/src/box/memtx_engine.c
> index 4da80824a..c19205d34 100644
> --- a/src/box/memtx_engine.c
> +++ b/src/box/memtx_engine.c
> @@ -1090,6 +1090,15 @@ memtx_engine_set_memory(struct memtx_engine *memtx, size_t size)
>  	return 0;
>  }
>  
> +void
> +memtx_engine_set_quota_strictness(struct engine *engine, bool strict)
> +{
> +	if (strncmp(engine->name, "memtx", 5) != 0)
> +		return;

memtx_ prefix in the name of this function assumes that it is called
only for memtx engine. Note that there's no such checks in memtx_engine
interface functions.

> +	struct memtx_engine *memtx = (struct memtx_engine *)engine;
> +	quota_set_strictness(&memtx->quota, strict);
 
What happens if quota is turned on back, but it still exceeds limit?
I guess it would turn out to be in inconsistent state, since in
normal operation mode there's no chance of being beyond the limit.

> +
>  void
>  memtx_engine_set_max_tuple_size(struct memtx_engine *memtx, size_t max_size)
>  {
> diff --git a/src/box/memtx_engine.h b/src/box/memtx_engine.h
> index f562c66df..0793fc2ab 100644
> --- a/src/box/memtx_engine.h
> +++ b/src/box/memtx_engine.h
> @@ -213,6 +213,9 @@ memtx_engine_set_snap_io_rate_limit(struct memtx_engine *memtx, double limit);
>  int
>  memtx_engine_set_memory(struct memtx_engine *memtx, size_t size);
>  
> +void
> +memtx_engine_set_quota_strictness(struct engine *engine, bool strict);
> +
>  void
>  memtx_engine_set_max_tuple_size(struct memtx_engine *memtx, size_t max_size);
>  
> diff --git a/src/lib/small b/src/lib/small
> index 50cb78743..bdcd569f9 160000
> --- a/src/lib/small
> +++ b/src/lib/small

What about adding unit and regression tests?

> @@ -1 +1 @@
> -Subproject commit 50cb7874380b286f3b34c82299cf5eb88b0d0059
> +Subproject commit bdcd569f9ed753efb805f708089ca86e2520a44f
> -- 
> 2.17.1
>