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

Add an optional way to reuse a socket instead of finding it in-Thread #49

Closed JustinAiken closed 8 years ago

JustinAiken commented 10 years ago

This leaves the current behavior unaffected, but provides an option of reusing a single socket for all the statsd, to avoid massive amounts of sockets being open/closed... or worse, opened and left...

A way to resolve https://github.com/reinh/statsd/issues/34 without creating a new socket for each stat.

JustinAiken commented 10 years ago

If anyone else needs this for some reason or the other, I've released a forked gem: justinaiken-statsd... it doesn't have any differences besides the name, this PR, and replaced ruby 1.8.7 with 2.0/2.1 in the travis matrix.

cheald commented 10 years ago

This isn't likely to be thread-safe, is it?

raggi commented 10 years ago

No it isn't, and creating threads constantly isn't likely to be performant either. The motivation behind this is misdesign, at least the ones I've seen (which are mostly use in celluloid).

cheald commented 10 years ago

There's actually a valid reason for something like this (though the original solution is the wrong solution specifically) - on JRuby, Thread.current is a different Ruby object for every Fiber, even though the threads that back the fibers are reused from a pool (so there isn't a new thread creation overhead for every new Fiber).

If you're using JRuby + Celluloid, then caching things that you want to use across fibers spawned from the same thread in Thread locals isn't going to work. In the case of statsd, it means that emitting messages from Celluloid workers results in a new socket opened for every Celluloid task. This is massively subpar for obvious reasons.

I've solved it locally by using a Statsd subclass that implements a threadsafe socket pool via a Queue, but since it's a solution to JRuby errata, I'm not sure that it belongs in upstream. But statsd-ruby + JRuby + fibers(/Celluloid) is bad news right now.

My implementation is working quite nicely in our case, though. https://gist.github.com/cheald/24cdc0161fe6d09672d5

JustinAiken commented 10 years ago

The way I was using this one (jRuby+celluloid) was to create an actor:

  class Stactor
    include Celluloid

    attr_accessor :statsd

    def initialize(statsd)
      @statsd = statsd
    end

    def send_stat(meth, *args, &blk)
      statsd.send meth, *args, &blk
    end

    def method_missing(meth, *args, &blk)
      self.async.send_stat meth, *args, &blk
    end
  end

Then wrap this Stats gem in that (with a reused socked), and wrapped all calls to statsd through there:

statsd = Statsd.new(host, port, UDPSocket.new)
MyApp.statsd = Stactor.new statsd

...it may not have been the best solution, but at least it quit leaking so many socket file descriptors that it would crash everything :laughing: