Close Redis Sentinel connection after resolving role

On our production instances, Redis Sentinel is configured with the
default `maxclients` setting of 10,000, while the Redis nodes are
configured with 50,000. After upgrading to the `redis` v5 gem, we
found that our clients kept hitting `ERR max number of clients
reached` errors on Redis Sentinel nodes.

In the `redis` v4.80 gem, `client.disconnect` is always called here:
https://github.com/redis/redis-rb/blob/v4.8.0/lib/redis/client.rb#L630
This ensured that every time `resolve_master` or `resolve_slave`
executed, clients would be disconnected.

This commit restores that behavior by closing the client in
`RedisClient::SentinelConfig#each_sentinel`. This should not impact
performance since resolving the role of master or replica only needs
to happen once, and `config` is memoized.

To see the effect of this change, run this example script in a Ruby
console with one Redis Sentinel on port 26379, and one Redis node on
the default port 6379:

```ruby
require 'connection_pool'
require 'redis'

SENTINELS = [{ host: "127.0.0.1", port: 26379 }]
pool = ConnectionPool.new(size: 25) { Redis.new(name: "mymaster", sentinels: SENTINELS, role: :master) }

100.times do
  Thread.new do
    pool.with { |redis| redis.ping }
  end
end
```

Previously with `redis` v5.1.0, which uses `redis-client`, we see
there are 25 connections to Redis Sentinel, and 25 connections to
Redis:

```shell
$ lsof -p 1992535 | grep "localhost:26379" | wc -l
25
$ lsof -p 1992535 | grep "localhost:redis" | wc -l
25
```

With this change:

```shell
$ lsof -p 1997557 | grep "localhost:26379" | wc -l
0
$ lsof -p 1997557 | grep "localhost:redis" | wc -l
25
```

Co-authored-by: Sylvester Chin <[email protected]>

Author: stanhu

lib/redis_client/sentinel_config.rb

@@ -188,6 +188,10 @@ def each_sentinel
           if success
             @sentinel_configs.unshift(@sentinel_configs.delete(sentinel_config))
           end
+          # Redis Sentinels may be configured to have a lower maxclients setting than
+          # the Redis nodes. Close the connection to the Sentinel node to avoid using
+          # a connection.
+          sentinel_client.close
         end
       end
 

test/sentinel/sentinel_test.rb

@@ -50,8 +50,11 @@ def test_sentinel_all_down
     end
 
     class SentinelClientMock
+      attr_reader :close_count
+
       def initialize(responses)
         @responses = responses
+        @close_count = 0
       end
 
       def call(*args)
@@ -66,6 +69,10 @@ def call(*args)
           raise "Expected #{command.inspect}, got: #{args.inspect}"
         end
       end
+
+      def close
+        @close_count += 1
+      end
     end
 
     def test_unknown_master
@@ -184,6 +191,7 @@ def test_successful_connection_refreshes_sentinels_list
       assert_equal Servers::SENTINELS.length + 1, @config.sentinels.length
       assert_equal new_sentinel_ip, @config.sentinels.last.host
       assert_equal new_sentinel_port, @config.sentinels.last.port
+      assert_equal 1, sentinel_client_mock.close_count
     end
 
     def test_sentinel_refresh_password
@@ -211,6 +219,8 @@ def test_sentinel_refresh_password
       @config.sentinels.each do |sentinel|
         refute_nil sentinel.password
       end
+
+      assert_equal 1, sentinel_client_mock.close_count
     end
 
     def test_config_user_password_from_url_for_redis_master_replica_only