Add support for quorum

[richardbadman]

This is in response to Issue #6.

Initial commit is simple in that we accept a vec of SocketAddr instead, this allows us to supply multiple hosts, where following the java docs it will shuffle the vec to provide a random candidate.

From the issue there is a comment of;

also handle session expiry differently from errors during reconnect.

But I'm not entirely sure what this means. So I'm happy for advice/pointers or more explanation.

I'm not expecting this to be the final complete solution either, for example, I was wondering if we wanted to persist the overall quorum information incase of connection error/dropped

[stackable-cla]

All committers have signed the CLA.

[maltesander]

Hi @richardbadman ,

thank you for tackling this.

You did the first part of #6 (connect).
There should also be a reconnect part of this PR

tokio-zookeeper/src/proto/packetizer.rs

Line 257 in 9c5f622

let retry = S::connect(this.addr.clone())

Currently the Packetizer holds a single addr and does not shuffle for reconnects. So if that server dies we retry that servrer forever and never fail over. The state is Reconnecting and a failed retry hits the error path at

tokio-zookeeper/src/proto/packetizer.rs

Line 236 in 9c5f622

return Poll::Ready(Err(e));

and exits the task entirely. So a single failed attempt kills the client with no backoff.

Could you move the shuffled list into Packetizer and rotate through it on reconnect?

For the expiry and persist the quorum information question:

Yes that is the reconnect part. A disconnect is recoverable, an expiry is not. On reconnect we resend Connect with the old session_id, and if the session survived, the server adopts it along with our watches and ephemeral nodes. If it expired, the server replies with session_id = 0 and timeout = 0, meaning "your session is gone". At which point the right move is to stop, emit KeeperState::Expired, and let the caller build a new client.

Right now we never look: the reconnect handshake response goes into a oneshot that's only traced

tokio-zookeeper/src/proto/packetizer.rs

Line 270 in 9c5f622

let (tx, rx) = oneshot::channel();

This is not part of this PR (and a general mess), since it also touches leader election...

One thing that might be worth considering is SocketAddr or now Vec<SocketAddr> which is IP only (zk-0.zk.svc.cluster.local:2181 wont parse).
I would not see this as part of this PR as well, but since we do a breaking signature change we might fix that for good (not required).

Small things for later:

• Breaking changelog entry
• I would not use the (crypto) rand dependency crate to shuffle servers
• whatever!("Could not connect to any node in quorum") drops all underlying error context
• quorom is a server-side concept (the majority needed to commit a write). What's being passed is the connect string / ensemble? Since this is public API, could you rename to addrs / ensemble? And connect_addr -> connect_addrs in the tests too.

Malte