redis-rb 5.0 #1120
redis-rb 5.0 #1120
Conversation
casperisfine
force-pushed
the
redis-rb-5.0
branch
6 times, most recently
from
0cbd654 to
a95c629
Compare
|
I'd suggest being more aggressive. Why not drop Redis 5? redis-rb 4.x supports Redis 5 just fine, people can continue using it if they don't want to upgrade for some reason. Redis 6.0 is from April 2020 and every major Redis provider supports 6.x. |
The problem isn't so much Redis 5, but RESP2. Some people migrated to Redis 6 or 7, but used proxies such as envoyproxy/envoy#17571 which don't yet support the newer protocol. That pains me but... So if I can support RESP2 in redis-rb 5.0, I can remove all the deprecated stuff and allow these people to upgrade, and in a shorter time I can drop RESP2 support in 6.0. That's the idea so far, we'll see how it goes. In casperisfine#1 a surprising amount of tests pass with extremely little effort. |
casperisfine
force-pushed
the
redis-rb-5.0
branch
2 times, most recently
from
1d4c0a3 to
71c2deb
Compare
casperisfine
force-pushed
the
redis-rb-5.0
branch
from
5c44568 to
63e436e
Compare
Even 5.0 is EOL, and it's the oldest version we test against, so no point keeping all these version checks.
`main` and `distributed` test suites pass. `cluster` and `sentinel` still need some updating
casperisfine
force-pushed
the
redis-rb-5.0
branch
from
01d6c9a to
08a2100
Compare
Now that we no longer swap the client during pipelining and such, we can get rid of that mess.
casperisfine
force-pushed
the
redis-rb-5.0
branch
from
5daaf35 to
ca491bc
Compare
|
Ok, so another status update:
Outstanding issues with
Overall I'm quite tempted to extra |
|
After sleeping on it, I think short term the best is to indeed split it in another gem, but keep it in the same repo so that it's easy to share code. One issue though is that the gem name is already in use -_- https://rubygems.org/gems/redis-cluster |
|
Ok so:
I think I'll bump the version number and merge this this afternoon, further changes can happen on master. I've pushed a |
|
Oh, and |
Now that synchrony is gone and hiredis support SSL we don't need to single out some drivers.
casperisfine
force-pushed
the
redis-rb-5.0
branch
from
ebce34d to
0dd4669
Compare
casperisfine
force-pushed
the
redis-rb-5.0
branch
from
37609ff to
dbfc6cc
Compare
Some threads would stay subscribed to various channel causing state leak across threads. It's still not perfect but should be much more stable.
casperisfine
force-pushed
the
redis-rb-5.0
branch
2 times, most recently
from
f705f90 to
578f752
Compare
casperisfine
force-pushed
the
redis-rb-5.0
branch
from
578f752 to
418e872
Compare
|
Aaaand it's green! Further work will happen on master. |
|
I’m sorry for the late reply. I agree with the decision. |
|
@byroot I might create a new issue... and let me know if I should, but wanted to see if I was missing anything. We use clustering for our Rails Cache (using the official Maybe I should work with the rails project to get clustering support working over there. |
|
Rails ActiveSupport::Cache::RedisCacheStore.new(redis: Redis::Cluster.new(....))or config.cache_store = :redis_store, { redis: Redis::Cluster.new(....) }NB: I didn't test, I may have got some of the syntax partially wrong, but the general idea works. |
|
Oh I definitely missed that... I'll play around with it. Thanks a lot! ...it was even right there in the method I highlighted 😅 |
Ok. There was an initializer introduced in discourse#11088 ``` Rack::MiniProfiler.counter_method(Redis::Client, :call) { 'redis' } ``` This now throws an error ``` gems/rack-mini-profiler-3.3.1/lib/mini_profiler/profiling_methods.rb:87:in alias_method': undefined method call' for class `Redis::Client' ``` The error states that the method `def call` is no longer defined for Redis::Client`. Which is accurate since that method was dropped in 5.0 via redis/redis-rb@349ddb1 ( redis/redis-rb#1120 ) This change ensures a known redis api
Ref: #1070
Ok, let's start the work to see what kind of roadblocks we'll run into.
One idea I'm toying with right now, is that rather than delegate everything to
redis-clientwhich would require Redis 6+, we could useredis-clientwith theRESP2protocol and keep all our type casting. The downside is that it doesn't allow to delete as much code, the upside is that we don't break people who are still on Redis 5, and we get the much better drivers ofredis-client.