redis-rb
redis-rb copied to clipboard
redis
Recommend alternatives to the deprecated `Redis.current` in the README and deprecation message
I'd be happy to make a PR if someone more knowledgeable can say what alternatives are suitable to recommend.
This would make for a better upgrade experience.
If it helps, I asked a similar question on the commit introducing the deprecation and got the following response from @byroot:
Because multi-threaded environments are very much the default these days, and sharing the same Redis instance between threads leads to tons of locking.
So I prefer not to encourage this anymore. As you said it's super easy to just use $redis instead if that's really what you want.
It's not the role of a database client to manage the lifecycle to the connection, it's up to the application to do that.
Do you recommend building an own place to store the redis client if needed
Yes. And you likely want to a connection pool with it.
I didn't expect people to still be using this to be honest.
There really isn't one replacement for it. If you really want the same behavior, you can use $redis global variable, at least it makes it obvious it's a shared global client. But I'd feel really weird recommending that in README or warning message.
The solution is mostly application specific so it's hard to give one guideline.
Thanks! It's fine for the recommendation to include that nuance, I think. Doesn't have to be super long. Maybe something along these lines?
You can replace it with a global
$redis = Redis.new(…), though this will share a single connection between threads, and could lead to lock contention in a multi-threaded environment. To avoid that, you can add a connection pool, e.g. as discussed in https://github.com/mperham/sidekiq/wiki/Using-Redis#complete-control.
For the single-connection case I guess there's no practical benefit (beyond subjective aesthetics) to replacing $redis with an accessor assigned at app boot, so I'm thinking this might be good enough.
I'd like to wait a bit to see if it's really causing confusion. I wouldn't be against a "deprecated" section in the readme, or better in a deprecations.md, but I'd rather not make the warning messages too big.
Alright. This issue and the updates to https://stackoverflow.com/questions/21075781/redis-global-variable-with-ruby-on-rails/34673035#34673035 may in themselves help people get an idea about their options, so perhaps it's fine. I thought it was odd to see the deprecation in the changelog without any context about reasons or alternatives.
For the record, this deprecation lead to some confusion on my team as well. I was very glad of this conversation and the suggestions and resources provided to point me in the right direction of how to solve this deprecation.
It left me wondering what to do too. I was using it so I could do things like Redis.current.ping in a health check URL, I also liked knowing it was there if I had to drop into using the raw Redis client instead of Rail's cache.
It would be great if the README file had a few examples of what to do in various use cases, for example if you're running a single Redis instance when using a multi-threaded web app server such as using Puma, etc.. Are there traffic considerations to take into account when picking a specific solution?
Maybe as a small counterpoint to the excessive locking around Redis.current: in lots of simple applications this is fair and as long as thread safety is ensured, the performance is not a problem. E.g. redis is sometimes only used to enqueue resque jobs every few seconds and lock contention is not a concern at all.
In the end it is also fair to remove it, just want to point out that not all use cases are high performance.
Nick Janetakis @.***> schrieb am Mo., 28. Feb. 2022, 19:01:
It left me wondering what to do too. I was using it so I could do things like Redis.current.ping in a health check URL, I also liked knowing it was there if I had to drop into using the raw Redis client instead of Rail's cache.
It would be great if the README file had a few examples of what to do in various use cases, for example if you're running a single Redis instance when using a multi-threaded web app server such as using Puma, etc.. Are there traffic considerations to take into account when picking a specific solution?
— Reply to this email directly, view it on GitHub https://github.com/redis/redis-rb/issues/1064#issuecomment-1054518640, or unsubscribe https://github.com/notifications/unsubscribe-auth/AADOXUB7GODEBVQTJCIH25DU5OZ6LANCNFSM5NOGZIIQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.
You are receiving this because you commented.Message ID: @.***>
@henrik Does this issue seem to be resolved by the following section, right?
https://github.com/redis/redis-rb?tab=readme-ov-file#connection-pooling-and-thread-safety
To me it answers all important questions:
- Yes it is safe to use a single instance in a multi threaded environment
- The recommended way is to pool connections for better scalability
- There is a suggestion how to do pooling
Mitsuaki Wada @.***> schrieb am Mi., 1. Mai 2024, 11:29:
@henrik https://github.com/henrik Does this issue seem to be resolved by the following section, right?
https://github.com/redis/redis-rb?tab=readme-ov-file#connection-pooling-and-thread-safety
— Reply to this email directly, view it on GitHub https://github.com/redis/redis-rb/issues/1064#issuecomment-2088206770, or unsubscribe https://github.com/notifications/unsubscribe-auth/AADOXUDML7M3XBKY2BQQIHLZACYZPAVCNFSM5NOGZII2U5DIOJSWCZC7NNSXTN2JONZXKZKDN5WW2ZLOOQ5TEMBYHAZDANRXG4YA . You are receiving this because you commented.Message ID: @.***>