[tarantool-patches] [PATCH v1] test: use unix sockets for iproto connections

[tarantool-patches] [PATCH v1] test: use unix sockets for iproto connections

[Alexander V. Tikhonov]

From: avtikhon <avtikhon@tarantool.org>

Enabled use_unix_sockets_iproto option to use unix sockets
iproto provides the new way to handle the problem with
'Address already in use' error. It lets test-run appoint
unix sockets for LISTEN environment variable values.

Before this change the TcpPortDispatcher was used to
eliminate the race condition when two workers trying to use
the same port: the idea was that each worker used its own
ports range. Really these ports could race with client ports
(from, say, net.box or replication), which typically didn't
use bind() and so bound to a random available port (despite
any dispatched ranges) and could produce 'Address already in
use' error.

Closes: #4008
---

Github: https://github.com/tarantool/tarantool/tree/avtikhon/gh-4008-test-in-parallel-iproto
Issue: https://github.com/tarantool/tarantool/issues/4008

 test/app/suite.ini               |  1 +
 test/box/suite.ini               |  1 +
 test/engine/suite.ini            |  1 +
 test/engine_long/suite.ini       |  1 +
 test/long_run-py/suite.ini       |  1 +
 test/replication/status.result   | 10 +++++-----
 test/replication/status.test.lua |  6 +++---
 test/replication/suite.ini       |  1 +
 test/sql/suite.ini               |  1 +
 test/vinyl/suite.ini             |  1 +
 test/wal_off/suite.ini           |  1 +
 test/xlog-py/suite.ini           |  1 +
 test/xlog/suite.ini              |  1 +
 13 files changed, 19 insertions(+), 8 deletions(-)

diff --git a/test/app/suite.ini b/test/app/suite.ini
index 4f9e51550..dd802d98c 100644
--- a/test/app/suite.ini
+++ b/test/app/suite.ini
@@ -4,5 +4,6 @@ description = application server tests
 script = app.lua
 lua_libs = lua/fiber.lua
 use_unix_sockets = True
+use_unix_sockets_iproto = True
 is_parallel = True
 pretest_clean = True
diff --git a/test/box/suite.ini b/test/box/suite.ini
index c7b75c173..9197e3c0f 100644
--- a/test/box/suite.ini
+++ b/test/box/suite.ini
@@ -6,5 +6,6 @@ disabled = rtree_errinj.test.lua tuple_bench.test.lua
 release_disabled = errinj.test.lua errinj_index.test.lua rtree_errinj.test.lua upsert_errinj.test.lua iproto_stress.test.lua
 lua_libs = lua/fifo.lua lua/utils.lua lua/bitset.lua lua/index_random_test.lua lua/push.lua lua/identifier.lua
 use_unix_sockets = True
+use_unix_sockets_iproto = True
 is_parallel = True
 pretest_clean = True
diff --git a/test/engine/suite.ini b/test/engine/suite.ini
index 64ddafa81..ee0814dd6 100644
--- a/test/engine/suite.ini
+++ b/test/engine/suite.ini
@@ -3,6 +3,7 @@ core = tarantool
 description = tarantool multiengine tests
 script = box.lua
 use_unix_sockets = True
+use_unix_sockets_iproto = True
 release_disabled = errinj.test.lua errinj_ddl.test.lua
 config = engine.cfg
 #disabled = replica_join.test.lua
diff --git a/test/engine_long/suite.ini b/test/engine_long/suite.ini
index 2447344fb..97d869042 100644
--- a/test/engine_long/suite.ini
+++ b/test/engine_long/suite.ini
@@ -5,6 +5,7 @@ script = box.lua
 long_run =  delete_replace_update.test.lua delete_insert.test.lua
 lua_libs = suite.lua
 use_unix_sockets = True
+use_unix_sockets_iproto = True
 config = engine.cfg
 is_parallel = True
 pretest_clean = True
diff --git a/test/long_run-py/suite.ini b/test/long_run-py/suite.ini
index 9050f9b77..110bbb548 100644
--- a/test/long_run-py/suite.ini
+++ b/test/long_run-py/suite.ini
@@ -7,4 +7,5 @@ valgrind_disabled =
 release_disabled =
 lua_libs = suite.lua
 use_unix_sockets = True
