Skip to content
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

Add an experimental keepalive keyword arg for setting keepalive on tc… #212

Merged
merged 3 commits into from
Mar 2, 2018

Conversation

quinnj
Copy link
Member

@quinnj quinnj commented Mar 2, 2018

…p sockets in connections

@quinnj quinnj requested a review from samoconnor March 2, 2018 13:04
@samoconnor
Copy link
Contributor

Nice one. This is worth trying for sure!

@samoconnor
Copy link
Contributor

I was just about to say, go ahead and merge this because it should have no effect when not enabled.
I think I'd be happier to merge this with the default being disabled, and then test Femtocleaner with #211 with and without this setting.

@quinnj
Copy link
Member Author

quinnj commented Mar 2, 2018

That sounds good; I wanted to enable it and have the test suite run just to see if it actually affected anything, but then we can merge w/ it off by default until we feel more comfortable.

function keepalive!(tcp)
@debug 2 "setting keepalive on tcp socket"
err = ccall(:uv_tcp_keepalive, Cint, (Ptr{Cvoid}, Cint, Cuint),
tcp.handle, 1, 1)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@samoconnor @Keno, the 2nd argument here is an "initial delay", which I assume is how long it waits before sending the first keepalive packet (it's unclear how frequently it sends it after that, but whatever). I just set this to 1 for now, but if you guys have other opinions or concerns, I'm happy to try whatever here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Every time I've looked into the libuv docs I find it very hard to figure out what the exact behaviour is supposed to be. They often just say "see the linux man page for more" without explaining how their behaviour differs. (Sometimes I wonder if we'd be better off using BSD sockets directly and using some kind of BSD/Winsock layer to make it work on windows. libuv is a weird beast).

I think in this case it would make sense to observe what goes on with wireshark.
Probably OSX, Linux and Windows will behave differently too.

Perhaps you can find some place in libcurl, or in python's requests library where they set keepalive options and see if there are any clues there...

@quinnj quinnj merged commit fe66594 into master Mar 2, 2018
@quinnj quinnj deleted the jq/keepalive branch March 2, 2018 17:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants