Add keep-alive and timeout options (thanks @rexxar) #125
Conversation
|
hmm its curious that 0.8.x fails here, looking into that. |
…ct (if thats possible)
jcrugzz
force-pushed
the
set-keep-alive
branch
from
a114f2f
to
cb70d69
Compare
| @@ -101,6 +101,11 @@ function connect(url, socketOptions, openCallback) { | |||
| var parts = URL.parse(url, true); // yes, parse the query string | |||
| var protocol = parts.protocol; | |||
| var noDelay = !!sockopts.noDelay; | |||
| // We should default to true as this nicely complements the heartbeat | |||
| var keepAlive = !!sockopts.keepAlive || true; | |||
There was a problem hiding this comment.
This doesn't actually do what you want... If I specify { keepAlive: false }, this will basically say var keepAlive = false || true;, which obviously is always true. So you're basically always enabling keepAlive here.
There was a problem hiding this comment.
ah good catch, thanks!
|
To be honest I'm not sure if using TCP keep-alive is appropriate here or not. It might be a better way (than using AMQP's heartbeats) of keeping load-balanced and NAT connections open. Is that the idea, @jcrugzz ? |
|
@squaremo the idea was to just be complementary to the heartbeats as they are both designed to keep the TCP socket open and detect when something has been broken. |
When implementing keep-alive support I just pulled in the work from @Rexxar on timeouts as i slightly refactored it to handle timeout events that could possibly happen after connect (which I have not seen but was going for the side of safety). @squaremo let me know your thoughts as I think having keep-alive support is a nice complement to the heartbeat functionality you have implemented.