+use_unix_sockets_iproto = True
 is_parallel = True
diff --git a/test/replication/status.result b/test/replication/status.result
index f9d20a923..a1e51e435 100644
--- a/test/replication/status.result
+++ b/test/replication/status.result
@@ -234,9 +234,9 @@ master.upstream.idle < 1
 ---
 - true
 ...
-master.upstream.peer:match("localhost")
+master.upstream.peer:match("unix/")
 ---
-- localhost
+- unix/
 ...
 master.downstream == nil
 ---
@@ -348,7 +348,7 @@ test_run:cmd("set variable master_port to 'replica.master'")
 replica_uri = os.getenv("LISTEN")
 ---
 ...
-box.cfg{replication = {"guest@localhost:" .. master_port, replica_uri}}
+box.cfg{replication = {"guest@unix/:" .. master_port, replica_uri}}
 ---
 ...
 master_id = test_run:get_server_id('default')
@@ -369,9 +369,9 @@ master.upstream.peer:match("guest")
 ---
 - guest
 ...
-master.upstream.peer:match("localhost")
+master.upstream.peer:match("unix/")
 ---
-- localhost
+- unix/
 ...
 master.downstream == nil
 ---
diff --git a/test/replication/status.test.lua b/test/replication/status.test.lua
index 8a82fe9ae..a26fec74b 100644
--- a/test/replication/status.test.lua
+++ b/test/replication/status.test.lua
@@ -86,7 +86,7 @@ master.uuid == box.space._cluster:get(master_id)[2]
 master.upstream.status == "follow"
 master.upstream.lag < 1
 master.upstream.idle < 1
-master.upstream.peer:match("localhost")
+master.upstream.peer:match("unix/")
 master.downstream == nil
 
 -- replica's status
@@ -125,14 +125,14 @@ test_run:cmd("clear filter")
 test_run:cmd('switch replica')
 test_run:cmd("set variable master_port to 'replica.master'")
 replica_uri = os.getenv("LISTEN")
-box.cfg{replication = {"guest@localhost:" .. master_port, replica_uri}}
+box.cfg{replication = {"guest@unix/:" .. master_port, replica_uri}}
 
 master_id = test_run:get_server_id('default')
 master = box.info.replication[master_id]
 master.id == master_id
 master.upstream.status == "follow"
 master.upstream.peer:match("guest")
-master.upstream.peer:match("localhost")
+master.upstream.peer:match("unix/")
 master.downstream == nil
 
 test_run:cmd('switch default')
diff --git a/test/replication/suite.ini b/test/replication/suite.ini
index 0ca85933b..ac35b94a7 100644
--- a/test/replication/suite.ini
+++ b/test/replication/suite.ini
@@ -7,6 +7,7 @@ release_disabled = catch.test.lua errinj.test.lua gc.test.lua gc_no_space.test.l
 config = suite.cfg
 lua_libs = lua/fast_replica.lua lua/rlimit.lua
 use_unix_sockets = True
+use_unix_sockets_iproto = True
 long_run = prune.test.lua
 is_parallel = True
 pretest_clean = True
diff --git a/test/sql/suite.ini b/test/sql/suite.ini
index ebaf81a53..03c0175bf 100644
--- a/test/sql/suite.ini
+++ b/test/sql/suite.ini
@@ -3,6 +3,7 @@ core = tarantool
 description = sql tests
 script = app.lua
 use_unix_sockets = True
+use_unix_sockets_iproto = True
 config = engine.cfg
 is_parallel = True
 lua_libs = lua/sql_tokenizer.lua
diff --git a/test/vinyl/suite.ini b/test/vinyl/suite.ini
index f6783105f..54e6618d6 100644
--- a/test/vinyl/suite.ini
+++ b/test/vinyl/suite.ini
@@ -6,6 +6,7 @@ release_disabled = errinj.test.lua errinj_ddl.test.lua errinj_gc.test.lua errinj
 config = suite.cfg
 lua_libs = suite.lua stress.lua large.lua txn_proxy.lua ../box/lua/utils.lua
 use_unix_sockets = True
+use_unix_sockets_iproto = True
 long_run = stress.test.lua large.test.lua write_iterator_rand.test.lua dump_stress.test.lua select_consistency.test.lua throttle.test.lua
 is_parallel = True
 # throttle.test.lua temporary disabled for gh-4168
