Skip to content

Commit

Permalink
Extend SlotLoader to work with multiple slot ranges
Browse files Browse the repository at this point in the history 
With multiple resharding, different slot ranges can be assigned to a
node. Current logic only pre assigns the last slot range, causing errors
& retries for unassigned slots.
This commit makes it work with multiple slot ranges and assigning all
slots.
  • Loading branch information
@rahul342
rahul342 committed Mar 23, 2020
1 parent 21ee8f4 commit 5849cb2
Show file tree
Hide file tree
Showing 3 changed files with 69 additions and 9 deletions.
7 changes: 5 additions & 2 deletions lib/redis/cluster/slot.rb
View file
Expand Up @@ -50,9 +50,12 @@ def slave?(node_key)
@node_flags[node_key] == ROLE_SLAVE
end

# available_slots is mapping of node_key to list of slot ranges
def build_slot_node_key_map(available_slots)
available_slots.each_with_object({}) do |(node_key, slots), acc|
slots.each { |slot| assign_node_key(acc, slot, node_key) }
available_slots.each_with_object({}) do |(node_key, slots_arr), acc|
slots_arr.each do |slots|
slots.each { |slot| assign_node_key(acc, slot, node_key) }
end
end
end

Expand Down
9 changes: 5 additions & 4 deletions lib/redis/cluster/slot_loader.rb
View file
Expand Up @@ -13,7 +13,7 @@ def load(nodes)
info = {}

nodes.each do |node|
info = Hash[*fetch_slot_info(node)]
info = fetch_slot_info(node)
info.empty? ? next : break
end

Expand All @@ -23,9 +23,11 @@ def load(nodes)
end

def fetch_slot_info(node)
hash_with_default_arr = Hash.new { |h, k| h[k] = [] }
node.call(%i[cluster slots])
.map { |arr| parse_slot_info(arr, default_ip: node.host) }
.flatten
.flat_map { |arr| parse_slot_info(arr, default_ip: node.host) }
.each_with_object(hash_with_default_arr) { |arr, h| h[arr[0]] << arr[1] }

rescue CannotConnectError, ConnectionError, CommandError
{} # can retry on another node
end
Expand All @@ -34,7 +36,6 @@ def parse_slot_info(arr, default_ip:)
first_slot, last_slot = arr[0..1]
slot_range = (first_slot..last_slot).freeze
arr[2..-1].map { |addr| [stringify_node_key(addr, default_ip), slot_range] }
.flatten
end

def stringify_node_key(arr, default_ip)
Expand Down
62 changes: 59 additions & 3 deletions test/cluster_client_slots_test.rb
View file
Expand Up @@ -7,7 +7,7 @@ class TestClusterClientSlots < Minitest::Test
include Helper::Cluster

def test_slot_class
slot = Redis::Cluster::Slot.new('127.0.0.1:7000' => 1..10)
slot = Redis::Cluster::Slot.new('127.0.0.1:7000' => [1..10])

assert_equal false, slot.exists?(0)
assert_equal true, slot.exists?(1)
Expand All @@ -26,15 +26,67 @@ def test_slot_class
assert_nil slot.put(1, '127.0.0.1:7001')
end

def test_slot_class_with_multiple_slot_ranges
slot = Redis::Cluster::Slot.new('127.0.0.1:7000' => [1..10, 30..40])

assert_equal false, slot.exists?(0)
assert_equal true, slot.exists?(1)
assert_equal true, slot.exists?(10)
assert_equal false, slot.exists?(11)
assert_equal true, slot.exists?(30)
assert_equal true, slot.exists?(40)
assert_equal false, slot.exists?(41)

assert_nil slot.find_node_key_of_master(0)
assert_nil slot.find_node_key_of_slave(0)
assert_equal '127.0.0.1:7000', slot.find_node_key_of_master(1)
assert_equal '127.0.0.1:7000', slot.find_node_key_of_slave(1)
assert_equal '127.0.0.1:7000', slot.find_node_key_of_master(10)
assert_equal '127.0.0.1:7000', slot.find_node_key_of_slave(10)
assert_equal '127.0.0.1:7000', slot.find_node_key_of_slave(30)
assert_equal '127.0.0.1:7000', slot.find_node_key_of_slave(40)
assert_nil slot.find_node_key_of_master(11)
assert_nil slot.find_node_key_of_slave(11)
assert_nil slot.find_node_key_of_master(41)
assert_nil slot.find_node_key_of_slave(41)

assert_nil slot.put(1, '127.0.0.1:7001')
assert_nil slot.put(30, '127.0.0.1:7001')
end

def test_slot_class_with_node_flags_and_replicas
slot = Redis::Cluster::Slot.new({ '127.0.0.1:7000' => 1..10, '127.0.0.1:7001' => 1..10 },
slot = Redis::Cluster::Slot.new({ '127.0.0.1:7000' => [1..10], '127.0.0.1:7001' => [1..10] },
{ '127.0.0.1:7000' => 'master', '127.0.0.1:7001' => 'slave' },
true)

assert_equal false, slot.exists?(0)
assert_equal true, slot.exists?(1)
assert_equal true, slot.exists?(10)
assert_equal false, slot.exists?(11)

assert_nil slot.find_node_key_of_master(0)
assert_nil slot.find_node_key_of_slave(0)
assert_equal '127.0.0.1:7000', slot.find_node_key_of_master(1)
assert_equal '127.0.0.1:7001', slot.find_node_key_of_slave(1)
assert_equal '127.0.0.1:7000', slot.find_node_key_of_master(10)
assert_equal '127.0.0.1:7001', slot.find_node_key_of_slave(10)
assert_nil slot.find_node_key_of_master(11)
assert_nil slot.find_node_key_of_slave(11)

assert_nil slot.put(1, '127.0.0.1:7002')
end

def test_slot_class_with_node_flags_replicas_and_slot_range
slot = Redis::Cluster::Slot.new({ '127.0.0.1:7000' => [1..10, 30..40], '127.0.0.1:7001' => [1..10, 30..40] },
{ '127.0.0.1:7000' => 'master', '127.0.0.1:7001' => 'slave' },
true)

assert_equal false, slot.exists?(0)
assert_equal true, slot.exists?(1)
assert_equal true, slot.exists?(10)
assert_equal false, slot.exists?(11)
assert_equal true, slot.exists?(30)
assert_equal false, slot.exists?(41)

assert_nil slot.find_node_key_of_master(0)
assert_nil slot.find_node_key_of_slave(0)
Expand All @@ -44,12 +96,16 @@ def test_slot_class_with_node_flags_and_replicas
assert_equal '127.0.0.1:7001', slot.find_node_key_of_slave(10)
assert_nil slot.find_node_key_of_master(11)
assert_nil slot.find_node_key_of_slave(11)
assert_equal '127.0.0.1:7000', slot.find_node_key_of_master(30)
assert_equal '127.0.0.1:7001', slot.find_node_key_of_slave(30)
assert_nil slot.find_node_key_of_master(41)
assert_nil slot.find_node_key_of_slave(41)

assert_nil slot.put(1, '127.0.0.1:7002')
end

def test_slot_class_with_node_flags_and_without_replicas
slot = Redis::Cluster::Slot.new({ '127.0.0.1:7000' => 1..10, '127.0.0.1:7001' => 1..10 },
slot = Redis::Cluster::Slot.new({ '127.0.0.1:7000' => [1..10], '127.0.0.1:7001' => [1..10] },
{ '127.0.0.1:7000' => 'master', '127.0.0.1:7001' => 'slave' },
false)

Expand Down

0 comments on commit 5849cb2

Please sign in to comment.