[Tarantool-patches] [PATCH v3 0/2] Safe truncation and deletion

[Tarantool-patches] [PATCH v3 0/2] Safe truncation and deletion

[Ilya Kosarev]

space:truncate() and space:delete() could fail on memory allocations
when reaching memtx_memory limit. As far as it is quite an ill
behaviour, it is fixed in this patchset through memtx quota strictness
adjustment. Now it can be overused if needed.
Also possible bps_tree_create_leaf NULL dereference issue is fixed.

Branch: https://github.com/tarantool/tarantool/tree/i.kosarev/gh-3807-safe-alloc-on-truncation
Issue: https://github.com/tarantool/tarantool/issues/3807

Changes in v2:
  Approach changed completely: now we are not trying to allocate
  service tuples in some safe way, but increasing memtx quota so
  that space:truncate() and space:delete() won't fail on allocation.

Changes in v3:
  Now we are not increasing memtx quota. Instead we just set a flag to
  allow quota overuse for space:truncate() and space:delete().

Ilya Kosarev (2):
  b-tree: return NULL on matras_alloc fail
  memtx: allow quota overuse for truncation and deletion

 src/box/box.cc           |  5 +++++
 src/box/memtx_engine.c   |  9 +++++++++
 src/box/memtx_engine.h   |  3 +++
 src/box/memtx_space.c    | 13 +++++++++----
 src/lib/salad/bps_tree.h |  7 +++++--
 src/lib/small            |  2 +-
 6 files changed, 32 insertions(+), 7 deletions(-)

-- 
2.17.1

[Tarantool-patches] [PATCH v3 1/2] b-tree: return NULL on matras_alloc fail

[Ilya Kosarev]

In bps_tree_create_leaf we use matras_alloc in case
bps_tree_garbage_pop didn't work out. However it also might not
succeed. Then we need to return NULL instead of dereferencing NULL
pointer.

Part of #3807
---
 src/lib/salad/bps_tree.h | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/src/lib/salad/bps_tree.h b/src/lib/salad/bps_tree.h
index d28b53f53..a10f0a32c 100644
--- a/src/lib/salad/bps_tree.h
+++ b/src/lib/salad/bps_tree.h
@@ -2147,8 +2147,11 @@ bps_tree_create_leaf(struct bps_tree *tree, bps_tree_block_id_t *id)
 {
 	struct bps_leaf *res = (struct bps_leaf *)
 			       bps_tree_garbage_pop(tree, id);
-	if (!res)
-		res = (struct bps_leaf *)matras_alloc(&tree->matras, id);
+	if (res == NULL) {
+		res = (struct bps_leaf *) matras_alloc(&tree->matras, id);
+		if (res == NULL)
+			return NULL;
+	}
 	res->header.type = BPS_TREE_BT_LEAF;
 	tree->leaf_count++;
 	return res;
-- 
2.17.1

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

[Ilya Kosarev]

Trying to perform space:truncate() and space:delete() 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 if needed for truncation or deletion, using flag
in struct quota. After performing truncation or deletion we reset the
flag so that quota can be overused any more.

Closes #3807
---
 src/box/box.cc         |  5 +++++
 src/box/memtx_engine.c |  9 +++++++++
 src/box/memtx_engine.h |  3 +++
 src/box/memtx_space.c  | 13 +++++++++----
 src/lib/small          |  2 +-
 5 files changed, 27 insertions(+), 5 deletions(-)

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);
+
 	if (box_upsert(BOX_TRUNCATE_ID, 0, tuple_buf, tuple_buf_end,
 		       ops_buf, ops_buf_end, 0, NULL) != 0)
 		diag_raise();
+
+	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;
+	struct memtx_engine *memtx = (struct memtx_engine *)engine;
+	quota_set_strictness(&memtx->quota, strict);
+}
+
 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/box/memtx_space.c b/src/box/memtx_space.c
