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

Feature request: handle cross-slot multikey queries when all the keys are on the same node #6794

Open
o948 opened this issue Jan 19, 2020 · 3 comments

Comments

@o948
Copy link

o948 commented Jan 19, 2020

My team is building a service. Each client of the service is reading from its own Redis stream. Each stream gets new entries appended after a random delay of 1sec-1min. We want clients to get the new data as soon as possible, and we want to serve about 100k-1M clients at the same time.

It's awesome that XREAD can read from multiple streams, this can greatly improve resource utilization: less empty reads, connections blocked for less time. Unfortunately this doesn't quite work in Redis Cluster due to the requirement that all the keys must hash to the same slot.

With our target numbers I'm not sure we can afford not to read multiple streams with single connection. We are currently considering the following hack, where we would call a lua script passing multiple stream keys (that we know are currently on the same node) as arguments:

local out = {}
for i = 2, #ARGV, 2 do
        local xs = redis.call('XRANGE', ARGV[i], ARGV[i + 1], '+', 'COUNT', ARGV[1])
        out[#out + 1] = {ARGV[i], xs}
end
return out

however, the script has quite a bit of overhead compared to XREAD...

I know Redis Cluster multikey issue has been discussed multiple times before (e.g. #5061, #5118):

this would be a bomb - you'd have code that works today because of an accident of how the slots are distributed between shards but that can break when an admin very legitimately migrates some of the slots to another shard.

but how about adding an option to disable this check for clients that understand the risk?

Maybe a new config option enable-local-cross-slot (similar to enable-cross-slot option in Redis Cluster Proxy project)?

@zuiderkwast
Copy link
Contributor

zuiderkwast commented Jul 12, 2023

Blocking XREAD with multiple keys is indeed a good use case.

One situation where you know your keys belong to the same node is when you got the keys using SCAN on some node.

But there may be other smart ways of knowing this. Although we want to encourage a separation between client lib (mapping key to slot and node) and end user (only care about {tags}), I think this is something we can allow advanced users to do. Redis is not overly protective in other matters.

but how about adding an option to disable this check for clients that understand the risk?

I like this idea. Maybe cluster-allow-cross-slot-commands. The default should be off (don't allow).

@singh-bhawani
Copy link

@madolson Can we expect this feature in future Redis releases.

As we know that single key command << pipelining <<< multi key command to write/read n key from Redis, and to leverage this multi key command on Redis to get the higher ops we can enable cross slot command execution only if all multi key are owned by executing shard (not involving multiple shards for single multi key command).

Design:
Client: Already aware about slot range owned by each shard of Redis cluster and update with multiple topology refresh strategy.

To execute n key multi-key command, client groups this into K commands with hash ranges owned by each shards. (K is number of shards in cluster)
Then client async executes these multi key commands and join and return.

Since In above multiple nodes are involved to execute multi-key command, so their execution is not guaranteed to be atomic (so, they can actually break the atomic design of many Redis commands).

For that we can add a new config in redis.conf enable-cross-slot yes (by default no) and only considered when running Redis in cluster mode. {similar to Redis Proxy: https://github.com/RedisLabs/redis-cluster-proxy}

@madolson
Copy link
Contributor

@singh-bhawani No, because Redis decided to change the license on us. https://redis.com/blog/redis-adopts-dual-source-available-licensing/ . If you're still interested in this feature, please request it here. https://github.com/placeholderkv/placeholderkv/issues

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants