Skip to content
This repository has been archived by the owner on May 4, 2018. It is now read-only.

Add interval and count to keep-alive #729

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jameshartig
Copy link

The default probe interval and failure counts set by the OS are way too extreme and if you have a real-time socket server, you're going to want to customize these so you can have a much lower keep-alive interval and count so you're notified of disconnected sockets sooner.

Unfortunately, Windows doesn't let you set count, but using SIO_KEEPALIVE_VALS we can set interval (see: http://msdn.microsoft.com/en-us/library/windows/desktop/dd877220(v=vs.85).aspx). The defaults for the OS are 2 hour timeouts and 1 second intervals. I was hesitant about setting the default 1 second because that's pretty insane but decided just to leave it.

@jameshartig
Copy link
Author

According to the node docs "Setting 0 for initialDelay will leave the value unchanged from the default (or previous) setting.". So I've added the delay back to the if checks in tcp.c to keep that true.

The default probe interval and failure counts set by the OS are
too extreme. Lowering them will allow you to know when clients
disconnect sooner, which is helpful if you're running a chat server.
@hertzg
Copy link

hertzg commented Jan 29, 2015

Any updates on this?

@saghul
Copy link
Contributor

saghul commented Jan 29, 2015

@hertzg Looks like this fell through the cracks long time ago. The problem with this patch now is that it changes the API and thus it cannot land in v1.x. That aside, the values are not stored and thus lost if the socket was not yet created.

There is a proposal to create sockets early: libuv/leps#5 which would simplify the problem, because the values can always be set.

Either way, libuv moved over to https://github.com/libuv/libuv, so please open a new issue there. Thanks!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants