Exposing #close as a public method, to help consumers avoid using too many file descriptors

[ramfjord]

It seems like a common use case for this client would be logging events in a web server. Many of these spin up a thread per response, or delegate response to a thread/process pool. My assumption is that these different threads/processes should not share the same Datadog::Statsd client, since I didn't see any sort of locking implemented around the socket IO.

For our use case, we don't mind creating a new UDPSocket every time we log a set of metrics, so I'd like to be able to close the socket when I'm done to avoid using thousands of file descriptors before the object get garbage collected. Please let me know if this is acceptable, or if I'm perhaps missing some more common way of using this library.

[degemer]

Hi @ramfjord, thanks for your contribution!

I'm fine with exposing close as I don't have a good answer right now for your use case, we would need to re-design Datadog::Statsd to be thread-safe which would probably require quite some work.

For the retry part: could we make sure we retry only once? Even though it's unlikely to happen, I'd prefer to avoid any risk of unbounded error loop.

[ramfjord]

@degemer I'm not sure if #connect_to_socket can ever return a closed socket, but I've added logic to retry only once if it does. Also, I realized that I wasn't catching errors raised by #connect_to_socket, which can certainly throw a SocketError if the host is no longer available, so I've added a check for that as well. Let me know if there's anything else I can do to improve this, and thanks for the quick review!

[ramfjord]

I'm pretty sure this isn't necessary because the before clause always initializes a new socket. If I can remove it, I can also remove the after clause in the #close tests, without having to mock out #clear.

[degemer]

Thanks a lot for the changes and the tests!
LGTM, it will be part of the next release (targeting end of next week).

[aviflax]

Um… I’m running multiple threads on JRuby, so I have “true” concurrency — no GIL. Should I be creating an instance of Datadog::Statsd for each thread? Because right now all threads are sharing a single instance.