Fix reuse of client port on Linux. Implement for OSX. #21818

amitmurthy · 2017-05-12T02:40:59Z

Closes #19893 .

ci skip till #21799 is merged.

This commit should backport cleanly. Will add another commit that uses getsockname everywhere once #21801 is closed.

tkelman · 2017-05-13T07:50:53Z

base/socket.jl

-function TCPSocket()
+function TCPSocket(; delay=true)  # kw arg "delay": if true, libuv delays creation of the socket
+                                  # fd till the first bind call
+


could we avoid starting the function with an empty line? it's not the usual formatting style

amitmurthy · 2017-05-14T17:47:37Z

base/distributed/cluster.jl

@@ -153,8 +153,16 @@ function start_worker(out::IO, cookie::AbstractString)
    init_worker(cookie)
    interface = IPv4(LPROC.bind_addr)
    if LPROC.bind_port == 0
-        (actual_port,sock) = listenany(interface, UInt16(9009))
-        LPROC.bind_port = actual_port
+#        (actual_port,sock) = listenany(interface, UInt16(9009))


When testing this PR, i.e., reusing client port numbers, our current strategy for workers to start listening for a free port starting from 9009 caused a major slowdown in addprocs when it was part of a repeated addprocs(x); run tests....; rmprocs(workers) cycle - as the test script indeed does. I suspect it was because the new workers listened on the same ports (starting from 9009), but setup of new connections with the same client port were being delayed, as TCP connections to previous workers (now in a TIME_WAIT state) still existed. And new connections with the same (clienthostport,serverhostport) tuple had to wait till the system cleaned them up (OSX in my case).

I changed the worker listening port selection logic to select a free ephemeral port and the slowdown disappeared. While that works, I am uncomfortable using ephemeral ports for workers to listen on as firewalling policies may disallow incoming connections to the ephemeral port range.

However, the current strategy of listening on a free port starting from 9009 is also not ideal when we have many workers on a node. For example, the 100th worker in an addprocs(100) will try and fail in listening on every port between 9009 and 9108 before succeeding to listen on 9109.

Looking for suggestions to work around this issue.

Maybe we can suggest that someone who needs that will write a UPnP package that automates the firewall configuration step? Although I think that the typical usage of this cluster code is intended to be in a closed environment where all of the devices are on the same side of the firewall (whether virtually or physically).

vtjnash · 2017-05-15T15:51:29Z

base/distributed/managers.jl

            rc = ccall(:jl_tcp_reuseport, Int32, (Ptr{Void},), s.handle)
            if rc > 0  # SO_REUSEPORT is unsupported, just return the ephemerally bound socket
                return s
            elseif rc < 0
                throw(SystemError("setsockopt() SO_REUSEPORT : "))
            end
-            getsockname(s)
+            is_apple() && bind_client_port(s)
        catch e
            # This is an issue only on systems with lots of client connections, hence delay the warning
            nworkers() > 128 && warn_once("Error trying to reuse client port number, falling back to plain socket : ", e)


why is this done as a try/catch rather than printing this message where we threw it 6 lines above and then returning from there

vtjnash · 2017-05-15T16:00:09Z

base/socket.jl

+    else
+        err = ccall(:uv_tcp_init_ex, Cint, (Ptr{Void}, Ptr{Void}, Cuint),
+                    eventloop(), tcp.handle, 2) # AF_INET is 2
+    end


