Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow RedisCacheStore to fail safely when maximum client connections reached #37085

Merged
merged 1 commit into from
Oct 1, 2019

Conversation

bmedenwald
Copy link
Contributor

@bmedenwald bmedenwald commented Aug 29, 2019

This fixes #37337

Summary

Recently I encountered a service interruption relating to Heroku Redis, which I use for caching. The Redis server returned ERR max number of clients reached. Redis Cache Store has failsafes to make sure data is still returned when Redis suffers connection errors, but the max number of clients reached error is not a Redis::ConnectionError, it is instead a Redis::CommandError. Instead of being returned fresh data from the database, my users were sent 500 errors.

This fix simply changes the rescue to save requests from all Redis errors. Of course, errors are still reported to the error handler that can be registered so errors will still be sent via the method a user chooses. This is consistent with the other caching stores, which save from virtually all errors:

  • MemCacheStore rescues from all Dalli errors (Dalli::DalliError).
  • ReadThis (another Redis cache) rescues from Redis::BaseError.
  • FileStore rescues doesn't declare a specific type and rescues from all.

@bmedenwald bmedenwald changed the title Allow Redis Cache Store to fail safely when maximum client connections reached Allow RedisCacheStore to fail safely when maximum client connections reached Aug 29, 2019
@bmedenwald
Copy link
Contributor Author

@jeremy Any chance we could get this merged in for the next minor release? I could really use this on Heroku given the state of their Redis servers lately.

@bmedenwald
Copy link
Contributor Author

I've added #37337 to outline this fix with the hope that this can make the 6.0.1 build of Rails.

@georgeclaghorn georgeclaghorn merged commit 6229c2b into rails:master Oct 1, 2019
@georgeclaghorn georgeclaghorn added this to the 6.0.1 milestone Oct 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

RedisCacheStore Fails Request when Max Number of Client Connection Raised
2 participants