Implement ZOOKEEPER-1576 for golang #188
Conversation
zk/dnshostprovider.go
Outdated
| @@ -34,17 +34,19 @@ func (hp *DNSHostProvider) Init(servers []string) error { | |||
| for _, server := range servers { | |||
| host, port, err := net.SplitHostPort(server) | |||
| if err != nil { | |||
| return err | |||
| // If one node is down, Init should still continue | |||
There was a problem hiding this comment.
why this change? in this case the node isn't down - instead it's a configuration error. shouldn't this fail-fast?
There was a problem hiding this comment.
Good call. If it's a configuration error, we should fail fast here
zk/dnshostprovider.go
Outdated
| } | ||
| for _, addr := range addrs { | ||
| found = append(found, net.JoinHostPort(addr, port)) | ||
| } | ||
| } | ||
|
|
||
| } | ||
| addrs, err := lookupHost(host) | ||
| if err != nil { | ||
| return err | ||
| // If one node is down, Init should still continue |
There was a problem hiding this comment.
Thanks @jdef for reviewing. I have updated this diff
|
rebase the pull request |
|
Prior to this change, problems resolving a host would error out. One of the consequences of this change is that if you specify 3 hosts, but there's an error resolving 1 of them, then the ZK connection will never attempt to connect to it. I'm not convinced that's desirable. For example, it's probably better to defer resolution of that host and try again later -- vs permanently forgetting about it. |
|
Thanks for the review! If there is a host down, we can add the down host into the dnsprovider server list and let the connect loop to automatically discover it when it's up again. That down host is just not used during Init. |
When one node in the zk connection string is down, the current behavior of this library is to fail the connection. In that case, Quorum does maintain but zkclient using this library will not be able to connect to zk.
This pull request implements Zookeeper-1576. As far as one node in the zk connection string is alive, zkclient should still able to connect.