From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <vdavydov.dev@gmail.com>
Date: Wed, 30 May 2018 19:08:25 +0300
From: Vladimir Davydov <vdavydov.dev@gmail.com>
Subject: Re: [PATCH] Allow to increase box.cfg.vinyl_memory and memtx_memory
 at runtime
Message-ID: <20180530160825.say4ia6morjcme6k@esperanza>
References: <5c833e39a3337fdf832ebd745ad89d901cf4c92e.1527684463.git.vdavydov.dev@gmail.com>
 <20180530154107.GA14567@atlas>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20180530154107.GA14567@atlas>
To: Konstantin Osipov <kostja@tarantool.org>
Cc: tarantool-patches@freelists.org
List-ID: <tarantool-patches.dev.tarantool.org>

On Wed, May 30, 2018 at 06:41:07PM +0300, Konstantin Osipov wrote:
> * Vladimir Davydov <vdavydov.dev@gmail.com> [18/05/30 15:55]:
> > Closes #2634
> > ---
> > https://github.com/tarantool/tarantool/issues/2634
> > https://github.com/tarantool/tarantool/commits/gh-2634-allow-to-increase-memory-size
> 
> It should still be possible to set the limit to the same size, 
> what do you think?

It is possible - box.cfg{memtx_memory = box.cfg.memtx_memory} works with
this patch.

> > +int
> > +memtx_engine_set_memory(struct memtx_engine *memtx, size_t size)
> > +{
> > +	if (size < quota_total(&memtx->quota)) {
> 
> Shouldn't this be <= rather than < ?

I don't think so - we shouldn't raise error if the new limit equals the
current one.

> 
> > +		diag_set(ClientError, ER_CFG, "memtx_memory",
> > +			 "cannot decrease memory size at runtime");
> > +		return -1;
> > +	}
> > +	quota_set(&memtx->quota, size);
> > +	return 0;
> > +}
> > +int
> > +vinyl_engine_set_memory(struct vinyl_engine *vinyl, size_t size)
> > +{
> > +	if (size < vinyl->env->quota.limit) {
> 
> Same here.
> 
> > +		diag_set(ClientError, ER_CFG, "vinyl_memory",
> > +			 "cannot decrease memory size at runtime");
> > +		return -1;
> > +	}
> > +	vy_quota_set_limit(&vinyl->env->quota, size);
> > +	return 0;
> > +}
> >  
> >  /**
> > index 00000000..e3e89393
> > --- /dev/null
> > +++ b/test/box/lua/cfg_memory.lua
> > @@ -0,0 +1,9 @@
> > +#!/usr/bin/env tarantool
> 
> Adding wal_mode=none will speed things up a bit when testing for
> memtx at least.
> 
> Similar tests reside in wal_mode suite.

OK, I'll move the test to wal_off suite.

> 
> > +
> > +local LIMIT = tonumber(arg[1])
> > +
> > +box.cfg{
> > +    memtx_memory = LIMIT,
> > +}
> > +
> > +---
> > +- error: 'Incorrect value for option ''memtx_memory'': cannot decrease memory size
> > +    at runtime'
> > +...
> > +box.slab.info().quota_size
> > +---
> > +- 268435456
> 
> As usual, I'd prefer a less cpu intensive test. E.g. do you have
> to double the amount of memory ? Perhaps only slightly increasing it
> would test the same even better?  
> The same remark applies to vinyl test.

OK, I'll try to tune the test.