fix: several issues (#149)

Author: supercaracal

lib/redis_client/cluster/node.rb

@@ -18,10 +18,12 @@ class Node
       MAX_STARTUP_SAMPLE = 37
       MAX_THREADS = Integer(ENV.fetch('REDIS_CLIENT_MAX_THREADS', 5))
       IGNORE_GENERIC_CONFIG_KEYS = %i[url host port path].freeze
+      SLOT_OPTIMIZATION_STRING = '0' * SLOT_SIZE
 
       ReloadNeeded = Class.new(::RedisClient::Error)
+
       Info = Struct.new(
-        'RedisNode',
+        'RedisClusterNode',
         :id, :node_key, :role, :primary_id, :ping_sent,
         :pong_recv, :config_epoch, :link_state, :slots,
         keyword_init: true
@@ -35,23 +37,27 @@ def replica?
         end
       end
 
-      SLOT_OPTIMIZATION_MAX_SHARD_SIZE = 256
-      SLOT_OPTIMIZATION_STRING = '0' * SLOT_SIZE
-      Slot = Struct.new('RedisSlot', :slots, :primary_node_keys, keyword_init: true) do
-        def [](slot)
-          primary_node_keys[slots.getbyte(slot)]
+      Slot = Struct.new('StringArray', :string, :elements, keyword_init: true) do
+        def [](index)
+          raise IndexError if index < 0
+          return if index >= string.bytesize
+
+          elements[string.getbyte(index)]
         end
 
-        def []=(slot, primary_node_key)
-          index = primary_node_keys.find_index(primary_node_key)
-          if index.nil?
-            raise(::RedisClient::Cluster::Node::ReloadNeeded, primary_node_key) if primary_node_keys.size >= SLOT_OPTIMIZATION_MAX_SHARD_SIZE
+        def []=(index, element)
+          raise IndexError if index < 0
+          return if index >= string.bytesize
+
+          pos = elements.find_index(element) # O(N)
+          if pos.nil?
+            raise(RangeError, 'full of elements') if elements.size >= 256
 
-            index = primary_node_keys.size
-            primary_node_keys << primary_node_key
+            pos = elements.size
+            elements << element
           end
 
-          slots.setbyte(slot, index)
+          string.setbyte(index, pos)
         end
       end
 
@@ -229,7 +235,12 @@ def any_replica_node_key(seed: nil)
       def update_slot(slot, node_key)
         return if @mutex.locked?
 
-        @mutex.synchronize { @slots[slot] = node_key }
+        @mutex.synchronize do
+          @slots[slot] = node_key
+        rescue RangeError
+          @slots = Array.new(SLOT_SIZE) { |i| @slots[i] }
+          @slots[slot] = node_key
+        end
       end
 
       private
@@ -256,11 +267,11 @@ def build_slot_node_mappings(node_info_list)
       end
 
       def make_array_for_slot_node_mappings(node_info_list)
-        return Array.new(SLOT_SIZE) if node_info_list.count(&:primary?) > SLOT_OPTIMIZATION_MAX_SHARD_SIZE
+        return Array.new(SLOT_SIZE) if node_info_list.count(&:primary?) > 256
 
         ::RedisClient::Cluster::Node::Slot.new(
-          slots: String.new(SLOT_OPTIMIZATION_STRING, encoding: Encoding::BINARY, capacity: SLOT_SIZE),
-          primary_node_keys: node_info_list.select(&:primary?).map(&:node_key)
+          string: String.new(SLOT_OPTIMIZATION_STRING, encoding: Encoding::BINARY, capacity: SLOT_SIZE),
+          elements: node_info_list.select(&:primary?).map(&:node_key)
         )
       end
 

lib/redis_client/cluster/node/latency_replica.rb

@@ -21,7 +21,7 @@ def initialize(replications, options, pool, **kwargs)
           @replications.each_value { |keys| keys.sort_by! { |k| latencies.fetch(k) } }
           @replica_clients = select_replica_clients(@replications, @clients)
           @clients_for_scanning = select_clients_for_scanning(@replications, @clients)
-          @existed_replicas = @replications.reject { |_, v| v.empty? }.values
+          @existed_replicas = @replications.values.reject(&:empty?)
         end
 
         def clients_for_scanning(seed: nil) # rubocop:disable Lint/UnusedMethodArgument

lib/redis_client/cluster/router.rb

@@ -12,6 +12,7 @@ class RedisClient
   class Cluster
     class Router
       ZERO_CURSOR_FOR_SCAN = '0'
+      METHODS_FOR_BLOCKING_CMD = %i[blocking_call_v blocking_call].freeze
 
       attr_reader :node
 
@@ -91,7 +92,7 @@ def try_send(node, method, command, args, retry_count: 3, &block) # rubocop:disa
           raise
         end
       rescue ::RedisClient::ConnectionError => e
-        raise if method == :blocking_call_v || (method == :blocking_call && e.is_a?(RedisClient::ReadTimeoutError))
+        raise if METHODS_FOR_BLOCKING_CMD.include?(method) && e.is_a?(RedisClient::ReadTimeoutError)
         raise if retry_count <= 0
 
         update_cluster_info!

test/cluster_controller.rb

@@ -10,7 +10,7 @@ class ClusterController
   DEFAULT_TIMEOUT_SEC = 5.0
 
   RedisNodeInfo = Struct.new(
-    'RedisNodeInfo',
+    'RedisClusterNodeInfo',
     :id, :node_key, :flags, :role, :myself?, :primary_id, :ping_sent, :pong_recv,
     :config_epoch, :link_state, :slots, :client, :client_node_key,
     keyword_init: true

test/redis_client/cluster/test_node.rb

@@ -382,7 +382,7 @@ def test_make_array_for_slot_node_mappings_optimized
 
         want = node_info_list.first.node_key
         got = @test_node.send(:make_array_for_slot_node_mappings, node_info_list)
-        assert_instance_of(Struct::RedisSlot, got)
+        assert_instance_of(Struct::StringArray, got)
         ::RedisClient::Cluster::Node::SLOT_SIZE.times do |i|
           got[i] = want
           assert_equal(want, got[i], "Case: #{i}")
@@ -415,13 +415,19 @@ def test_make_array_for_slot_node_mappings_max_shard_size
         end
 
         got = @test_node.send(:make_array_for_slot_node_mappings, node_info_list)
-        assert_instance_of(Struct::RedisSlot, got)
+        assert_instance_of(Struct::StringArray, got)
 
         ::RedisClient::Cluster::Node::SLOT_SIZE.times { |i| got[i] = node_info_list.first.node_key }
 
         got[0] = 'newbie:6379'
         assert_equal('newbie:6379', got[0])
-        assert_raises(::RedisClient::Cluster::Node::ReloadNeeded) { got[0] = 'zombie:6379' }
+        assert_raises(RangeError) { got[0] = 'zombie:6379' }
+
+        assert_raises(IndexError) { got[-1] = 'newbie:6379' }
+        assert_raises(IndexError) { got[-1] }
+
+        got[16_384] = 'newbie:6379'
+        assert_nil(got[16_384])
       end
 
       def test_build_replication_mappings_regular

test/redis_client/test_cluster.rb

@@ -37,7 +37,7 @@ def test_call
 
         assert(@client.call('PING') { |r| r == 'PONG' })
 
-        assert_equal(2, @client.call('HSET', 'hash', { foo: 1, bar: 2 }))
+        assert_equal(2, @client.call('HSET', 'hash', foo: 1, bar: 2))
         wait_for_replication
         assert_equal(%w[1 2], @client.call('HMGET', 'hash', %w[foo bar]))
       end
@@ -53,7 +53,7 @@ def test_call_once
 
         assert(@client.call_once('PING') { |r| r == 'PONG' })
 
-        assert_equal(2, @client.call_once('HSET', 'hash', { foo: 1, bar: 2 }))
+        assert_equal(2, @client.call_once('HSET', 'hash', foo: 1, bar: 2))
         wait_for_replication
         assert_equal(%w[1 2], @client.call_once('HMGET', 'hash', %w[foo bar]))
       end