redis / redis-rb

A Ruby client library for Redis
MIT License
3.97k stars 1.03k forks source link

Wrong return values due to reconnect AUTH #925

Closed ajvondrak closed 1 year ago

ajvondrak commented 4 years ago

I have my Redis calls instrumented in production with Honeycomb's Ruby beeline, which has pointed to an interesting issue with the reconnect logic.

We're using the synchrony driver, which I can completely believe results in this particular behavior, but I can't be sure. :expressionless: I'm in support of #915, but I'm stuck with EM for the time being.

Relevant versions from my Gemfile.lock:

eventmachine (1.2.7)
em-synchrony (1.0.6)
redis (4.1.0)
hiredis (0.6.3)

My code simply does an MGET to fetch an array of items that it converts to a set:

redis.mget(*keys).compact.to_set

From this line, we're seeing errors in production for

NoMethodError: undefined method `compact' for "OK":String

This doesn't happen all the time, but it does happen with some frequency.

A Honeycomb graph over the past 24 hours showing 22 intermittent occurrences of this error in my app.

On the relevant traces in Honeycomb, we can see that this results from the MGET calls triggering AUTH calls when redis-rb reconnects. Somehow, the AUTH's return value of "OK" is being returned as the output of Redis#mget.

A single Honeycomb trace showing the error occurring during a Redis `MGET` command that triggers a nested `AUTH` command.

This doesn't always happen on reconnect, though. Many calls return the MGET data just fine, even with an AUTH. But when this error does happen, it always seems to have an AUTH triggered. So I think it might be a strange race condition or something.

The only thing I can think to try is using the block form of Redis#mget to see if the value yielded there is any different from the ultimate return value of Redis::Client#call...? Not really convinced that'll do anything though.

byroot commented 1 year ago

We're using the synchrony driver,

Closing as the synchrony driver have been removed.