TCP backlog improvements #708
Conversation
default backlog value which is 5 is too low on MacOS X to successfully run parallel tests, also it was discovered that passing sockert options from config file stopped working thanks @pzel for helping me with this
| {packet, 0}, | ||
| {active, false}, | ||
| {reuseaddr, true}, | ||
| {nodelay, true}, | ||
| {send_timeout, ?TCP_SEND_TIMEOUT}, | ||
| {keepalive, true}, | ||
| {send_timeout_close, true}]), | ||
| {send_timeout_close, true} | SockOpts]), |
There was a problem hiding this comment.
Correct me if I'm wrong, but won't this prevent our overrides from ever being picked up by gen_tcp:listen? The 'updated' values should be in front of the proplist.
Or, better yet, let's remove old values and add our overrides (using dict: functions will be the quickest).
There was a problem hiding this comment.
No, they have to be at the end, I looked into the gen_tcp, and inet code. There is a recursive function iterating on this opts and construing a record with opts.
There was a problem hiding this comment.
I see -- so that's very non-obvious, and someone may come along and change it later. Let's do it the right way so no "alternative" interpretation is possible.
| do_override(inet6, Opts) -> | ||
| [inet6 | lists:delete(inet6, Opts)]; | ||
| do_override(inet, Opts) -> | ||
| [inet | lists:delete(inet, Opts)]; |
There was a problem hiding this comment.
I like this explicit solution! One minor nitpick:
Isn't
[inet6 | lists:delete(inet6, Opts)]
the same thing as
Opts ?
Also, it's not technically possible to delete one of the inet|inet6 options now. Do we need those clauses? Maybe just erroring out with error({cannot_override_option, O, Opts}) would be more straightforward for the user?
There was a problem hiding this comment.
No, it's not the same. What I wanted to achieve is to have only one inet6 or inet value in the list.
There was a problem hiding this comment.
I mean inet6 may not be in the Opts list but if it is, the I want only one such element in the list.
Proposed changes include:
Also this PR is important to allow running parallel tests, introduced in #706, on MacOS X. Together with @pzel we discovered (mainly based on blogpost [1]) that the backlog queue implementation on MacOS X is less efficient than on linux. Too low backlog on MongooseIM side for port 5222 manifests by strange
{error,econnreset}[1]. http://veithen.github.io/2014/01/01/how-tcp-backlog-works-in-linux.html