index 6ef84e045..20ce2f974 100644
--- a/src/box/memtx_space.c
+++ b/src/box/memtx_space.c
@@ -354,10 +354,15 @@ memtx_space_execute_delete(struct space *space, struct txn *txn,
 	struct tuple *old_tuple;
 	if (index_get(pk, key, part_count, &old_tuple) != 0)
 		return -1;
-	if (old_tuple != NULL &&
-	    memtx_space->replace(space, old_tuple, NULL,
-				 DUP_REPLACE_OR_INSERT, &stmt->old_tuple) != 0)
-		return -1;
+	if (old_tuple != NULL) {
+		memtx_engine_set_quota_strictness(space->engine, false);
+		int rc = memtx_space->replace(space, old_tuple, NULL,
+					      DUP_REPLACE_OR_INSERT,
+					      &stmt->old_tuple);
+		memtx_engine_set_quota_strictness(space->engine, true);
+		if (rc != 0)
+			return -1;
+	}
 	stmt->engine_savepoint = stmt;
 	*result = stmt->old_tuple;
 	return 0;
diff --git a/src/lib/small b/src/lib/small
index 50cb78743..bdcd569f9 160000
--- a/src/lib/small
+++ b/src/lib/small
@@ -1 +1 @@
-Subproject commit 50cb7874380b286f3b34c82299cf5eb88b0d0059
+Subproject commit bdcd569f9ed753efb805f708089ca86e2520a44f
-- 
2.17.1

Re: [Tarantool-patches] [PATCH v3 1/2] b-tree: return NULL on matras_alloc fail

[Nikita Pettik]

On 20 Jan 21:13, Ilya Kosarev wrote:
> In bps_tree_create_leaf we use matras_alloc in case
> bps_tree_garbage_pop didn't work out. However it also might not
> succeed. Then we need to return NULL instead of dereferencing NULL
> pointer.
> 
> Part of #3807
> ---
>  src/lib/salad/bps_tree.h | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/src/lib/salad/bps_tree.h b/src/lib/salad/bps_tree.h
> index d28b53f53..a10f0a32c 100644
> --- a/src/lib/salad/bps_tree.h
> +++ b/src/lib/salad/bps_tree.h
> @@ -2147,8 +2147,11 @@ bps_tree_create_leaf(struct bps_tree *tree, bps_tree_block_id_t *id)
>  {

What about bps_tree_create_inner()? It follows the same allocation pattern.
Doesn't it require same NULL check? What is more, result of
bps_tree_create_leaf() invokation is not checked in
bps_tree_process_insert_leaf().

>  	struct bps_leaf *res = (struct bps_leaf *)
>  			       bps_tree_garbage_pop(tree, id);
> -	if (!res)
> -		res = (struct bps_leaf *)matras_alloc(&tree->matras, id);
> +	if (res == NULL) {

Nit: personally I wouldn't strive to fix codestyle in this place
since all bps_tree.h code follows the same (even if a bit different)
codestyle. So now code being fixed looks a bit strange (IMHO).

> +		res = (struct bps_leaf *) matras_alloc(&tree->matras, id);
> +		if (res == NULL)
> +			return NULL;
> +	}
>  	res->header.type = BPS_TREE_BT_LEAF;
>  	tree->leaf_count++;
>  	return res;
> -- 
> 2.17.1
> 

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

[Nikita Pettik]

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
> 

Re: [Tarantool-patches] [PATCH v3 1/2] b-tree: return NULL on matras_alloc fail

[Vladislav Shpilevoy]

Hi! Thanks for the patch!

I agree with Nikita in everything, especially
about bps_tree_create_inner().

Sorry, when I said that code style should be
fixed I didn't take into account that bps_tree
is like a separate library with own code style
where ! is used instead of == NULL.

On 20/01/2020 19:13, Ilya Kosarev wrote:
> In bps_tree_create_leaf we use matras_alloc in case
> bps_tree_garbage_pop didn't work out. However it also might not
> succeed. Then we need to return NULL instead of dereferencing NULL
> pointer.
> 
> Part of #3807
> ---
>  src/lib/salad/bps_tree.h | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/src/lib/salad/bps_tree.h b/src/lib/salad/bps_tree.h
> index d28b53f53..a10f0a32c 100644
> --- a/src/lib/salad/bps_tree.h
> +++ b/src/lib/salad/bps_tree.h
> @@ -2147,8 +2147,11 @@ bps_tree_create_leaf(struct bps_tree *tree, bps_tree_block_id_t *id)
>  {
>  	struct bps_leaf *res = (struct bps_leaf *)
>  			       bps_tree_garbage_pop(tree, id);
> -	if (!res)
> -		res = (struct bps_leaf *)matras_alloc(&tree->matras, id);
> +	if (res == NULL) {
> +		res = (struct bps_leaf *) matras_alloc(&tree->matras, id);
> +		if (res == NULL)
> +			return NULL;
> +	}
>  	res->header.type = BPS_TREE_BT_LEAF;
>  	tree->leaf_count++;
>  	return res;
> 

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

[Vladislav Shpilevoy]

Thanks for the patch!

I answer some of Nikita's questions below and
provide my own review further.

See 7 comments below.

On 21/01/2020 12:42, Nikita Pettik wrote:
> 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. 

1. On/off sounds good.

>> +
>>  	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?

2. This is a bug, yes. But not the only one. The upsert will write
to WAL and yield. During that yield the quota is turned off. And
any other fiber can interfere and overuse the quota even more with
something not related to delete/truncate.

This can be fixed, for example, by making txn_begin() + quota_off() +
upsert() + quota_on() + commit(). Also we could just not touch truncate
and fix delete() only. Delete() can never leave the quota overused,
because is not stored anywhere in memtx. While truncate is stored in
_truncate. And delete would be sufficient, because can be used to free
the quota enough so as to call truncate()/whatever else the user wanted.

>> +
>> +	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.

3. That problem is not solvable. There is always a chance, that truncate
won't free any memory (for example, the target space could be empty
already). If, after the quota is back on, it appears to be overused, nothing
can be done with it. It will stay overused until more delete/truncate will be
executed and actually delete something, or the user will increase memtx memory.

> commit 45d9f491f91d93c8835ca725c234f7113c0d2aa8
> Author: Ilya Kosarev <i.kosarev@tarantool.org>
> Date:   Mon Jan 20 20:21:37 2020 +0300
> 
>     memtx: allow quota overuse for truncation and deletion
>     
>     
>     Trying to perform space:truncate() and space:delete() 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 if needed for truncation or deletion, using flag
>     in struct quota. After performing truncation or deletion we reset the
>     flag so that quota can be overused any more.
>     
>     Closes #3807
> 
> 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);
> +
>  	if (box_upsert(BOX_TRUNCATE_ID, 0, tuple_buf, tuple_buf_end,
>  		       ops_buf, ops_buf_end, 0, NULL) != 0)
>  		diag_raise();
> +
> +	memtx_engine_set_quota_strictness(truncate_space->engine, true);
>  }

