search

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

Resolve hostname once #45

Closed ossareh closed 8 years ago

ossareh commented 11 years ago

Hey,

I'm not entirely sure why you do not resolve the hostname once in your version, given we just had a production issue related to this decision I figured it a reasonable idea to supply a patch that caches the resolved address. Is the concerns that the IP address of the host may change?

Making this change has a slight performance increase (a few ms per request according to http://matt.aimonetti.net/posts/2013/06/26/practical-guide-to-graphite-monitoring/ ) - it also stops busy sites like ours crippling our DNS server when under load. We could, as you suggest, run a caching dns server on each of our app servers - but that seems like a poor tradeoff when the library could do it so easily.

As an aside, this brings it inline with the python statsd client too: https://github.com/jsocol/pystatsd/blob/master/statsd/client.py#L42

ossareh commented 11 years ago

I can look into the IPv6 failures if you think the patch, in essence, is correct.

reinh commented 11 years ago

Hey @ossareh, thanks for the PR! We strongly suggest using an IP address instead of a hostname because of exactly this issue but I don't see any reason not to improve the experience for those who do use a hostname.

:+1: from me. @raggi, any thoughts?

raggi commented 11 years ago

The problem with this patch is that it stops people who use DNS for management from gracefully handling upgrades.

We could consider installing a HUP handler that re-resolves. What do you think to that?

n.b. under MRI the trap method returns previously set handlers so that one can create correct chains.

reinh commented 11 years ago

@raggi I honestly feel like this could be solved with better documentation. Resolving DNS at the time the client is instantiated could easily be done by the consuming code.

raggi commented 11 years ago

We have a note already. I'm not sure how far to push this.