Default to ephemeral sockets #6

Closed
wants to merge 1 commit into
from

5 participants

@mojodna

The use of a single, shared socket that's never closed prevents node processes from completing normally.

This change allows an optional shared socket to be passed into the constructor. If present, node-statsd will use that (and will assume that the provider will close it when appropriate). Otherwise, node-statsd will create new sockets for each connection (the old, less efficient behavior).

@tmm1 - I was thinking about ways to default to the shared behavior, but the only way I could think of to do that cleanly involved a dynamic getter and a setInterval that periodically checks whether the socket has been used recently (in ms) and closes it if not (to be recreated in the getter as necessary). I'm curious to hear your thoughts.

@mojodna mojodna Default to ephemeral sockets
The use of a single, shared socket that's never closed prevents node processes
from completing normally.

This change allows an optional shared socket to be passed into the constructor.
If present, node-statsd will use that (and will assume that the provider will
close it when appropriate).  Otherwise, node-statsd will create new sockets for
each connection (the old, less efficient behavior).
2584c08
@fetep

+1, confirmed working.

@lloyd

+1

@Dieterbe
Collaborator

The use of a single, shared socket that's never closed prevents node processes from completing normally.

can you explain this more?

@lloyd

@Dieterbe love to! Let's try an example. Notice this program never ends:

var dgram = require('dgram');
var s = dgram.createSocket('udp4');
var b = new Buffer("foo");
s.send(b, 0, b.length, 8080, "10.0.0.1");

notice if you add another line to it it ends in 5 seconds:

setTimeout(function() { s.close(); }, 5000);

The moment you write() over an allocated udp socket, it prevents the nodejs process from exiting until closed.

make sense?

@Dieterbe
Collaborator

right. i notice with a process.exit(code=0); you can make node exit just fine, but that's not very nice/pretty.

Put in other words: it's not good practice to "impose" this stuck-ness and "just do process.exit(code=0); to exit"-behavior on any application that implements this library; because that's not what applications expect. right?

I implement this library in a daemon which runs until i send it a signal to stop (or until it crashes), so I never noticed this problem. I guess if you have applications that are supposed to terminate, this problem is really obvious.

@Dieterbe
Collaborator

deprecated in favor of #9

@Dieterbe Dieterbe closed this Feb 6, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment