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

Even when specifying sentinels, initial connect requires knowledge of current master #672

Closed
adimarco opened this issue Jan 24, 2017 · 7 comments

Comments

@adimarco
Copy link

adimarco commented Jan 24, 2017

Pardon me if I've missed something, but it appears that even when connecting to sentinel, Redis.new() requires the client to know which redis host is currently the master? This defeats the point of connecting to the sentinels at all.

e.g., I have 2 redis hosts on redis1 and redis2. redis1 is master, redis2 is slave (and I have a 3rd sentinel instance running on a host named arbiter). I connect like:

sentinels = [
  { :host => 'redis1', :port => 26379},  
  { :host => 'redis2', :port => 26379},
  { :host => 'arbiter', :port => 26379}]

redis = Redis.new(:url => 'redis://redis1', "sentinels" => sentinels, :role => :master)

and everything works fine.

However, if failover has occurred and redis1 and redis2 have swapped their roles as master and slave, if I restart my rails server or boot a new instance, the same connect incantation above yields:

Redis::CommandError: READONLY You can't write against a read only slave.

i.e., even though it has connected to the sentinels, it is not aware of the current master instance and attempts to write to the instance specified in the :url parameter regardless of the current master/slave configuration.

Am I missing something?

@badboy
Copy link
Contributor

badboy commented Jan 24, 2017

Is redis1 the name of your monitored pod in your sentinel configuration?

@adimarco
Copy link
Author

Actually it's the default from the examples: mymaster, though I can change that pretty easily. Does that name need to match?

@adimarco
Copy link
Author

Switching to redis://mymaster like so gets me -

irb(main):023:0> redis = Redis.new(:url => 'redis://mymaster', "sentinels" => sentinels, :role => :master)
=> #<Redis client v3.3.3 for redis://mymaster:6379/0>
irb(main):024:0> redis.set('foo','bar')
RuntimeError: Name or service not known
	from /var/lib/gems/2.3.0/gems/redis-3.3.3/lib/redis/connection/hiredis.rb:19:in `connect'
	from /var/lib/gems/2.3.0/gems/redis-3.3.3/lib/redis/connection/hiredis.rb:19:in `connect'
	from /var/lib/gems/2.3.0/gems/redis-3.3.3/lib/redis/client.rb:336:in `establish_connection'

Which I'm guessing means it's failing DNS resolution for mymaster since there's no host with that name? The sentinel hostnames are definitely correct.

@badboy
Copy link
Contributor

badboy commented Jan 24, 2017

Yes, the name in the first url needs to match, at least until we merge an option to specifiy it separately (at which point the url will not be used at all).

Are redis1, redis2 and arbiter actually resolvable domain names?

@adimarco
Copy link
Author

Yeah, redis1, redis2, and arbiter are all resolvable hostnames. The sentinel get-master-addr-by-name call returns an IP, though:

$ redis-cli -h arbiter -p 26379
arbiter:26379> sentinel get-master-addr-by-name mymaster
1) "10.0.199.170"
2) "6379"

So I guess I'm stuck until #640 gets merged? Renaming the sentinel pod to redis1 wouldn't do me much good if failover had happened and master was redis2.

Thanks for the quick reply.

@badboy
Copy link
Contributor

badboy commented Jan 24, 2017

Oh damn, I only see it now, you have run into another issue.

The following code should work for you:

sentinels = [
  { :host => 'redis1', :port => 26379},  
  { :host => 'redis2', :port => 26379},
  { :host => 'arbiter', :port => 26379}]

redis = Redis.new(:url => 'redis://mymaster', :sentinels => sentinels, :role => :master)

@adimarco
Copy link
Author

Heh. I had actually switched to "sentinels" because I apparently skimmed that issue a little too quickly and thought it was complaining that it didn't work with :sentinels symbolized.

That code you gave does work for me. Thanks for all your help.

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

2 participants