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

CrossSlotPipeliningError validation seems incorrect? #1019

Closed
ejinotti-mdsol opened this issue Jul 15, 2021 · 4 comments · Fixed by #1020
Closed

CrossSlotPipeliningError validation seems incorrect? #1019

ejinotti-mdsol opened this issue Jul 15, 2021 · 4 comments · Fixed by #1020

Comments

@ejinotti-mdsol
Copy link

From my reading of https://redis.io/topics/cluster-spec#keys-hash-tags it seems that the magic squiggly brackets to force a specific value for hashing should be acceptable anywhere in the key. However the raise of CrossSlotPipeliningError here:

raise CrossSlotPipeliningError, command_keys if node_keys.size > 1

Seems to only accept the magic squigglies as a prefix:

irb(main):001:0> REDIS.pipelined { ['{zed}one', '{zed}two'].each { |k| REDIS.exists?(k) } }
=> [false, false]
irb(main):002:0> REDIS.pipelined { ['one{zed}', 'two{zed}'].each { |k| REDIS.exists?(k) } }
Traceback (most recent call last):
        1: from (irb):2
Redis::Cluster::CrossSlotPipeliningError (Cluster client couldn't send pipelining to single node. The commands include cross slot keys. ["zed", "zed"])
irb(main):003:0> REDIS.pipelined { ['one{zed}one', 'two{zed}one'].each { |k| REDIS.exists?(k) } }
Traceback (most recent call last):
        2: from (irb):2
        1: from (irb):3:in `rescue in irb_binding'
Redis::Cluster::CrossSlotPipeliningError (Cluster client couldn't send pipelining to single node. The commands include cross slot keys. ["zed", "zed"])

This is v4.3.1

@ejinotti-mdsol
Copy link
Author

One additional piece of info: this was using a cluster with one shard and 5 replicas.

@johnduhart
Copy link

I had a brief look at this for the team, and I'm a bit suspicious of this code path:

node_keys = pipeline.commands.map { |cmd| find_node_key(cmd) }.compact.uniq

This is going to call find_node_key for each command individually, which results in looking up a slot (okay), and then calling find_node_key_of_slave. And then, here:

def find_node_key_of_slave(slot)
return nil unless exists?(slot)
return find_node_key_of_master(slot) if replica_disabled?
@map[slot][:slaves].sample
end

A random replica will be chosen, which may result in different node_keys even though all keys are in the same slot.

@supercaracal
Copy link
Contributor

It is a bug. It seems that the cross-slot validation should be skipped when commands in pipeline are sent to replicas.

@supercaracal
Copy link
Contributor

Actually, forget about that. The cross-slot validation in pipelining should be geared toward primary nodes.

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

Successfully merging a pull request may close this issue.

3 participants