Zookeeper driver support

[fanatic]

Adds a zookeeper driver that matches conventions used in the etcd, memcached, and redis libraries. All tests pass.

Note that unit tests require zookeeper server to be started by hand on :2182 so this may not be able to be merged until it can be modified to be skipped.

Also, I don't yet clean up dead backends after the ttls have expired, I just check the ttl on read. A future iteration should remove znodes in the dead path.

[dmp42]

@fanatic : great!!! Love it!

Let me have a look at it and get back to you soon (we need to get 0.3 released first, but this will definitely make it for 0.4).

About skipping the tests (on environments where we don't have / want a zookeeper service), have a look at how it's done for etcd:

• https://github.com/dotcloud/hipache/blob/master/.travis.yml#L6
• https://github.com/dotcloud/hipache/blob/master/test/unit/driver-etcd.js#L7

Thanks!

[dmp42]

Also: can you run gulp hint (or jshint) and make jshint happy on your files?

[fanatic]

Thanks for the encouragement (this is my first pull request)! I'll clean it up -- thanks for your feedback.

[fanatic]

I'd still like to refactor some of the get-then-set methods to respect versioning for atomic changes.

@dmp42 do you have any suggestions for cleaning up the entries in zookeeper that the unit tests add? I've been cleaning them up by hand after each run.

Zookeeper
zookeeper://:2182
✓ Domain with no match, no fallback
✓ Single domain with a backend
✓ Single domain with multiple backends
✓ Single domain with multiple backends and fallback
✓ Single domain with multiple backends and fallback plus dead
✓ Single domain with multiple backends and fallback plus a second dead
✓ ... let it expire (1505ms)
zookeeper://:2182/#someprefix
✓ Domain with no match, no fallback
✓ Single domain with a backend
✓ Single domain with multiple backends
✓ Single domain with multiple backends and fallback
✓ Single domain with multiple backends and fallback plus dead
✓ Single domain with multiple backends and fallback plus a second dead
✓ ... let it expire (1504ms)

14 passing (3s)

[dmp42]

@fanatic I guess zookeeper can be launched (by our fixtures script) specifying command line arguments and/or a custom config file. That config file would then point the zookeeper dataDir to a specific folder (say: /tmp/hipache-test) that we may be able to rm once the tests are finished - at least it wouldn't mess-up with other things on that zookeeper.

What do you think?

[fanatic]

Sounds like a plan. I was starting the server like this:
/usr/lib/zookeeper/bin/zkServer.sh start ./zoo-dev.cfg

And using this config:

# zoo-dev.cfg
# The number of milliseconds of each tick
tickTime=2000
# The number of ticks that the initial
# synchronization phase can take
initLimit=10
# The number of ticks that can pass between
# sending a request and getting an acknowledgement
syncLimit=5
maxClientCnxns=1000
# the directory where the snapshot is stored.
dataDir=/tmp/hipache-test/zoo-data
# the port at which the clients will connect
clientPort=2182

[dmp42]

I have the hope that zkServer is just a (very? simple?) wrapper that ultimately does something in the line of: java org.apache.zookeeper pathtoconfig

Given this is for the purpose of running tests available to developers only, I don't quite care if our launcher is not "very" portable.

So, if you can find a hack, or even commit a copy of zkServer.sh in our fixtures, I'd be ok with that.

And yes, commit the tests' config file as well.

[dmp42]

One on each line might be more readable?
Anyhow, thinking more about this: it might very well be possible to ask travis to install zookeeper (look at the other apt-get install we do) and have the tests run on it.
Can you have a look at this?

[dmp42]

Ok, I only have some minor nitpicks on this (see inline), mainly:

• would love to have a functioning fixture script to start / stop zookeepers servers instances on demand with custom config
• would love to have travis install zookeeper and run the tests

Apart from that, I'm a bit worried about performance right now - domains.with.lots.of.subdomains will generate lots of queries to ZK. Any idea how fast it is at that?

[fanatic]

Apologies for the delay. I believe I've addressed your concerns to the best of my abilities. I'm forced to drop the configuration file as ZK expects it to exist on disk, and I clean up the data directory during 'stop'.

ZK performance has always been respectable in my experience. On our 5 node cluster, we regularly reach 500k op/s with various applications with latency under 1ms, but I'm happy to make improvements if you have some suggestions.

[dmp42]

Hi @fanatic

No problem for the delay - I was and am busy as well.

I like what you did quite a lot.

Can you rebase it on top of master?

[dmp42]

LGTM

[dmp42]

Would be happy to merge this once rebased / reviewed again (cc @willdurand)

[willdurand]

see: #186

1. test suite is all green
2. looks like a good adaptation of existing drivers, so I think it is ok...