Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rediscover-ability on ConnectionErrors #893

Open
alecjacobs5401 opened this issue Mar 3, 2020 · 0 comments
Open

Rediscover-ability on ConnectionErrors #893

alecjacobs5401 opened this issue Mar 3, 2020 · 0 comments

Comments

@alecjacobs5401
Copy link

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant