Rediscover-ability on ConnectionErrors

[alecjacobs5401]

Howdy!

I was wondering if there were any build in ways to do client rediscover-ability? I could tell from digging through the source code that any of the provided options to Redis.new (that have default values) can be specified as a lamba, but it doesn't look like anywhere in the code that lambda is executed anywhere but in the initializer.

For the Sentinel case, redis-rb does a good job of knowing when the master Redis instance is no longer reachable (it will clear the connection and attempt to connect through the list of provided sentinels), but it doesn't handle the case when the actual Sentinels go away. It will constantly try a "bad" Sentinel and always fail over to the next one.

Added to that, it looks like sentinels must always be passed in as an array of hashes as well, so there isn't the option to provide a lambda currently.

I've managed to put together a patch that works in the case that the Sentinels themselves go away.

Patch

class Redis
  class Client
    class Connector
      class Sentinel
        module SentinelConnectorPatch
          def sentinel_detect
            puts "detecting"
            sentinels = @sentinels.respond_to?(:call) ? @sentinels.call : @sentinels
            sentinels.each do |sentinel|
              puts "trying #{sentinel}"
              client = Client.new(@options.merge({ host: sentinel[:host], port: sentinel[:port], reconnect_attempts: 0 }))

              begin
                if result = yield(client)
                  unless @sentinels.respond_to?(:call)
                    # This sentinel responded. Make sure we ask it first next time.
                    @sentinels.delete(sentinel)
                    @sentinels.unshift(sentinel)
                  end
                  return result
                end
              rescue BaseConnectionError
              ensure
                client.disconnect
              end
            end

            raise CannotConnectError, "No sentinels available."
          end
        end

        prepend SentinelConnectorPatch
      end
    end
  end
end

Would this feature be something (given the proper tests) that y'all would want to add to this client library? By simply allowing for a lambda function to be provided, the consumer can run their own service discovery to account for node failures.