Hi! Thanks for the comments. Sent v6 of the patch considering them. >Четверг, 26 декабря 2019, 2:43 +03:00 от Alexander Turenko : > >Now it works like a charm with 30 jobs and mostly works with 50 jobs in >parallel on my machine. This is the good change. > >A few notes below. > >WBR, Alexander Turenko. > >> Branch: https://github.com/tarantool/tarantool/tree/i.kosarev/gh-4426-4451-fix-socket-test >> Issues: https://github.com/tarantool/tarantool/issues/4426 >> https://github.com/tarantool/tarantool/issues/4451 >> https://github.com/tarantool/tarantool/issues/4469 >> @@ -230,7 +230,7 @@ function aexitst(ai, hostnames, port) >> end; >> >> >> -aexitst( socket.getaddrinfo('localhost', 'http', { protocol = 'tcp', >> +aexitst( socket.getaddrinfo('127.0.0.1', 'http', { protocol = 'tcp', >> type = 'SOCK_STREAM'}), {'127.0.0.1', '::1'}, 80 ); > >This is the test case for getaddrinfo() itself. It is better to check >here that 'localhost' actually resolved to either 127.0.0.1 or ::1 >(typically both, but it depends of IPv4 / IPv6 support on a particular >system). Right, got too far here. > > >> @@ -546,7 +535,7 @@ socket.getaddrinfo('host', 'port', { flags = 'WRONG' }) == nil and errno() == er >> test_run:cmd("setopt delimiter ';'") >> f = fiber.create(function() >> while true do >> - local result = socket.getaddrinfo('localhost', '80') >> + local result = socket.getaddrinfo('127.0.0.1', '80') >> fiber.sleep(0) >> end >> end); > >Same here. > >> @@ -955,17 +964,17 @@ client:write('world') >> client:read(5, 5) == 'world' >> -- cancel fiber >> fiber.cancel(echo_fiber) >> -client:read(1, 5) == '' >> +client:read(1, TIMEOUT) == '' >> server:close() > >It worth to use TIMEOUT constant instead of 5 in two near :read() calls >above this one: > > | client:read(5, 5) == 'hello' > | client:read(5, 5) == 'world' > >There is also other case where it is applicable: > > | sc:send('Hello') > | sc:send(', world') > | sc:send("\\nnew line") > | sa:read('\\n', 1) -- !! > | sa:read({ chunk = 1, delimiter = 'ine'}, 1) -- !! > | sa:read('ine', 1) -- !! > | sa:read('ine', 0.1) > >All reads except the last one should use the common TIMEOUT. The last >one expected to block on 0.1 seconds and then return nil, so don't >increase timeout here. > >And the case right after this one: > > | sc:send('Hello, world') > | sa:read(',', 1) -- !! > | sc:shutdown('W') > | sa:close() > | sc:close() > >This way the change will be consistent within the file, at least in >context of :read() calls with a timeout. > >Those cases do not fail for me (I don't know why), so it is purely for >consistency. Existing test cases will show our practices, that's good >for ones who'll write more cases in a future. Right, i also didn't see any fails for those reads and decided not to interfere with them. However i agree that consistency is also important. > -- Ilya Kosarev