[tarantool-patches] Re: [PATCH v2 2/2] iproto: allow to configure IPROTO_MSG_MAX

[Konstantin Osipov <kostja@tarantool.org>]

* Vladislav Shpilevoy <v.shpilevoy@tarantool.org> [18/05/04 00:07]:

> -struct iproto_bind_msg: public cbus_call_msg
> +struct iproto_cfg_msg: public cbus_call_msg
>  {
> +	/** New URI to bind to. */
>  	const char *uri;
> +	bool need_update_uri;
> +
> +	/** New IProto max message count in fly. */
> +	int iproto_msg_max;
> +	bool need_update_msg_max;
>  };

need_update -> update

> +void
> +fiber_pool_set_max_size(struct fiber_pool *pool, int new_max_size)
> +{
> +	if (new_max_size < FIBER_POOL_SIZE_DEFAULT)
> +		new_max_size = FIBER_POOL_SIZE_DEFAULT;
> +
> +	if (new_max_size > pool->max_size) {
> +		pool->max_size = new_max_size;
> +		cbus_process(&pool->endpoint);
> +	} else {
> +		pool->max_size = new_max_size;
> +	}

Why do you need cbus_process here? This is error-prone. Please
remove.

> +--
> +-- Test TX fiber pool size limit. It is increased together with
> +-- iproto msg max.
> +--
> +run_max = 2500
> +---
> +...
> +box.cfg{net_msg_max = 5000}

Please reduce the number of messages/workers in your tests to several or
maybe a dozen, not hundreds or thousands. By having a lot of
fibers/messages you don't test anything new, only make
the test run longer and harder to debug.

The test would win from adding more randomness and more tries to
changes in msg max: e.g. why not increase/decrease msg max a few
times under unceasing load? This would test both growing the fiber
pool and shrinking it.

> +---
> +...
> +run_workers(conn)
> +---
> +...
> +run_workers(conn2)
> +---
> +...
> +wait_active(5000)
> +---
> +...
> +wait_finished(run_max * 2)
> +---
> +...
>  conn2:close()
>  ---
>  ...
> @@ -130,6 +243,6 @@ conn:close()
>  box.schema.user.revoke('guest', 'read,write,execute', 'universe')
>  ---
>  ...
> -box.cfg{readahead = old_readahead}
> +box.cfg{readahead = old_readahead, net_msg_max = limit}
>  ---
>  ...
> diff --git a/test/box/net_msg_max.test.lua b/test/box/net_msg_max.test.lua
> index 39f8f53f7..887d73ddc 100644
> --- a/test/box/net_msg_max.test.lua
> +++ b/test/box/net_msg_max.test.lua
> @@ -9,7 +9,7 @@ conn2 = net_box.connect(box.cfg.listen)
>  active = 0
>  finished = 0
>  continue = false
> -limit = 768
> +limit = box.cfg.net_msg_max
>  run_max = (limit - 100) / 2
>  
>  old_readahead = box.cfg.readahead
> @@ -72,8 +72,63 @@ run_workers(conn)
>  wait_active(limit + 1)
>  wait_finished(run_max)
>  
> +--
> +-- gh-3320: allow to change maximal count of messages.
> +--
> +
> +--
> +-- Test minimal iproto msg count.
> +--
> +box.cfg{net_msg_max = 2}
> +conn:ping()
> +#conn.space._space:select{} > 0
> +run_max = 15
> +run_workers(conn)
> +wait_active(3)
> +active
> +wait_finished(run_max)
> +
> +--
> +-- Increate maximal message count when nothing is blocked.
> +--
> +box.cfg{net_msg_max = limit * 2}
> +run_max = limit * 2 - 100
> +run_workers(conn)
> +wait_active(run_max)
> +-- Max can be decreased back even if now the limit is violated.
> +-- But a new input is blocked in such a case.
> +box.cfg{net_msg_max = limit}
> +old_active = active
> +for i = 1, 300 do fiber.create(do_long, conn) end
> +-- Afer time active count is not changed - the input is blocked.
> +wait_active(old_active)
> +wait_finished(active + 300)
> +
> +--
> +-- Check that changing net_msg_max can resume stopped
> +-- connections.
> +--
> +run_max = limit / 2 + 100
> +run_workers(conn)
> +run_workers(conn2)
> +wait_active(limit + 1)
> +box.cfg{net_msg_max = limit * 2}
> +wait_active(run_max * 2)
> +wait_finished(active)
> +
> +--
> +-- Test TX fiber pool size limit. It is increased together with
> +-- iproto msg max.
> +--
> +run_max = 2500
> +box.cfg{net_msg_max = 5000}
> +run_workers(conn)
> +run_workers(conn2)
> +wait_active(5000)
> +wait_finished(run_max * 2)
> +

-- 
Konstantin Osipov, Moscow, Russia, +7 903 626 22 32
http://tarantool.io - www.twitter.com/kostja_osipov