diff --git a/test/wal_off/suite.ini b/test/wal_off/suite.ini
index b384c0a74..637678c26 100644
--- a/test/wal_off/suite.ini
+++ b/test/wal_off/suite.ini
@@ -3,6 +3,7 @@ core = tarantool
 script = wal.lua
 description = tarantool/box, wal_mode = none
 use_unix_sockets = True
+use_unix_sockets_iproto = True
 is_parallel = True
 pretest_clean = True
 disabled = iterator_lt_gt.test.lua
diff --git a/test/xlog-py/suite.ini b/test/xlog-py/suite.ini
index 18737e48f..39440d859 100644
--- a/test/xlog-py/suite.ini
+++ b/test/xlog-py/suite.ini
@@ -4,4 +4,5 @@ description = legacy python tests
 script = box.lua
 lua_libs = lua/fiber.lua lua/fifo.lua
 use_unix_sockets = True
+use_unix_sockets_iproto = True
 is_parallel = True
diff --git a/test/xlog/suite.ini b/test/xlog/suite.ini
index 06cfbb068..689d2b871 100644
--- a/test/xlog/suite.ini
+++ b/test/xlog/suite.ini
@@ -7,6 +7,7 @@ valgrind_disabled =
 release_disabled = errinj.test.lua panic_on_lsn_gap.test.lua panic_on_broken_lsn.test.lua checkpoint_threshold.test.lua
 config = suite.cfg
 use_unix_sockets = True
+use_unix_sockets_iproto = True
 long_run = snap_io_rate.test.lua
 is_parallel = True
 pretest_clean = True
-- 
2.17.1

[tarantool-patches] Re: [PATCH v1] test: use unix sockets for iproto connections

[Kirill Yukhin]

Hello,

On 07 Jun 07:21, Alexander V. Tikhonov wrote:
> From: avtikhon <avtikhon@tarantool.org>
> 
> Enabled use_unix_sockets_iproto option to use unix sockets
> iproto provides the new way to handle the problem with
> 'Address already in use' error. It lets test-run appoint
> unix sockets for LISTEN environment variable values.
> 
> Before this change the TcpPortDispatcher was used to
> eliminate the race condition when two workers trying to use
> the same port: the idea was that each worker used its own
> ports range. Really these ports could race with client ports
> (from, say, net.box or replication), which typically didn't
> use bind() and so bound to a random available port (despite
> any dispatched ranges) and could produce 'Address already in
> use' error.
> 
> Closes: #4008
> ---
> 
> Github: https://github.com/tarantool/tarantool/tree/avtikhon/gh-4008-test-in-parallel-iproto
> Issue: https://github.com/tarantool/tarantool/issues/4008

I've checked your patch into master and 2.1 branch.

--
Regards, Kirill Yukhin

[tarantool-patches] Re: [PATCH v1] test: use unix sockets for iproto connections

[Alexander Turenko]

On Fri, Jun 07, 2019 at 08:20:42AM +0300, Kirill Yukhin wrote:
> Hello,
> 
> On 07 Jun 07:21, Alexander V. Tikhonov wrote:
> > From: avtikhon <avtikhon@tarantool.org>
> > 
> > Enabled use_unix_sockets_iproto option to use unix sockets
> > iproto provides the new way to handle the problem with
> > 'Address already in use' error. It lets test-run appoint
> > unix sockets for LISTEN environment variable values.
> > 
> > Before this change the TcpPortDispatcher was used to
> > eliminate the race condition when two workers trying to use
> > the same port: the idea was that each worker used its own
> > ports range. Really these ports could race with client ports
> > (from, say, net.box or replication), which typically didn't
> > use bind() and so bound to a random available port (despite
> > any dispatched ranges) and could produce 'Address already in
> > use' error.
> > 
> > Closes: #4008
> > ---
> > 
> > Github: https://github.com/tarantool/tarantool/tree/avtikhon/gh-4008-test-in-parallel-iproto
> > Issue: https://github.com/tarantool/tarantool/issues/4008
> 
> I've checked your patch into master and 2.1 branch.

I think it is needed for 1.10 too.

[tarantool-patches] Re: [PATCH v1] test: use unix sockets for iproto connections