4. IMO, we should not touch quota at all until we see a not empty diag
with OOM error type in it. Both here and in delete. Truncate is not
a frequent operation (although it may be in some rare installations),
but delete surely should not touch quota each time.

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

5. That check should not exist. It slows down the caller, and
in upper code you always know whether you work with memtx or
not. So it makes no sense to check it here. Truncate is
always about insertion into a memtx space. Delete touches qouta
inside memtx_space implementation. No chances that a vinyl space
would call this function.

> +	struct memtx_engine *memtx = (struct memtx_engine *)engine;
> +	quota_set_strictness(&memtx->quota, 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
> @@ -1 +1 @@
> -Subproject commit 50cb7874380b286f3b34c82299cf5eb88b0d0059
> +Subproject commit bdcd569f9ed753efb805f708089ca86e2520a44f

6. Please, write tests. They should include the bug test itself, and
complex things such as quota overuse kept in case truncate() didn't
free anything. You can use memtx_memory parameter to make it small
enough so as the test would not take much time.

7. Seems like you changed something in small/. You need to send it as
a separate patch with its own tests, and all.

Re: [Tarantool-patches] [PATCH v3 1/2] b-tree: return NULL on matras_alloc fail

[Konstantin Osipov]

* Nikita Pettik <korablev@tarantool.org> [20/01/21 13:37]:
> On 20 Jan 21:13, Ilya Kosarev wrote:
> > In bps_tree_create_leaf we use matras_alloc in case
> > bps_tree_garbage_pop didn't work out. However it also might not
> > succeed. Then we need to return NULL instead of dereferencing NULL
> > pointer.

I don't understand the attempt to fix it.

The reason the allocations are not checked - most likely -because
BPS should refuse to even begin an operation if there is not
enough memory in matras.

Most likely Alexander Lyapunov was relying on that, and this is why you
don't have these checks all over bps code.



-- 
Konstantin Osipov, Moscow, Russia

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

[Konstantin Osipov]

* Ilya Kosarev <i.kosarev@tarantool.org> [20/01/20 21:18]:
> Trying to perform space:truncate() and space:delete() 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 if needed for truncation or deletion, using flag
> in struct quota. After performing truncation or deletion we reset the
> flag so that quota can be overused any more.

This is duplicating the tech already existing with reserved
extents.

Why not use the reserved extents instead? Why do you need to turn
quota completely on/off?


-- 
Konstantin Osipov, Moscow, Russia

Re: [Tarantool-patches] [PATCH v3 1/2] b-tree: return NULL on matras_alloc fail

[Nikita Pettik]

On 31 Jan 11:18, Konstantin Osipov wrote:
> * Nikita Pettik <korablev@tarantool.org> [20/01/21 13:37]:
> > On 20 Jan 21:13, Ilya Kosarev wrote:
> > > In bps_tree_create_leaf we use matras_alloc in case
> > > bps_tree_garbage_pop didn't work out. However it also might not
> > > succeed. Then we need to return NULL instead of dereferencing NULL
> > > pointer.
> 
> I don't understand the attempt to fix it.
> 
> The reason the allocations are not checked - most likely -because
> BPS should refuse to even begin an operation if there is not
> enough memory in matras.

According to the code it doesn't look so. Matras allocation checked
on fails, except two ones in bps_tree_create_inner() and
bps_tree_create_leaf().
 
> Most likely Alexander Lyapunov was relying on that, and this is why you
> don't have these checks all over bps code.> 
> 
> -- 
> Konstantin Osipov, Moscow, Russia

Re: [Tarantool-patches] [PATCH v3 1/2] b-tree: return NULL on matras_alloc fail

[Konstantin Osipov]

* Nikita Pettik <korablev@tarantool.org> [20/02/04 20:21]:
> > > On 20 Jan 21:13, Ilya Kosarev wrote:
> > > > In bps_tree_create_leaf we use matras_alloc in case
> > > > bps_tree_garbage_pop didn't work out. However it also might not
> > > > succeed. Then we need to return NULL instead of dereferencing NULL
> > > > pointer.
> > 
> > I don't understand the attempt to fix it.
> > 
> > The reason the allocations are not checked - most likely -because
> > BPS should refuse to even begin an operation if there is not
> > enough memory in matras.
> 
> According to the code it doesn't look so. Matras allocation checked
> on fails, except two ones in bps_tree_create_inner() and
> bps_tree_create_leaf().

Because reserve_blocks() should reserve enough blocks or fail, and
it is called before create_inner/create_leaf in all execution paths?

-- 
Konstantin Osipov, Moscow, Russia

Re: [Tarantool-patches] [PATCH v3 1/2] b-tree: return NULL on matras_alloc fail

[Nikita Pettik]

On 04 Feb 20:25, Konstantin Osipov wrote:
> * Nikita Pettik <korablev@tarantool.org> [20/02/04 20:21]:
> > > > On 20 Jan 21:13, Ilya Kosarev wrote:
> > > > > In bps_tree_create_leaf we use matras_alloc in case
> > > > > bps_tree_garbage_pop didn't work out. However it also might not
> > > > > succeed. Then we need to return NULL instead of dereferencing NULL
> > > > > pointer.
> > > 
> > > I don't understand the attempt to fix it.
> > > 
> > > The reason the allocations are not checked - most likely -because
> > > BPS should refuse to even begin an operation if there is not
> > > enough memory in matras.
> > 
> > According to the code it doesn't look so. Matras allocation checked
> > on fails, except two ones in bps_tree_create_inner() and
> > bps_tree_create_leaf().
> 
> Because reserve_blocks() should reserve enough blocks or fail, and
> it is called before create_inner/create_leaf in all execution paths?

Consider following path:

memtx_tree_index_replace
|
->memtx_tree_insert (bps_tree_insert)
  |
  ->bps_tree_insert_first_elem
    |
    ->bps_tree_create_leaf
      |
      -> matras_alloc

In this case reserve_blocks() is not called. AFAIU path is likely to
be reachable. Am I missing smth?
 
> -- 
> Konstantin Osipov, Moscow, Russia

Re: [Tarantool-patches] [PATCH v3 1/2] b-tree: return NULL on matras_alloc fail

[Konstantin Osipov]

* Nikita Pettik <korablev@tarantool.org> [20/02/04 21:10]:
> On 04 Feb 20:25, Konstantin Osipov wrote:
> > * Nikita Pettik <korablev@tarantool.org> [20/02/04 20:21]:
> > > > > On 20 Jan 21:13, Ilya Kosarev wrote:
> > > > > > In bps_tree_create_leaf we use matras_alloc in case
> > > > > > bps_tree_garbage_pop didn't work out. However it also might not
> > > > > > succeed. Then we need to return NULL instead of dereferencing NULL
> > > > > > pointer.
> > > > 
> > > > I don't understand the attempt to fix it.
> > > > 
> > > > The reason the allocations are not checked - most likely -because
> > > > BPS should refuse to even begin an operation if there is not
> > > > enough memory in matras.
> > > 
> > > According to the code it doesn't look so. Matras allocation checked
> > > on fails, except two ones in bps_tree_create_inner() and
> > > bps_tree_create_leaf().
> > 
> > Because reserve_blocks() should reserve enough blocks or fail, and
> > it is called before create_inner/create_leaf in all execution paths?
> 
> Consider following path:
> 
> memtx_tree_index_replace
> |
> ->memtx_tree_insert (bps_tree_insert)
>   |
>   ->bps_tree_insert_first_elem
>     |
>     ->bps_tree_create_leaf
>       |
>       -> matras_alloc
> 
> In this case reserve_blocks() is not called. AFAIU path is likely to
> be reachable. Am I missing smth?

No, in this trace I agree with you, we clearly see some sort of
refactoring artefact or a coding bug.

What I am observing is that bps_tree_insert_first_elem() actually checks
the return value of bps_tree_create_leaf(). But
bps_tree_create_leaf() never returns NULL, so this check is never
false.

bps was originally written in C++, so all of these checks for OOm
were added when it was rewritten in C, after the fact.

Looks like we're dealing with an artefact of this rewrite - the
checks are not consistent.


-- 
Konstantin Osipov, Moscow, Russia
https://scylladb.com

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

[Ilya Kosarev]

[-- Attachment #1: Type: text/plain, Size: 959 bytes --]


Hi!
Thanks for your comments.
When performing space: truncate(), we are executing
simple upsert. How can we distinguish it from any other upsert to use reserved extents tech? пятница, 31 января 2020г., 11:21 +03:00 от Konstantin Osipov  kostja.osipov@gmail.com :

>* Ilya Kosarev < i.kosarev@tarantool.org> [20/01/20 21:18]:
> Trying to perform space:truncate() and space:delete() 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 if needed for truncation or deletion, using flag
> in struct quota. After performing truncation or deletion we reset the
> flag so that quota can be overused any more.
>
>This is duplicating the tech already existing with reserved
>extents.
>
>Why not use the reserved extents instead? Why do you need to turn
>quota completely on/off?
>
>
>-- 
>Konstantin Osipov, Moscow, Russia

[-- Attachment #2: Type: text/html, Size: 1721 bytes --]

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

[Konstantin Osipov]

* Ilya Kosarev <i.kosarev@tarantool.org> [20/02/04 22:00]:
> Hi!
> Thanks for your comments.
> When performing space: truncate(), we are executing
> simple upsert. How can we distinguish it from any other upsert
> to use reserved extents tech? пятница, 31 января 2020г., 11:21

What code path is troubling you?

Just like you disable quota, you can "enable" use of reserved
extents. You can use the same injection points for this code, the
only difference is that you don't allocate memory beyond quota,
but rather use pre-allocated one, within the quota.


-- 
Konstantin Osipov, Moscow, Russia

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

[Ilya Kosarev]

[-- Attachment #1: Type: text/plain, Size: 899 bytes --]


You mean using some specific extent_pool, like indext_extent_pool
with it's memtx_index_extent_reserve() and memtx_index_extent_alloc()?
  
>Вторник, 4 февраля 2020, 23:06 +03:00 от Konstantin Osipov <kostja.osipov@gmail.com>:
> 
>* Ilya Kosarev < i.kosarev@tarantool.org > [20/02/04 22:00]:
>> Hi!
>> Thanks for your comments.
>> When performing space: truncate(), we are executing
>> simple upsert. How can we distinguish it from any other upsert
>> to use reserved extents tech? пятница, 31 января 2020г., 11:21
>What code path is troubling you?
>
>Just like you disable quota, you can "enable" use of reserved
>extents. You can use the same injection points for this code, the
>only difference is that you don't allocate memory beyond quota,
>but rather use pre-allocated one, within the quota.
>
>
>--
>Konstantin Osipov, Moscow, Russia
 
 
--
Ilya Kosarev
 

[-- Attachment #2: Type: text/html, Size: 1486 bytes --]

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

[Konstantin Osipov]

* Ilya Kosarev <i.kosarev@tarantool.org> [20/02/05 22:15]:
> 
> You mean using some specific extent_pool, like indext_extent_pool
> with it's memtx_index_extent_reserve() and memtx_index_extent_alloc()?

Yes.



-- 
Konstantin Osipov, Moscow, Russia

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

[Ilya Kosarev]

[-- Attachment #1: Type: text/plain, Size: 692 bytes --]


Well, the problem here is that garbage collection won't work properly
in this case. I guess it can be adjusted, however, there will be some
kind of crutches and it doesn't seem to be a good idea. Just switching
quota on and off seems to be more suitable and convenient for
truncations and deletions.
  
>Среда, 5 февраля 2020, 22:17 +03:00 от Konstantin Osipov <kostja.osipov@gmail.com>:
> 
>* Ilya Kosarev < i.kosarev@tarantool.org > [20/02/05 22:15]:
>>
>> You mean using some specific extent_pool, like indext_extent_pool
>> with it's memtx_index_extent_reserve() and memtx_index_extent_alloc()?
>Yes.
>
>
>
>--
>Konstantin Osipov, Moscow, Russia 
 
 
--
Ilya Kosarev
 

[-- Attachment #2: Type: text/html, Size: 1251 bytes --]

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

[Konstantin Osipov]

* Ilya Kosarev <i.kosarev@tarantool.org> [20/02/06 13:51]:
> 
> Well, the problem here is that garbage collection won't work properly
> in this case.

I don't understand what GC has to do with it. Please clarify.

> I guess it can be adjusted, however, there will be some
> kind of crutches and it doesn't seem to be a good idea. Just switching
> quota on and off seems to be more suitable and convenient for
> truncations and deletions.
>  

-- 
Konstantin Osipov, Moscow, Russia

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

[Ilya Kosarev]

[-- Attachment #1: Type: text/plain, Size: 1125 bytes --]


From memtx_tuple_new() we call smalloc(). In case it is calling
mempool_alloc(), we are passing the pool from memtx->alloc, chosen
depending on it's size. Later this tuple may be deleted. Then smfree()
is called, which uses mempool_find() to acquire the pool we want to
free.
In case we are using some other specific pool, current mempool_find()
won't be able to find it. This means we need to process this case
explicitly and it seems to add noticeable unjustified complexity to the
code.

  
>Четверг, 6 февраля 2020, 17:29 +03:00 от Konstantin Osipov <kostja.osipov@gmail.com>:
> 
>* Ilya Kosarev < i.kosarev@tarantool.org > [20/02/06 13:51]:
>>
>> Well, the problem here is that garbage collection won't work properly
>> in this case.
>
>I don't understand what GC has to do with it. Please clarify.
>
>> I guess it can be adjusted, however, there will be some
>> kind of crutches and it doesn't seem to be a good idea. Just switching
>> quota on and off seems to be more suitable and convenient for
>> truncations and deletions.
>>  
>
>--
>Konstantin Osipov, Moscow, Russia 
 
 
--
Ilya Kosarev
 

[-- Attachment #2: Type: text/html, Size: 1696 bytes --]

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

[Konstantin Osipov]

* Ilya Kosarev <i.kosarev@tarantool.org> [20/02/06 19:15]:
> 
> From memtx_tuple_new() we call smalloc(). In case it is calling
> mempool_alloc(), we are passing the pool from memtx->alloc, chosen
> depending on it's size. Later this tuple may be deleted. Then smfree()
> is called, which uses mempool_find() to acquire the pool we want to
> free.
> In case we are using some other specific pool, current mempool_find()
> won't be able to find it. This means we need to process this case
> explicitly and it seems to add noticeable unjustified complexity to the
> code.

Why do you need to use a special pool? Can't you reserve a single
slab in memtx_arena, just like memtx reserves extents now, and
allow use of this slab for any pool *only* when processing truncate? 

-- 
Konstantin Osipov, Moscow, Russia
https://scylladb.com

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

[Ilya Kosarev]

[-- Attachment #1: Type: text/plain, Size: 1662 bytes --]


As far as i see, memtx reserves extents using specific pool and
mempool_alloc. We will need to implement additional feature to reserve
slabs in memtx arena and provide them specifically for truncation
needs. However, after that we will encounter the necessity to calculate
how much space we need to reserve for truncation tuples. I guess we
might want to reserve a truncation buf for each created space and
re-reserve it on each truncation, although such logic seems
overcomplicated.
Furthermore, the problem with space:delete() might happen exactly when
we are trying to reserve a slab to guarantee successful statement-level
rollback. This can't be fixed using reservation tech. 
>Четверг, 6 февраля 2020, 19:26 +03:00 от Konstantin Osipov <kostja.osipov@gmail.com>:
> 
>* Ilya Kosarev < i.kosarev@tarantool.org > [20/02/06 19:15]:
>>
>> From memtx_tuple_new() we call smalloc(). In case it is calling
>> mempool_alloc(), we are passing the pool from memtx->alloc, chosen
>> depending on it's size. Later this tuple may be deleted. Then smfree()
>> is called, which uses mempool_find() to acquire the pool we want to
>> free.
>> In case we are using some other specific pool, current mempool_find()
>> won't be able to find it. This means we need to process this case
>> explicitly and it seems to add noticeable unjustified complexity to the
>> code.
>Why do you need to use a special pool? Can't you reserve a single
>slab in memtx_arena, just like memtx reserves extents now, and
>allow use of this slab for any pool *only* when processing truncate?
>
>--
>Konstantin Osipov, Moscow, Russia
>https://scylladb.com 
 
 
--
Ilya Kosarev
 

[-- Attachment #2: Type: text/html, Size: 2338 bytes --]

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

[Konstantin Osipov]

* Ilya Kosarev <i.kosarev@tarantool.org> [20/02/10 11:26]:
> 
> As far as i see, memtx reserves extents using specific pool and
> mempool_alloc. We will need to implement additional feature to reserve
> slabs in memtx arena and provide them specifically for truncation
> needs.

Yes. But the way you say it sounds negative. Are you suggesting
not implementing this feature and just hacking in quota bypass is
better? Why?

> However, after that we will encounter the necessity to calculate
> how much space we need to reserve for truncation tuples.

No. 1 slab is enough.

> I guess we
> might want to reserve a truncation buf for each created space and
> re-reserve it on each truncation, although such logic seems
> overcomplicated.

Indeed, this would be strange.

> Furthermore, the problem with space:delete() might happen exactly when
> we are trying to reserve a slab to guarantee successful statement-level
> rollback. This can't be fixed using reservation tech. 

Two events can't happen at the same time in a single-threaded
application. Please take time to explain what you mean here. 


-- 
Konstantin Osipov, Moscow, Russia

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

[Ilya Kosarev]

[-- Attachment #1: Type: text/plain, Size: 1801 bytes --]


>> As far as i see, memtx reserves extents using specific pool and
>> mempool_alloc. We will need to implement additional feature to reserve
>> slabs in memtx arena and provide them specifically for truncation
>> needs.
>
>Yes. But the way you say it sounds negative. Are you suggesting
>not implementing this feature and just hacking in quota bypass is
>better? Why?
I am afraid that ‘reserving’ approach both doesn’t always guarantee
successful truncation (without some overcomplicated actions) and adds
extra code complexity comparing to the quota on/off mechansm.
I see that simple quota hacking doesn’t look good, but i think it might
be even worse to implement partial solution along with all the extra
code. So far it might be better not to touch this problem, as Vlad
suggests, considering it as not critical.
  
>> However, after that we will encounter the necessity to calculate
>> how much space we need to reserve for truncation tuples.
>
>No. 1 slab is enough.
>
>> I guess we
>> might want to reserve a truncation buf for each created space and
>> re-reserve it on each truncation, although such logic seems
>> overcomplicated.
>
>Indeed, this would be strange.
>
>> Furthermore, the problem with space:delete() might happen exactly when
>> we are trying to reserve a slab to guarantee successful statement-level
>> rollback. This can't be fixed using reservation tech.
>
>Two events can't happen at the same time in a single-threaded
>application. Please take time to explain what you mean here.
I mean «when» in terms of place (where we reserve), not time. I mean
this is one more problem which can't be solved using 'reserve' tech,
although i am not sure it really should be solved through quota bypass.

>--
>Konstantin Osipov, Moscow, Russia 
 
 
--
Ilya Kosarev
 

[-- Attachment #2: Type: text/html, Size: 2428 bytes --]

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

[Konstantin Osipov]

* Ilya Kosarev <i.kosarev@tarantool.org> [20/02/10 13:44]:

Ilya, please explain the issues in specific terms: references to
source code, test conditions, and so on.

I don't see any issue with reserving a slab at Tarantool start for 
out-of-memory truncate. I don't see any issues with deletes,
because deletes are not supposed to use this slab, it should only hold
truncate record. Deletes do not allocate tuples anyway.

I think you should try to do it and send a patch when you get
stuck.

You can't expect me to explain how to do it because I don't know
what you don't know.

-- 
Konstantin Osipov, Moscow, Russia

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

[Ilya Kosarev]

[-- Attachment #1: Type: text/plain, Size: 1061 bytes --]


Sent tarantool/small patch and patchset with explanations on reserve
tech drawbacks and space:delete() problem:
https://lists.tarantool.org/pipermail/tarantool-patches/2020-February/014191.html
  
>Понедельник, 10 февраля 2020, 13:50 +03:00 от Konstantin Osipov <kostja.osipov@gmail.com>:
> 
>* Ilya Kosarev < i.kosarev@tarantool.org > [20/02/10 13:44]:
>
>Ilya, please explain the issues in specific terms: references to
>source code, test conditions, and so on.
>
>I don't see any issue with reserving a slab at Tarantool start for
>out-of-memory truncate. I don't see any issues with deletes,
>because deletes are not supposed to use this slab, it should only hold
>truncate record. Deletes do not allocate tuples anyway.
Right, space:delete() problem is not connected with the
space:truncate() problem.
>
>I think you should try to do it and send a patch when you get
>stuck.
>
>You can't expect me to explain how to do it because I don't know
>what you don't know.
>
>--
>Konstantin Osipov, Moscow, Russia 
 
 
--
Ilya Kosarev
 

[-- Attachment #2: Type: text/html, Size: 1795 bytes --]

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

[Ilya Kosarev]

[-- Attachment #1: Type: text/plain, Size: 6617 bytes --]


Thanks for your review!

Sent tarantool/small patch and v4 of this patchset considering the
drawbacks. 
>Вторник, 21 января 2020, 23:59 +03:00 от Vladislav Shpilevoy <v.shpilevoy@tarantool.org>:
> 
>Thanks for the patch!
>
>I answer some of Nikita's questions below and
>provide my own review further.
>
>See 7 comments below.
>
>On 21/01/2020 12:42, Nikita Pettik wrote:
>> 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.
>
>1. On/off sounds good.
>
>>> +
>>> 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?
>
>2. This is a bug, yes. But not the only one. The upsert will write
>to WAL and yield. During that yield the quota is turned off. And
>any other fiber can interfere and overuse the quota even more with
>something not related to delete/truncate.
>
>This can be fixed, for example, by making txn_begin() + quota_off() +
>upsert() + quota_on() + commit(). Also we could just not touch truncate
>and fix delete() only. Delete() can never leave the quota overused,
>because is not stored anywhere in memtx. While truncate is stored in
>_truncate. And delete would be sufficient, because can be used to free
>the quota enough so as to call truncate()/whatever else the user wanted.
>
>>> +
>>> + 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.
>
>3. That problem is not solvable. There is always a chance, that truncate
>won't free any memory (for example, the target space could be empty
>already). If, after the quota is back on, it appears to be overused, nothing
>can be done with it. It will stay overused until more delete/truncate will be
>executed and actually delete something, or the user will increase memtx memory.
>
>> commit 45d9f491f91d93c8835ca725c234f7113c0d2aa8
>> Author: Ilya Kosarev < i.kosarev@tarantool.org >
>> Date: Mon Jan 20 20:21:37 2020 +0300
>>
>> memtx: allow quota overuse for truncation and deletion
>>
>>
>> Trying to perform space:truncate() and space:delete() 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 if needed for truncation or deletion, using flag
>> in struct quota. After performing truncation or deletion we reset the
>> flag so that quota can be overused any more.
>>
>> Closes #3807
>>
>> 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);
>> +
>> if (box_upsert(BOX_TRUNCATE_ID, 0, tuple_buf, tuple_buf_end,
>> ops_buf, ops_buf_end, 0, NULL) != 0)
>> diag_raise();
>> +
>> + memtx_engine_set_quota_strictness(truncate_space->engine, true);
>> }
>
>4. IMO, we should not touch quota at all until we see a not empty diag
>with OOM error type in it. Both here and in delete. Truncate is not
>a frequent operation (although it may be in some rare installations),
>but delete surely should not touch quota each time.
>
>> 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;
>
>5. That check should not exist. It slows down the caller, and
>in upper code you always know whether you work with memtx or
>not. So it makes no sense to check it here. Truncate is
>always about insertion into a memtx space. Delete touches qouta
>inside memtx_space implementation. No chances that a vinyl space
>would call this function.
>
>> + struct memtx_engine *memtx = (struct memtx_engine *)engine;
>> + quota_set_strictness(&memtx->quota, 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
>> @@ -1 +1 @@
>> -Subproject commit 50cb7874380b286f3b34c82299cf5eb88b0d0059
>> +Subproject commit bdcd569f9ed753efb805f708089ca86e2520a44f
>
>6. Please, write tests. They should include the bug test itself, and
>complex things such as quota overuse kept in case truncate() didn't
>free anything. You can use memtx_memory parameter to make it small
>enough so as the test would not take much time.
>
>7. Seems like you changed something in small/. You need to send it as
>a separate patch with its own tests, and all. 
 
 
--
Ilya Kosarev
 

[-- Attachment #2: Type: text/html, Size: 7916 bytes --]