Ignore Sentinel slaves that are flaged as down (#829)

Author: supercaracal

lib/redis/client.rb

@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 require_relative "errors"
 require "socket"
 require "cgi"
@@ -349,7 +351,8 @@ def establish_connection
            Errno::EHOSTUNREACH,
            Errno::ENETUNREACH,
            Errno::ENOENT,
-           Errno::ETIMEDOUT
+           Errno::ETIMEDOUT,
+           Errno::EINVAL
 
       raise CannotConnectError, "Error connecting to Redis on #{location} (#{$!.class})"
     end
@@ -604,9 +607,19 @@ def resolve_master
         def resolve_slave
           sentinel_detect do |client|
             if reply = client.call(["sentinel", "slaves", @master])
-              slave = Hash[*reply.sample]
-
-              {:host => slave.fetch("ip"), :port => slave.fetch("port")}
+              slaves = reply.map { |s| s.each_slice(2).to_h }
+              slaves.each { |s| s['flags'] = s.fetch('flags').split(',') }
+              slaves.reject! { |s| s.fetch('flags').include?('s_down') }
+
+              if slaves.empty?
+                raise CannotConnectError, 'No slaves available.'
+              else
+                slave = slaves.sample
+                {
+                  host: slave.fetch('ip'),
+                  port: slave.fetch('port'),
+                }
+              end
             end
           end
         end

test/sentinel_test.rb

@@ -20,6 +20,36 @@ def test_sentinel_slave_role_connection
     assert_equal MASTER_PORT.to_i, actual[2]
   end
 
+  def test_the_client_can_connect_to_available_slaves
+    commands = {
+      sentinel: lambda do |*_|
+        [
+          ['ip', '127.0.0.1', 'port', '6382', 'flags', 'slave'],
+          ['ip', '127.0.0.1', 'port', '6383', 'flags', 's_down,slave,disconnected']
+        ]
+      end
+    }
+    RedisMock.start(commands) do |port|
+      redis = build_slave_role_client(sentinels: [{ host: 'localhost', port: port }])
+      assert_equal 'PONG', redis.ping
+    end
+  end
+
+  def test_the_client_raises_error_when_there_is_no_available_slaves
+    commands = {
+      sentinel: lambda do |*_|
+        [
+          ['ip', '127.0.0.1', 'port', '6382', 'flags', 's_down,slave,disconnected'],
+          ['ip', '127.0.0.1', 'port', '6383', 'flags', 's_down,slave,disconnected']
+        ]
+      end
+    }
+    RedisMock.start(commands) do |port|
+      redis = build_slave_role_client(sentinels: [{ host: 'localhost', port: port }])
+      assert_raise(Redis::CannotConnectError) { redis.ping }
+    end
+  end
+
   def test_sentinel_failover
     sentinels = [{:host => "127.0.0.1", :port => 26381},
                  {:host => "127.0.0.1", :port => 26382}]