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

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

[avtikhon]

Set to use unix sockets iproto instead of TCP sockets.
---

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/replica_rejoin.lua    |  2 +-
 test/vinyl/replica_rejoin.result | 20 ++++++--------------
 test/vinyl/suite.ini             |  1 +
 test/wal_off/suite.ini           |  1 +
 test/xlog-py/suite.ini           |  1 +
 test/xlog/suite.ini              |  1 +
 15 files changed, 26 insertions(+), 23 deletions(-)

diff --git a/test/app/suite.ini b/test/app/suite.ini
index 7f3c0a8f5..d79ebc2de 100644
--- a/test/app/suite.ini
+++ b/test/app/suite.ini
@@ -4,4 +4,5 @@ description = application server tests
 script = app.lua
 lua_libs = lua/fiber.lua
 use_unix_sockets = True
+use_unix_sockets_iproto = True
 is_parallel = True
diff --git a/test/box/suite.ini b/test/box/suite.ini
index fee1c40b4..221004e93 100644
--- a/test/box/suite.ini
+++ b/test/box/suite.ini
@@ -6,4 +6,5 @@ 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
diff --git a/test/engine/suite.ini b/test/engine/suite.ini
index 3db02ab6f..2fb14a49a 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
 config = engine.cfg
 #disabled = replica_join.test.lua
diff --git a/test/engine_long/suite.ini b/test/engine_long/suite.ini
index 66df26715..0ebb7c9f8 100644
--- a/test/engine_long/suite.ini
+++ b/test/engine_long/suite.ini
@@ -5,5 +5,6 @@ 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
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 dd5b01405..46a118e61 100644
--- a/test/replication/suite.ini
+++ b/test/replication/suite.ini
@@ -7,5 +7,6 @@ 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
diff --git a/test/sql/suite.ini b/test/sql/suite.ini
index bfe0fa063..a7b0e4d4c 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/replica_rejoin.lua b/test/vinyl/replica_rejoin.lua
index 7cb7e09a4..9b95d50aa 100644
--- a/test/vinyl/replica_rejoin.lua
+++ b/test/vinyl/replica_rejoin.lua
@@ -7,7 +7,7 @@ end
 
 box.cfg({
     replication     = replication,
-    vinyl_memory    = 1024 * 1024,
+    vinyl_memory    = 107374182,
 })
 
 require('console').listen(os.getenv('ADMIN'))
diff --git a/test/vinyl/replica_rejoin.result b/test/vinyl/replica_rejoin.result
index 1dfcb91b8..f590e05b2 100644
--- a/test/vinyl/replica_rejoin.result
+++ b/test/vinyl/replica_rejoin.result
@@ -51,8 +51,6 @@ fio.glob(fio.pathjoin(box.space.test.id, 0, '*'))
 ---
 - - 9000/0/00000000000000000002.index
   - 9000/0/00000000000000000002.run
-  - 9000/0/00000000000000000004.index
-  - 9000/0/00000000000000000004.run
 ...
 test_run:cmd("switch default")
 ---
@@ -106,10 +104,8 @@ fio.chdir(box.cfg.vinyl_dir)
 ...
 fio.glob(fio.pathjoin(box.space.test.id, 0, '*'))
 ---
-- - 9000/0/00000000000000000008.index
-  - 9000/0/00000000000000000008.run
-  - 9000/0/00000000000000000010.index
-  - 9000/0/00000000000000000010.run
+- - 9000/0/00000000000000000006.index
+  - 9000/0/00000000000000000006.run
 ...
 box.space.test:count() -- 99
 ---
@@ -173,10 +169,8 @@ fio.chdir(box.cfg.vinyl_dir)
 ...
 fio.glob(fio.pathjoin(box.space.test.id, 0, '*'))
 ---
-- - 9000/0/00000000000000000008.index
-  - 9000/0/00000000000000000008.run
-  - 9000/0/00000000000000000010.index
-  - 9000/0/00000000000000000010.run
+- - 9000/0/00000000000000000006.index
+  - 9000/0/00000000000000000006.run
 ...
 box.space.test:count() -- 99
 ---
@@ -220,10 +214,8 @@ fio.chdir(box.cfg.vinyl_dir)
 ...
 fio.glob(fio.pathjoin(box.space.test.id, 0, '*'))
 ---