I think you can call the _ex version unconditionally, and just select between AF_INET = 2 and AF_UNSPEC = 0 (that's what libuv will do internally anyways)

vtjnash · 2017-05-15T16:10:11Z

Looks like this will be a bit better after #21801 is merged too.

tkelman · 2017-05-19T06:48:41Z

base/distributed/cluster.jl

+            LPROC.bind_port = port
+        else
+            close(sock)
+            error("no ports available")


should throw a more specific exception type if possible

amitmurthy · 2017-05-19T08:20:40Z

Added a NEWS entry. Will this fix make 0.6? Should it be in the 0.6 section?

amitmurthy · 2017-05-19T10:32:15Z

AV failure is unrelated.

amitmurthy · 2017-05-20T03:54:39Z

Will merge this in a day.

tkelman · 2017-05-22T11:32:39Z

This is failing tests on the buildbots. https://build.julialang.org/builders/build_ubuntu14.04-x64/builds/366/steps/shell_2/logs/stdio

Should fix buildbot failure #21818 (comment) A bit of a mystery as to how Linux CI passed correctly.....

Fix reuse of client port on Linux. Implement for OSX. Ref #21818 (cherry picked from commit e5fb87d)

Ref #21818 (cherry picked from commit 0394f47)

Ref #21818 (cherry picked from commit 2272036)

Ref #21818 (cherry picked from commit fa8c4d2)

Ref #21818 (cherry picked from commit 0394f47)

Ref #21818 (cherry picked from commit 2272036)

Ref #21818 (cherry picked from commit fa8c4d2)

Ref #21818 (cherry picked from commit 0394f47)

Ref #21818 (cherry picked from commit 2272036)

Ref #21818 (cherry picked from commit fa8c4d2)

Partially reverts #21818. Fixes #24722.

Partially reverts the backport of #21818 in 0.6.1. Fixes #24722.

Partially reverts the backport of #21818 in 0.6.1. Fixes #24722. Remove support for OSX client port reuse.

Partially reverts the backport of #21818 in 0.6.1. Fixes #24722. Revert client_socket_reuse

amitmurthy force-pushed the amitm/reuseportfix branch from 557352f to da8a991 Compare May 13, 2017 07:48

amitmurthy changed the title ~~WIP : Fix reuse of client port on Linux. Implement for OSX. [ci skip]~~ Fix reuse of client port on Linux. Implement for OSX. May 13, 2017

amitmurthy requested a review from vtjnash May 13, 2017 07:50

tkelman reviewed May 13, 2017

View reviewed changes

amitmurthy force-pushed the amitm/reuseportfix branch from da8a991 to 58e104e Compare May 14, 2017 16:46

amitmurthy changed the title ~~Fix reuse of client port on Linux. Implement for OSX.~~ WIP: Fix reuse of client port on Linux. Implement for OSX. May 14, 2017

amitmurthy commented May 14, 2017

View reviewed changes

vtjnash reviewed May 15, 2017

View reviewed changes

vtjnash approved these changes May 15, 2017

View reviewed changes

vchuravy added backport pending 0.5 labels May 16, 2017

amitmurthy force-pushed the amitm/reuseportfix branch 2 times, most recently from f5ad929 to c354dfd Compare May 19, 2017 05:26

tkelman reviewed May 19, 2017

View reviewed changes

amitmurthy changed the title ~~WIP: Fix reuse of client port on Linux. Implement for OSX.~~ Fix reuse of client port on Linux. Implement for OSX. May 19, 2017

amitmurthy added 3 commits May 21, 2017 23:44

Fix reuse of client port on Linux. Implement for OSX.

0394f47

fix listenany to return correct port number for porthint of 0

2272036

added NEWS entry

fa8c4d2

amitmurthy force-pushed the amitm/reuseportfix branch from 66ea85a to fa8c4d2 Compare May 21, 2017 18:15

amitmurthy merged commit e5fb87d into master May 22, 2017

amitmurthy deleted the amitm/reuseportfix branch May 22, 2017 03:55

amitmurthy added a commit that referenced this pull request May 22, 2017

Return socket correctly in socket_reuse_port

3bfea97

Should fix buildbot failure #21818 (comment) A bit of a mystery as to how Linux CI passed correctly.....

amitmurthy mentioned this pull request May 22, 2017

Return socket correctly in socket_reuse_port #22017

Merged

ararslan pushed a commit that referenced this pull request Sep 11, 2017

Merge pull request #21818 from JuliaLang/amitm/reuseportfix

7bdf3e5

Fix reuse of client port on Linux. Implement for OSX. Ref #21818 (cherry picked from commit e5fb87d)

ararslan pushed a commit that referenced this pull request Sep 13, 2017

Fix reuse of client port on Linux. Implement for OSX.

8c812ef

Ref #21818 (cherry picked from commit 0394f47)

ararslan pushed a commit that referenced this pull request Sep 13, 2017

fix listenany to return correct port number for porthint of 0

1dc14b3

Ref #21818 (cherry picked from commit 2272036)

ararslan pushed a commit that referenced this pull request Sep 13, 2017

added NEWS entry

a3c37a2

Ref #21818 (cherry picked from commit fa8c4d2)

vtjnash pushed a commit that referenced this pull request Sep 14, 2017

Fix reuse of client port on Linux. Implement for OSX.

04a19cc

Ref #21818 (cherry picked from commit 0394f47)

vtjnash pushed a commit that referenced this pull request Sep 14, 2017

fix listenany to return correct port number for porthint of 0

a5ed930

Ref #21818 (cherry picked from commit 2272036)

vtjnash pushed a commit that referenced this pull request Sep 14, 2017

added NEWS entry

44b8192

Ref #21818 (cherry picked from commit fa8c4d2)

ararslan pushed a commit that referenced this pull request Sep 15, 2017

Fix reuse of client port on Linux. Implement for OSX.

c1fe105

Ref #21818 (cherry picked from commit 0394f47)

ararslan pushed a commit that referenced this pull request Sep 15, 2017

fix listenany to return correct port number for porthint of 0

a50154e

Ref #21818 (cherry picked from commit 2272036)

ararslan pushed a commit that referenced this pull request Sep 15, 2017

added NEWS entry

152f814

Ref #21818 (cherry picked from commit fa8c4d2)

ararslan removed the backport pending 0.6 label Oct 7, 2017

amitmurthy mentioned this pull request Nov 23, 2017

addprocs problem with multiple nodes in 0.6.1 #24722

Closed

ararslan mentioned this pull request Nov 23, 2017

WIP: Backports for 0.6.2 #24519

Merged

ararslan pushed a commit that referenced this pull request Nov 24, 2017

Only listen on a port starting from 9009

86a0e9c

Partially reverts #21818. Fixes #24722.

ararslan pushed a commit that referenced this pull request Nov 25, 2017

Only listen on a port starting from 9009

0ad7336

Partially reverts #21818. Fixes #24722.

ararslan pushed a commit that referenced this pull request Nov 25, 2017

Listen on the first free port from 9009

eda0b7c

Partially reverts the backport of #21818 in 0.6.1. Fixes #24722.

amitmurthy added a commit that referenced this pull request Nov 25, 2017

Listen on the first free port from 9009

ed763c5

Partially reverts the backport of #21818 in 0.6.1. Fixes #24722. Remove support for OSX client port reuse.

amitmurthy added a commit that referenced this pull request Nov 27, 2017

Listen on the first free port from 9009

6736f45

Partially reverts the backport of #21818 in 0.6.1. Fixes #24722. Revert client_socket_reuse

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix reuse of client port on Linux. Implement for OSX. #21818

Fix reuse of client port on Linux. Implement for OSX. #21818

amitmurthy commented May 12, 2017

tkelman May 13, 2017

amitmurthy May 14, 2017

vtjnash May 15, 2017

vtjnash May 15, 2017 •

edited

Loading

vtjnash May 15, 2017

vtjnash commented May 15, 2017

tkelman May 19, 2017

amitmurthy commented May 19, 2017

amitmurthy commented May 19, 2017

amitmurthy commented May 20, 2017

tkelman commented May 22, 2017

Fix reuse of client port on Linux. Implement for OSX. #21818

Fix reuse of client port on Linux. Implement for OSX. #21818

Conversation

amitmurthy commented May 12, 2017

tkelman May 13, 2017

Choose a reason for hiding this comment

amitmurthy May 14, 2017

Choose a reason for hiding this comment

vtjnash May 15, 2017

Choose a reason for hiding this comment

vtjnash May 15, 2017 • edited Loading

Choose a reason for hiding this comment

vtjnash May 15, 2017

Choose a reason for hiding this comment

vtjnash commented May 15, 2017

tkelman May 19, 2017

Choose a reason for hiding this comment

amitmurthy commented May 19, 2017

amitmurthy commented May 19, 2017

amitmurthy commented May 20, 2017

tkelman commented May 22, 2017

vtjnash May 15, 2017 •

edited

Loading