Thanks for the patch!
JFI, I am still against this patch. It adds huge and
unnecessary complexity to the code, which we will need
to support forever. It is just not worth the pros the
patch gives.
On 13/01/2020 22:31, Ilya Kosarev wrote:
> 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 increasing
> memtx quota if needed for truncation or deletion. After performing it
> quota is being set back to the previous value if possible, while it
> should be so for almost any case, since we are meant to free some space
> during deletion or truncation.
>
> Closes #3807
> ---
> src/box/blackhole.c | 1 +
> src/box/box.cc | 36 +++++++++++++++++++++++++++++++++++-
> src/box/engine.c | 11 +++++++++++
> src/box/engine.h | 9 +++++++++
> src/box/memtx_engine.c | 20 ++++++++++++++++++++
> src/box/memtx_engine.h | 4 ++++
> src/box/service_engine.c | 1 +
> src/box/sysview.c | 1 +
> src/box/vinyl.c | 1 +
> 9 files changed, 83 insertions(+), 1 deletion(-)
>
> diff --git a/src/box/blackhole.c b/src/box/blackhole.c
> index 69f1deba1..af587f434 100644
> --- a/src/box/blackhole.c
> +++ b/src/box/blackhole.c
> @@ -194,6 +194,7 @@ static const struct engine_vtab blackhole_engine_vtab = {
> /* .commit_checkpoint = */ generic_engine_commit_checkpoint,
> /* .abort_checkpoint = */ generic_engine_abort_checkpoint,
> /* .collect_garbage = */ generic_engine_collect_garbage,
> + /* .guarantee_memory = */ generic_engine_guarantee_memory,
The only problem is with memtx engine, and I propose to solve it
on memtx engine level. Vinyl will never need this method.
(But even better I propose to drop the patch and close the issue as
won't fix.)
> /* .backup = */ generic_engine_backup,
> /* .memory_stat = */ generic_engine_memory_stat,
> /* .reset_stat = */ generic_engine_reset_stat,
> diff --git a/src/box/box.cc b/src/box/box.cc
> index 1b2b27d61..18c09ce1b 100644
> --- a/src/box/box.cc
> +++ b/src/box/box.cc
> @@ -1321,9 +1341,23 @@ space_truncate(struct space *space)
> ops_buf_end = mp_encode_uint(ops_buf_end, 1);
> assert(ops_buf_end < buf + buf_size);
>
> + size_t total;
> + bool extended = false;
> + space->engine->vtab->guarantee_memory(space->engine,
> + MEMTX_SLAB_SIZE,
> + &total, &extended);
> +
Truncate is always about insertion into the memtx space _truncate.
Here you are calling 'guarantee_memory' for the user space's engine.
And it just won't work in case I try to truncate a vinyl space.
Moreover, the encapsulation of 'memory guarantee' is broken anyway,
because 1) you pass 'MEMTX_SLAB_SIZE' parameter to the engine's
virtual method, 2) below you touch memtx engine explicitly.