reinh / statsd

A Ruby Statsd client that isn't a direct port of the Python example code. Because Ruby isn't Python.
MIT License
411 stars 154 forks source link

better thread safety and socket semantics #56

Closed raggi closed 8 years ago

raggi commented 10 years ago

This needs to be a major version change, as it changes a lot of implied semantics.

Includes addition of a "hup" method that would reconnect the sockets, typically this would be used in response to a HUP signal.

Closes #55 Closes #54 Closes #50 Closes #49 Closes #45

Review please @reinh @cheald cc @thwarted

zanker commented 9 years ago

Hey @raggi, we just got bit by this, since in Sidekiq on JRuby the Thread.current changes with each job. Is there anything specific that would need to be done get this merged? Happy to take a look at the work needed.

raggi commented 9 years ago

@zanker I was mostly just hoping for a code review, but none presented.

thwarted commented 9 years ago

I looked it over, but I don't have much to add because my ruby-fu is weak.

raggi commented 9 years ago

I have updated this to respond to the review comments (thanks all).

Note that this solution is insufficient to support #61 which does not maintain thread safety (TCP writes are not necessarily thread safe).

raggi commented 9 years ago

@reinh PTAL.

raggi commented 9 years ago

And squashed.

reinh commented 9 years ago

@raggi Replacing a thread-local socket with an instance variable and wrapping use-sites with a mutex? Seems good. What's up with CI? What's our plan for #61? Land this first and then continue with TCP support?