Allow "slaveof no one" in redis config

[sheldonh]

I'm trying to make it really easy for my team to try out haredis. So I'm making Get You Some Redis Cluster.

When I run haredis' test suite with "make test-cluster", I get this error:

Note: test requires redis daemons running locally on ports 6380, 6381, and 6382!
client: Error: getaddrinfo ENOTFOUND
    at errnoException (dns.js:37:11)
    at Object.onanswer [as oncomplete] (dns.js:124:16)
Uncaught exception: AssertionError: true == false
    at process.<anonymous> (/home/sheldonh/git/haredis/test.js:1429:12)
    at process.EventEmitter.emit (events.js:95:17)
    at process.exit (node.js:707:17)
    at RedisHAClient.<anonymous> (/home/sheldonh/git/haredis/test.js:1409:13)
    at RedisHAClient.EventEmitter.emit (events.js:95:17)
    at Node.<anonymous> (/home/sheldonh/git/haredis/index.js:516:14)
    at Node.EventEmitter.emit (events.js:95:17)
    at RedisClient.<anonymous> (/home/sheldonh/git/haredis/lib/node.js:65:10)
    at RedisClient.EventEmitter.emit (events.js:95:17)
    at /home/sheldonh/git/haredis/index.js:599:27
make: *** [test-cluster] Error 1

This comes from index.js, where we do

Node.resolveHost(info.master_host, function(err, host) { ... }

If the default master instance (6380) is configured with "slaveof no one", then info.master_host is "no".

And indeed, when I tcpdump, I see that my host sent out a DNS request for the hostname "no.hearnlan". That looks like it tried to resolve "no", appending the localdomain.

This pull request changes the guard condition from inspecting master_host to inspecting master_port. With this change, I get past the DNS problem, to a new error in the multi_1 test. But I bet that's unrelated and will raise a separate issue / pull request for that once I have a handle on it.

[sheldonh]

Hmmm... Is it possible that "slaveof no one" is allowed as a command, but not as a configuration option? If so, then the patch would only be necessary if we want to support the edge case where someone sends "slaveof no one" as a command. But I'm not sure people are supposed to be fiddling "behind haredis' back".

What do you think?

[carlos8f]

The configuration option syntax is:

slaveof <masterip> <masterport>

so "no one" applies only to the command version. the equivalent conf to "slaveof no one" would be commenting out the slaveof line.

haredis works best if all your nodes start out configured to be master (slaveof line in conf commented out) and it will detect or elect a master automatically, sending the slaveof commands.

disabling slaveof in your confs also has the benefit that if/when nodes restart, they will not accidentally replicate from a (possibly stale) node pointed to in the conf. instead, haredis will detect a "master conflict" (2 or more masters in the pool) and re-assign slave status to the appropriate node(s).

[sheldonh]

Ah, okay got it. Thanks for clarifying.

In that case, I think the README is misleading when it says "6380 should be SLAVEOF NO ONE. 6381 and 82 should be slaves to 6380." I don't mean "SLAVEOF NO ONE" should change -- that was just me being a newbie. I mean that the README recommends slaving instances.

On Thu, Apr 11, 2013 at 12:51 AM, Carlos Rodriguez <notifications@github.com

wrote:

The configuration option syntax is:

slaveof

so "no one" applies only to the command version. the equivalent conf to "slaveof no one" would be commenting out the slaveof line.

haredis works best if all your nodes start out configured to be master ( slaveof line in conf commented out) and it will detect or elect a master automatically, sending the slaveof commands.

disabling slaveof in your confs also has the benefit that if/when nodes restart, they will not accidentally replicate from a (possibly stale) node pointed to in the conf. instead, haredis will detect a "master conflict" (2 or more masters in the pool) and re-assign slave status to the appropriate node(s).

— Reply to this email directly or view it on GitHubhttps://github.com/carlos8f/haredis/pull/7#issuecomment-16207410 .