Conn.CreateProtectedEphemeralSequential loops endlessly when ZK is down about go-zookeeper HOT 4 CLOSED

(First off, thanks @samuel for the library. I was very happy to find a pure-Go Zookeper option!)

I recently ran into the issue raised by @onsi while working on the zk command line client based on this library. For reference, the error
looks like:

$ zk children /
2014/05/20 20:03:42 Failed to connect to 192.168.12.12:2182: dial tcp 192.168.12.12:2182: i/o timeout
2014/05/20 20:03:43 Failed to connect to 192.168.12.13:2182: dial tcp 192.168.12.13:2182: i/o timeout
2014/05/20 20:03:44 Failed to connect to 192.168.12.11:2182: dial tcp 192.168.12.11:2182: i/o timeout
2014/05/20 20:03:46 Failed to connect to 192.168.12.12:2182: dial tcp 192.168.12.12:2182: i/o timeout

Where the log messages above are emitted by the go-zookeeper library.

This is problematic for me for a couple reasons:

• With control not returning to the caller in the case of connection-time errors I'm not able to present a good error message to the user.
• The library emits log messages which I have no control over in terms of destination, formatting, etc.

@samuel do you have any thoughts about what could be done in the library and/or in calling code to handle connection-time errors to servers? I would be happy to make changes in my code and/or consider writing a patch for this library. Some ideas:

• The caller just needs to set up their own timeout, perhaps calling Close if it hits the timeout branch.
• If the library can't establish a connection with the first server it tries when Connect is called, return an error. Or return an error after all servers have been tried once. The calling code could retry Connects if it wanted to.
• Some sort of pluggable logging setup (this would only address the second point above).

from go-zookeeper.

Sorry, I should gotten to this a while back :)

I'm not sure if a timeout or instant fail is the best and open to either option, but I implemented the instant fail when all servers are tried but none succeed. This I hope will solve the common problem of waiting forever. For logging, it's possible to add an optional way to set a custom logger (accepting a log.Logger interface), but I'll leave that for another patch.

#30

Let me know if this works for you. Thanks!

from go-zookeeper.

Hey @samuel, thanks, this does definitely help. With this change we're able to avoid looping forever in our CLI tool / having to force an out-of-band timeout.

There are still some log messages that get dumped, but as you mentioned we can circle back on that latter.

from go-zookeeper.

Merged #30

from go-zookeeper.