-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
SO_REUSEPORT support #654
Comments
golang 1.11 is slated to support setting socket options before listening or dialing, which includes so_reuseport and tcp_fastopen. Might be better to just wait it out and create a PR when it's natively supported. |
Yeah will be much better. This is not a PR, I just meant to share some findings of the benefits you could expect from using SO_REUSEPORT. |
Interesting, @chantra Weren't there counter-arguments in reusing ports on highly-loaded DNS servers ? i.e. magic number overlap, TCP starvation, spoofing, ordering vs holding, etc ? As you outline, balance might be when all processors take part into orchestration, and there is a limit where is not so advantageous anymore (load matters, test data too). I believe it must deal with time_wait and how aggressively or mildly the conneciton is detected as idle, for TCPs. less clear how it acts on UDP .. Anyway I can see that your code implement both UDP and TCP, could you split the perf results accordingly ? just interested at this since some time. bonnes vacances, |
I think you are mixing this up with
This was purely UDP payload performed from the local host. |
it is actually dominated by syscall.Syscall which is just below syscall.sendmsg ... |
This is pretty neat. I might be worth checking out how this can fit in CoreDNS. Go does this now automatically I believe:
So basically NUMCPU is the amount of listeners we want/need. |
I wonder how much of #565 can be attributed to less contention - or maybe we should do both... |
Quickly checked #565 and it does perform better in the "reflect" case only when I use a very small amount of workers (5-10) which bring the QPS in the 8/8 case to 250k vs 225k. |
[ Quoting <[email protected]> in "Re: [miekg/dns] SO_REUSEPORT suppor..." ]
Quickly checked #565 and it does perform better in the "reflect" case only when I use a very small amount of workers (5-10) which bring the QPS in the 8/8 case to 250k vs 225k.
Thanks. Yes, my big dislike of that PR was the magic worker number...
Just using NUMCPU seems a much better why of dealing with this.
/Miek
…--
Miek Gieben
|
I'm trying to replace this, but don't see the impressive increase in qps that you've observed... |
HI @miekg how is your test performed ? Is source port randomized ? is server multihomed ? |
With go 1.11beta2 out, I played a bit with the new API to set SO_REUSEPORT. is an example. With this diff applied, The current patch is not great, I believe at least we may need to put a default function which is a noop and have a linux version that do set SO_REUSEPORT. Anyway, feel free to play with it. |
golang/go#26771 is tracking the availability of syscall.SO_REUSEPORT constant |
ok, so unix.SO_REUSEPORT it will be as apparently syscall only get new constant when required by standard library. |
@chantra What're your thoughts about this issue since Golang 1.11 now supports setting these socket options, and you no longer have to include a 3rd-party library? Not only is SO_REUSEPORT going to be helpful, but also TCP_FASTOPEN for DNS-over-TLS/HTTPS support. |
Is anyone in a position to create/rework this code into a proper PR? Support wise we need to check if this features is available as we want to support 1.10 and 1.11 (last two releases) |
* Use strings.TrimSuffix in ListenAndServe for TLS This replaces the if/else statements with something simpler. Interestingly, the first pull request I submitted to this library was to fix the tcp6-tls case way back in 4744e91. * Add SO_REUSEPORT implementation Fixes #654 * Rename Reuseport field to ReusePort * Rename supportsReuseport to match ReusePort * Rename listenUDP and listenTCP file to listen_*.go
Just dropping this here. I tested SO_REUSEPORT on go-dns using:
https://gist.github.com/chantra/dfab011267be22989b25134537963e5c
To test, I patched
reflect
with https://gist.github.com/chantra/abc8edf0c4cdce2e5aaf8c0d5473dd79Perf test
~88k
~92k
~225k
~221k
~150k
~150k
So there is ways to squeeze more out of the service but there is some fine tuning that needs to be done between GOMAXPROCS and the number of sockets.
known issue
The empty address is not supported, e.g passing
:5353
as an IP/Port to bind to would fail and report error:see libp2p/go-reuseport#7
The text was updated successfully, but these errors were encountered: