I think that the code also needs: * comments. You need to put some java docs in that say what the different classes and methods are intended to do. External documentation is nice, but hardly sufficient.
* global handling of disconnection and pausing of masters during a disconnect. * a description of how you think you are handling error conditions * tests that demonstrate that you handle error conditions I would also take issue with Jordan's suggestion for retry logic. With ephemeral sequential node, retries are very dangerous in certain corner failure modes. This is the primary reason that I prefer the single file leader election method. With a reasonable number of masters (the most common case) this is completely sufficient since the herd effect isn't a problem for such a small herd. On Fri, Nov 25, 2011 at 10:17 PM, Jordan Zimmerman <jzimmer...@netflix.com>wrote: > A few comments: > > * NodeMonitor.createRootIfNotExists() should catch NodeExists. In the case > of multiple clients, this is a possibility. > > * I'd add a start method and create the ZooKeeper instance there. This > gives users a chance to set a listener so as to receive all messages. > > * All ZooKeeper operations should be in some kind of retry loop. The > client can lose connection to a given server but successfully reconnect to > another one in the cluster. > > * When creating the Znode, it can succeed on the server but fail to return > the result to the client. On a Disconnect/Session exception, you should > retry and then call getChildren and search for your node. > > -JZ > > On 11/25/11 2:45 AM, "Olivier Van Acker" <cyberroa...@gmail.com> wrote: > > >I've written a example app on how to do implement leadership election in > >with zookeeper > >Is there anyone on the list who'd like to review my app and if it needs > >improvement or not? > > > >the app is on github: > >https://github.com/cyberroadie/zookeeper-leader > > > >and explained how it works on my blog: > > > http://cyberroadie.wordpress.com/2011/11/24/implementing-leader-election-w > >ith-zookeeper/ > > > >Olivier > >