-- - 9000/0/00000000000000000022.index
-  - 9000/0/00000000000000000022.run
-  - 9000/0/00000000000000000024.index
-  - 9000/0/00000000000000000024.run
+- - 9000/0/00000000000000000014.index
+  - 9000/0/00000000000000000014.run
 ...
 box.space.test:count() -- 98
 ---
diff --git a/test/vinyl/suite.ini b/test/vinyl/suite.ini
index d2a194d85..cc6356a86 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 = False
 disabled = upgrade.test.lua
diff --git a/test/wal_off/suite.ini b/test/wal_off/suite.ini
index bd3a349db..45cc0b111 100644
--- a/test/wal_off/suite.ini
+++ b/test/wal_off/suite.ini
@@ -3,4 +3,5 @@ core = tarantool
 script = wal.lua
 description = tarantool/box, wal_mode = none
 use_unix_sockets = True
+use_unix_sockets_iproto = True
 is_parallel = True
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 4043f3700..29bef1e0b 100644
--- a/test/xlog/suite.ini
+++ b/test/xlog/suite.ini
@@ -7,5 +7,6 @@ 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
-- 
2.17.1

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

[Alexander Turenko]

Hi!

Please, do self-review before send a patch. I'm sure you would ask the
same questions that I asked below and either fix them or describe
reasons in the commit message.

WBR, Alexander Turenko.

On Thu, Apr 04, 2019 at 11:49:50AM +0300, avtikhon wrote:
> Set to use unix sockets iproto instead of TCP sockets.

A reason? Yep, I know, it is avoiding 'Address already in use' errors.
And I know that it is because port checking in test-run is not
sufficient and races are possible. But a reader of your commit unlikely
have enough context.

It worth to add a link to related test-run issue here.

And don't forget to add fixes / closes #xxxx and the end of a commit
message.

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

> diff --git a/test/vinyl/replica_rejoin.lua b/test/vinyl/replica_rejoin.lua
> index 7cb7e09a4..9b95d50aa 100644
> --- a/test/vinyl/replica_rejoin.lua
> +++ b/test/vinyl/replica_rejoin.lua
> @@ -7,7 +7,7 @@ end
>  
>  box.cfg({
>      replication     = replication,
> -    vinyl_memory    = 1024 * 1024,
> +    vinyl_memory    = 107374182,
>  })

How it related to the patch?

>  require('console').listen(os.getenv('ADMIN'))
> diff --git a/test/vinyl/replica_rejoin.result b/test/vinyl/replica_rejoin.result
> index 1dfcb91b8..f590e05b2 100644
> --- a/test/vinyl/replica_rejoin.result
> +++ b/test/vinyl/replica_rejoin.result
> @@ -51,8 +51,6 @@ fio.glob(fio.pathjoin(box.space.test.id, 0, '*'))
>  ---
>  - - 9000/0/00000000000000000002.index
>    - 9000/0/00000000000000000002.run
> -  - 9000/0/00000000000000000004.index
> -  - 9000/0/00000000000000000004.run

And this.

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

[Alexander Turenko]

I'm ok with the change itself, so formally LGTM. Please, proceed further
with Kirill.

WBR, Alexander Turenko.

> test: use unix sockets iproto

Typo: iproto -> for iproto. Maybe better 'test: use unix sockets for
iproto connections'.

First of all I would state explicitly 'why': "eliminated most of
'address already in use' errors seen from time to time in our testing".
Then you can describe the reason of those fails, provide information
about previous tries to solve it, give needed background and so on.

> Enabled use_unix_sockets and use_unix_sockets_iproto options

use_unix_sockets is already enabled, the phrase is misleading.

> to use unix sockets iproto instead of TcpPortDispatcher which
> was previously introduced 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.

I would separate a description of the new way to handle the problem
('Enabled use_unix_sockets_iproto option to let test-run appoint unix
sockets for LISTEN environment variable values') and a description of
previous approaches ('TCP port ranges and so on') if you really want to
give enough context and describe the latter one entirely. It is not
obligatory I think.

I mean, now it is not easy to understand that you describe disadvantages
on a *previous* approach and almost all this paragraph is not about a
code that is enabled in this commit.

I think it can mislead someone if (s)he has less knowledge around
test-run then you. You are formally right however, so ignore my wording
nitpicking if you don't agree and just proceed with Kirill.

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