[Alexander Turenko]

Alexander, I have already answered for this patch. Please, read the
previous review:

https://www.freelists.org/post/tarantool-patches/PATCH-v1-test-use-unix-sockets-for-iproto-connections,3

WBR, Alexander Turenko.

On Mon, Jul 08, 2019 at 03:03:49PM +0300, Alexander V. Tikhonov wrote:
> Use unix sockets for iproto connections in sql-tap suite.
> Enabled use_unix_sockets_iproto option to use unix sockets
> iproto provides the new way to handle the problem with
> 'Address already in use' error. It lets test-run appoint
> unix sockets for LISTEN environment variable values.
> Check the previous commit for the other suites:
> 
> 60f84cbfca24e3a91cea067c923e006b44ee589f ('test: use unix sockets for iproto connections')
> 
> Closes #4008
> ---
> 
> Github: https://github.com/tarantool/tarantool/tree/avtikhon/avtikhon/use_unix_sockets_iproto_sql_tap
> Issue: https://github.com/tarantool/tarantool/issues/4008
> 
>  test/sql-tap/suite.ini | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/test/sql-tap/suite.ini b/test/sql-tap/suite.ini
> index 2dfae4442..cba1d1fc6 100644
> --- a/test/sql-tap/suite.ini
> +++ b/test/sql-tap/suite.ini
> @@ -24,6 +24,8 @@ disabled = selectA.test.lua ;
>  
>  lua_libs = lua/sqltester.lua ../sql/lua/sql_tokenizer.lua ../box/lua/identifier.lua
>  is_parallel = True
> +use_unix_sockets = True
> +use_unix_sockets_iproto = True
>  release_disabled = debug_mode_only.test.lua
>  long_run = gh-3332-tuple-format-leak.test.lua, gh-3083-ephemeral-unref-tuples.test.lua
>  config = engine.cfg
> -- 
> 2.17.1
> 

[tarantool-patches] Re: [PATCH v1] test: use unix sockets for iproto connections

[Alexander Turenko]

Sorry, my bad. I forgot that I had implemented it only for core =
tarantool test suites.

So first implement it in test-run for core = app suites, please.

WBR, Alexander Turenko.

> test: use unix sockets for iproto connections

It is not good to have two different commits with the same commit
message header (say, you use a header to search a commit that is
cherry-picked from another one). I think it worth to mention that the
commit is about sql-tap if the 50 symbols limit allows.

> Use unix sockets for iproto connections in sql-tap suite.
> Enabled use_unix_sockets_iproto option to use unix sockets
> iproto provides the new way to handle the problem with

'Use unix sockets iproto' sound as a bag of words.

> 'Address already in use' error. It lets test-run appoint
> unix sockets for LISTEN environment variable values.
> Check the previous commit for the other suites:
> 
> 60f84cbfca24e3a91cea067c923e006b44ee589f ('test: use unix sockets for iproto connections')
> 
> Closes #4008

The issue is already closed, it is better to say 'Follows up #4008'.

> ---
> 
> Github: https://github.com/tarantool/tarantool/tree/avtikhon/use_unix_sockets_iproto_sql_tap
> Issue: https://github.com/tarantool/tarantool/issues/4008
> 
>  test/sql-tap/suite.ini | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/test/sql-tap/suite.ini b/test/sql-tap/suite.ini
> index 2dfae4442..cba1d1fc6 100644
> --- a/test/sql-tap/suite.ini
> +++ b/test/sql-tap/suite.ini
> @@ -24,6 +24,8 @@ disabled = selectA.test.lua ;
>  
>  lua_libs = lua/sqltester.lua ../sql/lua/sql_tokenizer.lua ../box/lua/identifier.lua
>  is_parallel = True
> +use_unix_sockets = True

As far as I see this option does not affect anything for core = app test
suites. It is about ADMIN env variable, it is not used in app tests.

Aside of that this is not mentioned in the commit message.

> +use_unix_sockets_iproto = True
>  release_disabled = debug_mode_only.test.lua
>  long_run = gh-3332-tuple-format-leak.test.lua, gh-3083-ephemeral-unref-tuples.test.lua
>  config = engine.cfg
> -- 
> 2.17.1
> 

Sasha, please check that the patch really works next time. It is in your
responsibility.