connection timeout removes all nodes from cluster

[stephanos]

We noticed some odd behaviour in production: under certain circumstances, memcachir lost all nodes in our app. But they were running just fine.

Turned out, that if there is a connection timeout, memcachir thinks all nodes are gone:

memcachir/lib/memcachir/discovery/elasticache.ex

Line 16 in 562c8a6

[]

cc @michaeljguarino

[stephanos]

@michaeljguarino I guess that never happened to you?

[michaeljguarino]

Oh I've never seen that. We probably need to retry on that error.

[michaeljguarino]

What's the exact error tuple you're seeing?

[stephanos]

@michaeljguarino Yep, retrying would be good. And/or ignoring timeouts maybe, just in case the retries all fail? I've not actually seen the error tuple since it's not logged anywhere. Our best guess is that it's due to a timeout.

[stephanos]

Oh, I just notice that it is logged. let me search the logs.

[stephanos]

I see unable to fetch ElastiCache servers: :timeout in the logs.

We rolled back to 3.2.1 for our services because of this. Seemed to do the trick. I think 3.2.1 just restarts itself for every change: A timeout would yield [] and that would be different from the existing setup, a restart would happen, next time the fetch would work again and another restarts puts us back to where we were. Not ideal, but it seems to work just fine.

What we observed with 3.2.2 was that once there was a timeout talking to ElastiCache, all subsequent GET calls would error with unable to get: no_nodes. It seemed to be stuck. My colleague said the only option was to manually restart the app. I just glanced at the herd code to see what it does once it received [] as the node list result, but I couldn't figure out what it does exactly yet.

[michaeljguarino]

It shouldn't kill anything I believe, it should just dynamically supervise nothing then poll again and add them when something is discovered.

I can add a retry, and add a test that takes everything out then tries to add things back in to verify it works as it should.

[stephanos]

That would be awesome, thank you.

[stephanos]

I just talked to my colleague again and I think I know what's going on now. When we upgraded to 3.2.2 the elasticachex version we used was still 1.1.1 (missing the memory leak fix). Then, since we have lots of apps connect to that ElastiCache host, memcached must have run out of connections and so our apps were seeing timeouts. That's what triggered the problem. And since we never closed any connections, it looked like it was the client library's fault. 3.2.1 would have restarted in that case and closed all the connections.

Anyway, one way or another, I think a few retries might be good because it would make it more robust.

[michaeljguarino]

Ahh, yeah that makes sense. I'll try to add some timeouts asap though, and dig into the failure to reregister everything.

[stephanos]

@michaeljguarino did you make any progress on the retries? last night our app seemed to have issues talking to ElasticCache (I see 36 unable to fetch ElastiCache servers: :timeout) and nodes were removed and added again for a while. It'd be much appreciated to make this more robust.

[stephanos]

my coworkers point out that it'd be even better if an error like timeout (even after retries), should not lead to the whole cluster being removed. it should just continue with what it has. I think that's a very fair point.

[stephanos]

please let me know whether you intend to fix this (I believe this requires some changes to herd). We'd prefer to move forward but might have to go back to 3.2.1 (which doesn't exhibit those kinds of issues).

[michaeljguarino]

Sorry, I got sidetracked. I think I almost had it all fixed actually. Let me see if I can open a pr.

I'm not sure about implementing ignoring timeouts. It can theoretically be done of course.

[michaeljguarino]

Here's the pr: #12