-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Recommend alternatives to the deprecated Redis.current
in the README and deprecation message
#1064
Comments
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.
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 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?
For the single-connection case I guess there's no practical benefit (beyond subjective aesthetics) to replacing |
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 |
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. |
Post-install message from sidekiq-unique-jobs: IMPORTANT! Automatic configuration of the sidekiq middleware is no longer done. Please see: https://github.com/mhenrixon/sidekiq-unique-jobs/blob/master/README.md#add-the-middleware `Redis.current=` is deprecated and will be removed in 5.0 redis/redis-rb#1064
Post-install message from sidekiq-unique-jobs: IMPORTANT! Automatic configuration of the sidekiq middleware is no longer done. Please see: https://github.com/mhenrixon/sidekiq-unique-jobs/blob/master/README.md#add-the-middleware `Redis.current=` is deprecated and will be removed in 5.0 redis/redis-rb#1064
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. |
Redis-rb, the Redis gem used by Rails apps, is deprecating a part of its API which stores a global Redis connection. See redis/redis-rb#1064 for context around that deprecation. To prepare for when this behavior will be removed, we must update this application so that Redis will be passed in as an instance to be checked, because it will not be globally available.
Redis-rb, the Redis gem used by Rails apps, is deprecating a part of its API which stores a global Redis connection. See redis/redis-rb#1064 for context around that deprecation. To prepare for when this behavior will be removed, we must update this application so that Redis will be passed in as an instance to be checked, because it will not be globally available.
Redis-rb, the Redis gem used by Rails apps, is deprecating a part of its API which stores a global Redis connection. See redis/redis-rb#1064 for context around that deprecation. To prepare for when this behavior will be removed, we must update this application so that Redis will be passed in as an instance to be checked, because it will not be globally available.
Redis-rb, the Redis gem used by Rails apps, is deprecating a part of its API which stores a global Redis connection. See redis/redis-rb#1064 for context around that deprecation. To prepare for when this behavior will be removed, we must update this application so that Redis will be passed in as an instance to be checked, because it will not be globally available.
It left me wondering what to do too. I was using it so I could do things like 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
<#1064 (comment)>,
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:
***@***.***>
|
because current has been removed in later versions without replacement. See redis/redis-rb#1064
@henrik 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
<#1064 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AADOXUDML7M3XBKY2BQQIHLZACYZPAVCNFSM5NOGZII2U5DIOJSWCZC7NNSXTN2JONZXKZKDN5WW2ZLOOQ5TEMBYHAZDANRXG4YA>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
Yeah, agreed. Closing this issue 🫡 |
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.
The text was updated successfully, but these errors were encountered: