diff --git a/README.md b/README.md index 07b0ea5..39e5739 100644 --- a/README.md +++ b/README.md @@ -442,6 +442,7 @@ Also, it should handle errors. * https://redis.io/docs/reference/cluster-spec/ * https://github.com/redis/redis-rb/issues/1070 * https://github.com/redis/redis/issues/8948 +* https://github.com/valkey-io/valkey/issues/384 * https://github.com/antirez/redis-rb-cluster * https://twitter.com/antirez * http://antirez.com/latest/0 diff --git a/test/test_against_cluster_broken.rb b/test/test_against_cluster_broken.rb index a8cff3b..05ee480 100644 --- a/test/test_against_cluster_broken.rb +++ b/test/test_against_cluster_broken.rb @@ -3,7 +3,9 @@ require 'testing_helper' class TestAgainstClusterBroken < TestingWrapper - WAIT_SEC = 3 + WAIT_SEC = 1 + MAX_ATTEMPTS = 60 + NUMBER_OF_KEYS = 10 def setup @captured_commands = ::Middlewares::CommandCapture::CommandBuffer.new @@ -24,23 +26,26 @@ def setup ) @captured_commands.clear @redirect_count.clear + @cluster_down_error_count = 0 end def teardown @client&.close @controller&.close - print "#{@redirect_count.get}, ClusterNodesCall: #{@captured_commands.count('cluster', 'nodes')} = " + print "#{@redirect_count.get}, "\ + "ClusterNodesCall: #{@captured_commands.count('cluster', 'nodes')}, "\ + "ClusterDownError: #{@cluster_down_error_count} = " end def test_a_replica_is_down sacrifice = @controller.select_sacrifice_of_replica - do_test_a_node_is_down(sacrifice, number_of_keys: 10) + do_test_a_node_is_down(sacrifice, number_of_keys: NUMBER_OF_KEYS) refute(@captured_commands.count('cluster', 'nodes').zero?, @captured_commands.to_a.map(&:command)) end def test_a_primary_is_down sacrifice = @controller.select_sacrifice_of_primary - do_test_a_node_is_down(sacrifice, number_of_keys: 10) + do_test_a_node_is_down(sacrifice, number_of_keys: NUMBER_OF_KEYS) refute(@captured_commands.count('cluster', 'nodes').zero?, @captured_commands.to_a.map(&:command)) end @@ -57,8 +62,8 @@ def wait_for_replication def do_test_a_node_is_down(sacrifice, number_of_keys:) prepare_test_data(number_of_keys: number_of_keys) - kill_a_node(sacrifice, kill_attempts: 10) - wait_for_cluster_to_be_ready(wait_attempts: 10) + kill_a_node(sacrifice, kill_attempts: MAX_ATTEMPTS) + wait_for_cluster_to_be_ready(wait_attempts: MAX_ATTEMPTS) assert_equal('PONG', @client.call('PING'), 'Case: PING') do_assertions_without_pipelining(number_of_keys: number_of_keys) @@ -75,15 +80,15 @@ def kill_a_node(sacrifice, kill_attempts:) refute_nil(sacrifice, "#{sacrifice.config.host}:#{sacrifice.config.port}") loop do - break if kill_attempts <= 0 + raise MaxRetryExceeded if kill_attempts <= 0 + kill_attempts -= 1 sacrifice.call('SHUTDOWN', 'NOSAVE') rescue ::RedisClient::CommandError => e raise unless e.message.include?('Errors trying to SHUTDOWN') rescue ::RedisClient::ConnectionError break ensure - kill_attempts -= 1 sleep WAIT_SEC end @@ -92,11 +97,13 @@ def kill_a_node(sacrifice, kill_attempts:) def wait_for_cluster_to_be_ready(wait_attempts:) loop do - break if wait_attempts <= 0 || @client.call('PING') == 'PONG' + raise MaxRetryExceeded if wait_attempts <= 0 + + wait_attempts -= 1 + break if @client.call('PING') == 'PONG' rescue ::RedisClient::Cluster::NodeMightBeDown - # ignore + @cluster_down_error_count += 1 ensure - wait_attempts -= 1 sleep WAIT_SEC end end diff --git a/test/test_against_cluster_scale.rb b/test/test_against_cluster_scale.rb index 6f32bfc..6fd2dd8 100644 --- a/test/test_against_cluster_scale.rb +++ b/test/test_against_cluster_scale.rb @@ -3,6 +3,8 @@ require 'testing_helper' class TestAgainstClusterScale < TestingWrapper + WAIT_SEC = 1 + MAX_ATTEMPTS = 20 NUMBER_OF_KEYS = 20_000 def self.test_order @@ -23,12 +25,15 @@ def setup @client.call('echo', 'init') @captured_commands.clear @redirect_count.clear + @cluster_down_error_count = 0 end def teardown @client&.close @controller&.close - print "#{@redirect_count.get}, ClusterNodesCall: #{@captured_commands.count('cluster', 'nodes')} = " + print "#{@redirect_count.get}, "\ + "ClusterNodesCall: #{@captured_commands.count('cluster', 'nodes')}, "\ + "ClusterDownError: #{@cluster_down_error_count} = " end def test_01_scale_out @@ -57,12 +62,8 @@ def test_02_scale_in @controller.scale_in NUMBER_OF_KEYS.times do |i| - assert_equal(i.to_s, @client.call('GET', "key#{i}"), "Case: key#{i}") - rescue ::RedisClient::CommandError => e - raise unless e.message.start_with?('CLUSTERDOWN Hash slot not served') - - # FIXME: Why does the error occur? - p "key#{i}" + got = retry_call(attempts: MAX_ATTEMPTS) { @client.call('GET', "key#{i}") } + assert_equal(i.to_s, got, "Case: key#{i}") end want = TEST_NODE_URIS.size @@ -98,4 +99,18 @@ def build_additional_node_urls max = TEST_REDIS_PORTS.max (max + 1..max + 2).map { |port| "#{TEST_REDIS_SCHEME}://#{TEST_REDIS_HOST}:#{port}" } end + + def retry_call(attempts:) + loop do + raise MaxRetryExceeded if attempts <= 0 + + attempts -= 1 + break yield + rescue ::RedisClient::CommandError => e + raise unless e.message.start_with?('CLUSTERDOWN Hash slot not served') + + @cluster_down_error_count += 1 + sleep WAIT_SEC + end + end end diff --git a/test/test_against_cluster_state.rb b/test/test_against_cluster_state.rb index b3bf603..29df288 100644 --- a/test/test_against_cluster_state.rb +++ b/test/test_against_cluster_state.rb @@ -25,7 +25,8 @@ def setup def teardown @controller&.close @client&.close - print "#{@redirect_count.get}, ClusterNodesCall: #{@captured_commands.count('cluster', 'nodes')} = " + print "#{@redirect_count.get}, "\ + "ClusterNodesCall: #{@captured_commands.count('cluster', 'nodes')} = " end def test_the_state_of_cluster_down diff --git a/test/testing_helper.rb b/test/testing_helper.rb index 7f57bd7..206311f 100644 --- a/test/testing_helper.rb +++ b/test/testing_helper.rb @@ -14,6 +14,8 @@ when 'hiredis' then require 'hiredis-client' end +MaxRetryExceeded = Class.new(StandardError) + class TestingWrapper < Minitest::Test private