redis-rb icon indicating copy to clipboard operation
redis-rb copied to clipboard

Plan for redis-rb 5.0

Open byroot opened this issue 3 years ago • 14 comments

A large part of redis-rb maintenance cost comes from having to implement each command Redis add. A year after Redis 6.2 release, we're still missing a large part of the newly added commands, and tengentially commands added by Redis modules are hard to use.

This comes down to the RESP2 protocol that requires to know how to parse each command output.

However Redis 6.0 introduced the RESP3 protocol, which is properly typed allowing clients to not know anything about the commands they are sending, which means a drastically simpler and smaller implementation that don't need keeping up with commands added upstream.

So I'd like to split redis-rb in multiple gems (probably kept in the same git repository)

redis-client gem

What I'd like to do is to start a new simpler redis-client gem, that only support Redis 6.0+ and pretty much only offer the following API:

redis = Redis::Client.new(...)

# sending command
redis.call('SMEMBERS', 'mykey', ...)

# pipelining
redis.pipeline do |pipe|
  pipe.call(...)
end

We may also need some API for dealing with subscriptions commands, but that's about it unless I'm missing something.

This simple client would:

  • Not support redis-cluster, it should be implemented as another gem, the vast majority of people don't need it.
  • No longer wrap calls with a mutex, it's a mis-feature of redis-rb, to share a client between theads use a thread pool.

Users should be encouraged to use redis-client directly.

I explored this avenue back in 2019.

redis-cluster-client gem

Clustering support adds lots of complexity, so I think it should be shipped separately for people that need it.

It should however be API compatible.

One difficulty with clustering is that the client must be able to extract the parts of the command that are keys.

Right now this is handled with lots of ad hoc code for each command, but we should instead load the list of command signatures from the redis server with COMMAND and extract the keys that way.

With the above redis-cluster-client should be able to be a drop-in replacement for redis-client

redis-rb 5.0

redis-rb should then be refactored to be a thin wrapper around redis-client allowing to keep backward compatibility for the most part. Users would still be encouraged to migrate to redis-client even though there is not plan to sunset the redis gem any time soon.

Since we wouldn't have to know about return types, we could also explore generating the methods from the COMMANDS output, as to simplify keeping up with Redis.

Timeline

I don't have a specific timeline yet.

Redis 6.0 will soon be the oldest supported version. I also see that Sidekiq plans to only support Redis 6+ soon, so 2022 is likely the good time for this to happen.

byroot avatar Feb 14 '22 10:02 byroot

Yes, I’d love to switch to something like redis-client and lose the redundant mutex. I would also up the minimum Ruby version to 2.7 and clean up any legacy IO hacks.

Sidekiq 7.0 will match Rails defaults of Ruby 2.7+ and require Redis 6+.

mperham avatar Feb 14 '22 19:02 mperham

Current implementation of cluster mode already extracts keys by using COMMAND information. It might be easy to separate them.

  • https://github.com/redis/redis-rb/blob/master/lib/redis/cluster/command_loader.rb
  • https://github.com/redis/redis-rb/blob/master/lib/redis/cluster/command.rb

supercaracal avatar Feb 15 '22 00:02 supercaracal

Current redis-rb has the following features. Are they should to be out of support or to be separate?

  • Sentinel support
  • Client side consistent hashing
    • https://github.com/redis/redis-rb/blob/master/lib/redis/distributed.rb
    • https://redis.io/topics/partitioning#clients-supporting-consistent-hashing

supercaracal avatar Feb 15 '22 00:02 supercaracal

Are they should to be out of support or to be separate?

So, I don't have a specific answer to it right now, but my reasoning is: 95% of users should only need redis-client.

My hunch says both Sentinel and consistent hashing are only used by a small minority of users. So they should probably come in a well integrated extra gem.

We'll have to experiment a bit how we can make all these plug an play. Maybe some kind of "middleware system" might make sense.

casperisfine avatar Feb 15 '22 08:02 casperisfine

So I'd like to split redis-rb in multiple gems (probably kept in the same git repository)

So I've put a bit more thought into it, and while in theory I think a mono-repo with both redis and redis-client would be best, in practice I'm still not admin of redis/redis-rb etc, which creates some complications, most notably I can't add regular contributors as owners (thinking @supercaracal here who's been pretty much single handedly maintaining the redis-cluster stuff).

So I'm very tempted to do this in a new repo.

casperisfine avatar Feb 28 '22 10:02 casperisfine

Ok, so a quick update. The code can be seen at: https://github.com/redis-rb/redis-client

I believe it's now pretty much feature complete for "regular" redis (no cluster support). At this stage is mostly need to be tested in production like conditions to iron out potential bugs.

There's also an open question on wether it should provide pooling out of the box or not (https://github.com/redis-rb/redis-client/issues/4).

@mperham are you interested in trying to convert Sidekiq to use redis-client? Your feedback would be appreciated. Alternatively I can try to do it myself and open a PR. The goal would be mostly to see if there's any issue with the library "ergonomics".

Also @supercaracal, let me know if you'd like to participate on the cluster support, I can add you as a collaborator to the redis-rb organization, otherwise I'll probably look at that in a few days. I'm still undecided wether it should be a separate gem or not, and separate repo or not, let me know if you have an opinion.

casperisfine avatar Mar 21 '22 14:03 casperisfine

@casperisfine I love the idea and I would be happy to help with a prototype spike if you want to open an issue or PR against Sidekiq's 7-0 branch. This week is bad as I'm taking a small vacation but I can review and work on it next week.

mperham avatar Mar 21 '22 16:03 mperham

@mperham sounds good, I'll take a stab at it tomorrow. Enjoy your vacation!

casperisfine avatar Mar 21 '22 18:03 casperisfine

@casperisfine I would definitely like to participate in the organization.

I think it would be better to separate gem and repository for cluster support. It seems that Redis cluster V2 will no longer need heavy implementation of the clients. So we can maybe remove cluster support in the future. I'd say that the redis-client should be kept thin.

https://github.com/redis/redis/issues/8948

supercaracal avatar Mar 22 '22 10:03 supercaracal

@supercaracal sounds good. I sent you an invite, feel free to create a redis-cluster-client repo whenever you want.

For now redis-client is still a bit of a moving target, so it's up to you wether you'd like to experiment now, or wait a bit more. Feel free to tag me on issues or PRs if you need any information.

casperisfine avatar Mar 22 '22 11:03 casperisfine

@casperisfine I understand. Thank you for your invitation.

supercaracal avatar Mar 22 '22 11:03 supercaracal

Quick update here since it has been a while.

redis-client is now feature complete enough to support Sidekiq use case. Also the retrofit effort in Sidekiq showed that rebuilding redis-rb on top of redis-client should be fairly easy: https://github.com/mperham/sidekiq/pull/5298.

Since Redis 5.x is no longer supported (October 31, 2021), I'll likely soon start working on a new major version of the redis gem as planned.

However since it's quite a big breaking change, I'll likely cut a few more 4.x releases to ease the transition. The first 5.x might also be beta releases, we'll see.

casperisfine avatar May 05 '22 08:05 casperisfine

redis-cluster-client gem is here: https://github.com/redis-rb/redis-cluster-client

supercaracal avatar Jun 17 '22 07:06 supercaracal

❤️

casperisfine avatar Jun 17 '22 08:06 casperisfine

Closing as most of this has now been merged. I will likely release a beta in a couple days.

byroot avatar Aug 17 '22 16:08 byroot