uber-archive / statsrelay

A consistent-hashing relay for statsd and carbon metrics
Other
101 stars 28 forks source link

Fix #66 by adding always_resolve_dns config option #67

Closed JeremyGrosser closed 7 years ago

CLAassistant commented 7 years ago

CLA assistant check
All committers have signed the CLA.

mschurenko commented 7 years ago

Is anyone around that can merge this PR?

JeremyGrosser commented 7 years ago

I'm waiting to merge it until you've confirmed that it solves your problem

mschurenko commented 7 years ago

Oh sorry. I'll try to test it this week. Thanks again!

mschurenko commented 7 years ago

It doesn't look to be working. After I change the IP address of the backend I get this in the logs:

Dec 21 20:03:43 statsrelay1: tcpclient: Error resolving backend address (null): Unknown error
JeremyGrosser commented 7 years ago

I think this new patch should fix that issue. 896e817

mschurenko commented 7 years ago

It looks like the ip address actually does not get updated. I added a bit of debugging (CLIENT ADDR IS: ) :

https://github.com/mschurenko/statsrelay/blob/testing/src/tcpclient.c#L329

When I run it I get this in the logs:

Jan  3 21:17:18 pipes-vagrant-v1-2 statsrelay: statsrelay 1.6.8
Jan  3 21:17:18 pipes-vagrant-v1-2 statsrelay: carbon has no backends, skipping
Jan  3 21:17:18 pipes-vagrant-v1-2 statsrelay: CLIENT ADDR IS: 127.0.0.1
Jan  3 21:17:18 pipes-vagrant-v1-2 statsrelay: tcpclient[host1/9600/tcp]: State transition INIT -> CONNECTING
Jan  3 21:17:18 pipes-vagrant-v1-2 statsrelay: tcpserver: Listening on frontend 127.0.0.1[:9126], fd = 7
Jan  3 21:17:18 pipes-vagrant-v1-2 statsrelay: udpserver: Listening on frontend 127.0.0.1[:9126], fd = 8
Jan  3 21:17:18 pipes-vagrant-v1-2 statsrelay: main: Starting event loop
Jan  3 21:17:18 pipes-vagrant-v1-2 statsrelay: tcpclient[host1/9600/tcp]: State transition CONNECTING -> CONNECTED
Jan  3 21:17:52 pipes-vagrant-v1-2 statsrelay: tcpclient[host1/9600/tcp]: Server closed connection
Jan  3 21:17:52 pipes-vagrant-v1-2 statsrelay: tcpclient[host1/9600/tcp]: State transition CONNECTED -> INIT
Jan  3 21:17:57 pipes-vagrant-v1-2 statsrelay: CLIENT ADDR IS: 127.0.0.1
Jan  3 21:17:57 pipes-vagrant-v1-2 statsrelay: tcpclient[host1/9600/tcp]: State transition INIT -> CONNECTING
Jan  3 21:17:57 pipes-vagrant-v1-2 statsrelay: tcpclient[host1/9600/tcp]: Connect failed: Connection refused
Jan  3 21:17:57 pipes-vagrant-v1-2 statsrelay: tcpclient[host1/9600/tcp]: State transition CONNECTING -> BACKOFF

In my test I am changing the IP for host1 from 127.0.0.1 to 127.0.0.2. The second occurrence of "CLIENT ADDR IS:" should be "127.0.0.2" and not "127.0.0.1". (Unless my debug logging is incorrect. I don't really know C so that's possible.)

JeremyGrosser commented 7 years ago

I can't reproduce this issue... I setup statsrelay in a container on kubernetes, pointed it at a service name "carbon.default.svc.cluster.local", then changed the IP that name points to several times and saw statsrelay resolve the new IP each time after the record's TTL expired. The test config I'm using is here: https://storage.googleapis.com/statsrelay-config.legitdata.co/statsrelay.yaml

mschurenko commented 7 years ago

Ok I tested it using a DNS server instead of /etc/hosts and it does work. I'm not sure why it wasn't picking up the IP address change when just using the hosts file. Thanks

mschurenko commented 7 years ago

Hey. Just wondering if you are ok to merge this now? Thanks

JeremyGrosser commented 7 years ago

I've merged it into JeremyGrosser/master, but it looks like Uber's removed my commit access to uber/statsrelay, so there's that.

mschurenko commented 7 years ago

I tried emailing and tweeting @eklitzke but haven't got a response.