-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
reuse client port number if possible #12158
Conversation
probably worth taking a look at libuv/libuv@a385ae4, where libuv explictly disabled this and the bug referenced there https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=174087 |
Thanks. Those links are about issues with SO_REUSEADDR. The option being set in this PR is SO_REUSEPORT which was introduced first in BSD and fairly recently in Linux (kernel 3.9 onwards). This PR gets an ephemeral port number the first time around and then reuses the same port number for all subsequent The funny thing though is that on the client side, setting SO_REUSEPORT before |
ccall(:jl_tcp_reuseport, Int32, (Ptr{Void}, ), s.handle) < 0 && throw("SO_REUSEPORT error") | ||
ccall(:jl_tcp_getsockname_v4, Int32, | ||
(Ptr{Void}, Ref{Cuint}, Ref{Cushort}), | ||
s.handle, client_host, client_port) < 0 && throw("getsockname() error") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you are throwing strings here
👍 |
b6000d9
to
40c622b
Compare
reuse client port number if possible
Is this warning during Travis CI related to this PR and is it expected? |
It's in the CI run for this PR as well. @amitmurthy |
I forgot Travis runs on Ubuntu 12.04, so it is expected. I'll modify the warning to show up only when we are adding a large number of workers, say > 128, since it is not really an issue below that. |
Is that because the kernel is not new enough? and can it be detected? |
Yes. And yes. |
I'm seeing this during tests:
|
Working on this here - #12192 Looks like on OSX we need to setsockopt before bind (as sounds logical), but on Linux that did not work and bind followed by setsockopt worked. Maybe I should limit setting this only for Linux since it is unlikely that folks on OSX will run with hundreds of workers? |
Nah, let's fix it right. Better to have things just work. Someone may want hundreds of workers on OS X. |
Maybe this is just because in my use cases, things are generally I/O bound, so I'm used to needing lots and lots of workers, even on a old 8-core Mac Pro, as opposed to the probably more CPU bound stuff that is more frequently done in numerical/scientific computing. |
Let me know if you need help testing, @amitmurthy. |
Thanks @StefanKarpinski . I have access to a Mac and was trying it make it work there. The bad news is that I don't think I can make it work on OSX because we don't have a way to access the socket fd before a On Linux kernels > 3.9 The ability to get access to a socket before Also SO_REUSEPORT is not supported on Windows. I'll go ahead and make this a Linux only thing for now. Will revisit OSX when our libuv fork catches up with the above mentioned patch. |
Sounds good. |
This fixes a couple of issues that we ran into while launching a 1000 workers, with 64 workers to a node
accept
call fails.