Open sj26 opened 4 years ago
Hello,
I would say that we should keep consistency as a client's API if we implement the feature.
I think we can ignore Redis::Distributed
as a client side consistent hashing feature in this case.
@byroot would be contribution of support for REDIS_SENTINEL_URLS
welcomed?
would be contribution of support for
REDIS_SENTINEL_URLS
welcomed?
Yes and no :)
I don't really want the gem to read magic environment variables, IMO reading $REDIS_URL
in Redis#initialize
is a mistake / historical cruft, it should be the application that passes the url.
However, I'm open to changes that would make instantiating a client with $REDIS_SENTINEL_URLS
a one liner, e.g. Redis.new(sentinel_urls: ENV["REDIS_SENTINEL_URLS"])
or something like that.
@casperisfine alternatively what about to support some kind of REDIS_URL with query parameters to setup various options like sentinels, role, ...?
Yeah, worth exploring. I suppose a combinaison of scheme and query parameters might work.
Would be worth having a quick look if other clients have something similar, or if some hosting services already have a convention for this stuff.
Would be worth having a quick look if other clients have something similar, or if some hosting services already have a convention for this stuff.
I quickly check other libraries and there is no support for sentinel setup with ENV vars. :(
there is no support for sentinel setup with ENV vars. :(
kk, thanks for looking.
I'll take another look and try to bring back some ideas.
There's even a really old gem that adds the functionality of setting a "redis+sentinel://" URL to the redis-rb gem. As there were no updates to this gem since 6 years I hoped that this feature was already included in redis-rb.
I'm currently planning to use a redis+sentinel replication on Kubernetes as a target for sidekiq and it would be very helpful if I could use the RAILS_ENV variable to tell the application if it should connect to a single redis instance in dev or a sentinel-setup in production.
@arusa for now we have following "naive" approach (it is not bulletproof) in app.
class Config
def self.redis
{
url: ENV['REDIS_URL'],
name: ENV['REDIS_SENTINEL_NAME'],
sentinels: ENV['REDIS_SENTINEL_URLS']&.split(',')&.map { |url| {url: url} },
driver: :hiredis,
timeout: 60,
}.compact
end
end
# ...
redis = Redis.new(Config.redis)
It's a common pattern to configure an application with something like:
When migration from a single Redis instance to a redis cluster, this become a little awkward, because the ruby redis library must be explicitly put into cluster mode:
Especially if you have multiple redis connections
and defaults:
It'd be super nice if we could just change the URL to indicate that it is a cluster URL. Maybe a query parameter in the style of
DATABASE_URL=postgres://localhost/name?encrypt=true
parameters like:or indicate that the connection mode is fundamentally different by using a different scheme:
which pares that later example back to:
Would there be interest in a PR to implement either the query param or URL scheme option? I'm leaning toward the query parameter because most redis libraries don't seem to need to differentiate, so there isn't a precedent to change the scheme, and it's technically the same protocol.