Re: [Tarantool-patches] [PATCH v2] Add replication_connect_timeout to quorum test

[Serge Petrenko <sergepetrenko@tarantool.org>]

03.12.2020 13:14, Alexander V. Tikhonov пишет:
> Added replication_connect_timeout to replication/*quorum.lua scripts to
> decrease replication/quorum.test.lua test run time in 2 times which was
> 150 seconds before it. Before the patch this test run time was near to
> 'test-timeout' limit of 110 seconds and even to 120 seconds of
> 'no-output-timeout' limit. It caused test to fail because of it. Also
> the test uses to wait for 3rd replica till it will be connected and
> this timeout helps to avoid of long waitings.
> ---


Hi! Thanks for the patch! Looks good, just one question.

Is it possible to set replication_connect_timeout to a smaller value?
Say, 5 seconds or less. WIll the test pass reliably under load then?

Ideally we may make the test run as fast as we want by decreasing the 
timeout.

But I'm not sure it won't hurt stability.


>
> Github: https://github.com/tarantool/tarantool/tree/avtikhon/test_quorum_timeout
>
>   test/replication/quorum.lua         | 4 +++-
>   test/replication/quorum.result      | 8 ++++----
>   test/replication/quorum.test.lua    | 8 ++++----
>   test/replication/replica_quorum.lua | 2 ++
>   4 files changed, 13 insertions(+), 9 deletions(-)
>
> diff --git a/test/replication/quorum.lua b/test/replication/quorum.lua
> index 129e676e0..616b6b607 100644
> --- a/test/replication/quorum.lua
> +++ b/test/replication/quorum.lua
> @@ -5,7 +5,8 @@ local INSTANCE_ID = string.match(arg[0], "%d")
>   
>   local SOCKET_DIR = require('fio').cwd()
>   
> -local TIMEOUT = tonumber(arg[1])
> +local TIMEOUT = arg[1] and tonumber(arg[1]) or 0.1
> +local CONNECT_TIMEOUT = arg[2] and tonumber(arg[2]) or 10
>   
>   local function instance_uri(instance_id)
>       --return 'localhost:'..(3310 + instance_id)
> @@ -18,6 +19,7 @@ require('console').listen(os.getenv('ADMIN'))
>   box.cfg({
>       listen = instance_uri(INSTANCE_ID);
>       replication_timeout = TIMEOUT;
> +    replication_connect_timeout = CONNECT_TIMEOUT;
>       replication_sync_lag = 0.01;
>       replication_connect_quorum = 3;
>       replication = {
> diff --git a/test/replication/quorum.result b/test/replication/quorum.result
> index 4b90d13cb..dc47a8e38 100644
> --- a/test/replication/quorum.result
> +++ b/test/replication/quorum.result
> @@ -27,7 +27,7 @@ test_run:cmd('switch quorum2')
>   ---
>   - true
>   ...
> -test_run:cmd('restart server quorum2 with args="0.1"')
> +test_run:cmd('restart server quorum2 with args="0.1 10"')
>   box.info.status -- orphan
>   ---
>   - orphan
> @@ -55,7 +55,7 @@ box.info.status -- running
>   ---
>   - running
>   ...
> -test_run:cmd('restart server quorum2 with args="0.1"')
> +test_run:cmd('restart server quorum2 with args="0.1 10"')
>   box.info.status -- orphan
>   ---
>   - orphan
> @@ -86,7 +86,7 @@ box.info.status -- running
>   ---
>   - running
>   ...
> -test_run:cmd('restart server quorum2 with args="0.1"')
> +test_run:cmd('restart server quorum2 with args="0.1 10"')
>   box.info.status -- orphan
>   ---
>   - orphan
> @@ -427,7 +427,7 @@ test_run:cmd('create server replica_quorum with script="replication/replica_quor
>   -- Arguments are: replication_connect_quorum, replication_timeout
>   -- If replication_connect_quorum was ignored here, the instance
>   -- would exit with an error.
> -test_run:cmd('start server replica_quorum with wait=True, wait_load=True, args="1 0.05"')
> +test_run:cmd('start server replica_quorum with wait=True, wait_load=True, args="1 0.05 10"')
>   ---
>   - true
>   ...
> diff --git a/test/replication/quorum.test.lua b/test/replication/quorum.test.lua
> index 7c4b76671..d2b3da587 100644
> --- a/test/replication/quorum.test.lua
> +++ b/test/replication/quorum.test.lua
> @@ -18,7 +18,7 @@ test_run:cmd('stop server quorum1')
>   
>   test_run:cmd('switch quorum2')
>   
> -test_run:cmd('restart server quorum2 with args="0.1"')
> +test_run:cmd('restart server quorum2 with args="0.1 10"')
>   box.info.status -- orphan
>   box.ctl.wait_rw(0.001) -- timeout
>   box.info.ro -- true
> @@ -27,7 +27,7 @@ box.space.test:replace{100} -- error
>   box.cfg{replication={}}
>   box.info.status -- running
>   
> -test_run:cmd('restart server quorum2 with args="0.1"')
> +test_run:cmd('restart server quorum2 with args="0.1 10"')
>   box.info.status -- orphan
>   box.ctl.wait_rw(0.001) -- timeout
>   box.info.ro -- true
> @@ -37,7 +37,7 @@ box.ctl.wait_rw()
>   box.info.ro -- false
>   box.info.status -- running
>   
> -test_run:cmd('restart server quorum2 with args="0.1"')
> +test_run:cmd('restart server quorum2 with args="0.1 10"')
>   box.info.status -- orphan
>   box.ctl.wait_rw(0.001) -- timeout
>   box.info.ro -- true
> @@ -161,7 +161,7 @@ test_run:cmd('create server replica_quorum with script="replication/replica_quor
>   -- Arguments are: replication_connect_quorum, replication_timeout
>   -- If replication_connect_quorum was ignored here, the instance
>   -- would exit with an error.
> -test_run:cmd('start server replica_quorum with wait=True, wait_load=True, args="1 0.05"')
> +test_run:cmd('start server replica_quorum with wait=True, wait_load=True, args="1 0.05 10"')
>   test_run:cmd('switch replica_quorum')
>   -- If replication_connect_quorum was ignored here, the instance
>   -- would exit with an error.
> diff --git a/test/replication/replica_quorum.lua b/test/replication/replica_quorum.lua
> index fe51da802..8dfad8b6e 100644
> --- a/test/replication/replica_quorum.lua
> +++ b/test/replication/replica_quorum.lua
> @@ -4,6 +4,7 @@ local SOCKET_DIR = require('fio').cwd()
>   
>   local QUORUM = tonumber(arg[1])
>   local TIMEOUT = arg[2] and tonumber(arg[2]) or 0.1
> +local CONNECT_TIMEOUT = arg[3] and tonumber(arg[3]) or 10
>   INSTANCE_URI = SOCKET_DIR .. '/replica_quorum.sock'
>   
>   function nonexistent_uri(id)
> @@ -15,6 +16,7 @@ require('console').listen(os.getenv('ADMIN'))
>   box.cfg{
>       listen = INSTANCE_URI,
>       replication_timeout = TIMEOUT,
> +    replication_connect_timeout = CONNECT_TIMEOUT,
>       replication_connect_quorum = QUORUM,
>       replication = {INSTANCE_URI,
>                      nonexistent_uri(1),

-- 
Serge Petrenko