Add new master_name param for sentinel. Fixes #531

[BookOfGreg]

Associated with issue #531.
Based on #534 with the addition of tests.
Didn't know how to keep @nguyenductung 's commit sadly but this PR does fix the issue and is tested.

[BookOfGreg]

Hi team,
All builds in Travis passed except JRuby9.0.5 on unstable.
https://travis-ci.org/redis/redis-rb/jobs/168656998
Seems like it failed with something to do with the redis_mock method. Am I using it incorrectly in my test?

[Dahie]

I just discovered the same issue and was about to file a similar pull request. Thanks for coming up with a solution already. :) I'd be very happy to see this merged.

[ac-astuartkregor]

Until this is merged, due to the nature of the issue, does this mean that sentinels must be installed on the same host as the redis master?

[Dahie]

In the configuration for each sentinel you define the address and port of the master and assign it a name. Right now the redis gem assumes this name is the same as the host name.

[finger-berlin]

Hi, this param is critical to make the gem work with sentinel, because the hostnames/ip addresses (should) have no relation to the master-name (http://redis.io/topics/sentinel). So +1(!). Thanks.

[ac-astuartkregor]

@Dahie The master name is assumed to be the same as the hostname of the redis master or the hostname of the sentinel? As far as I can tell, you connect to one or more sentinels using the host: parameter and then it assumes that the name of the cluster, as defined in the redis sentinel config is the same as the host: parameter, which I am assuming is the/a sentinel host.

[Dahie]

@ac-astuartkregor The gem currently assumes the master name is the same as the host the master redis is running on.

[ac-astuartkregor]

@Dahie Huh, I tried that and it barfed, so perhaps I messed up something else as well.

[BookOfGreg]

We're using redis with the following conf.
We've named our redis in sentinel to 'redis_queue' for the eventuality we add 'redis_cache' later
Works great :)

Sidekiq.configure_server do |config|
  config.redis = {
    sentinels: [
      { host: "queue.example.com", port: 26379 }
    ],
    failover_reconnect_timeout: 20,
    master_name: 'redis_queue'
  }
end

remember the configure_client part also

[BookOfGreg]

Ping. Is this repo still maintained?

[BookOfGreg]

I thought merging in master would help but turns out master currently fails the build also.

[badboy]

Yey for flaky tests. That's a known issue with current Redis and unrelated to this PR.

[finger-berlin]

Ping... any news here?

[BookOfGreg]

Looks like master still fails with Redis Unstable, meaning no PRs are being merged in this project until sorted?
https://travis-ci.org/redis/redis-rb

[BookOfGreg]

Ping @badboy

Is there anything left for me to do to get this into a mergeable state?

[badboy]

Nah, I just didn't spend any time on this project in the last months. I try to catch up.

[badboy]

@BookOfGreg could you add a line for this new option to the client documentation?

[badboy]

bors r+

[BookOfGreg]

Great catch, thank you!
I chose the words on the documentation line carefully so googling them is likely to come up with the sentinel monitor docs:

[sesposito]

@BookOfGreg hey, why was this PR closed and never merged, is there another way to pass the master_name into the configurations?

EDIT:
Figured out this works: #531 (comment)