----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/17440/#review32956 -----------------------------------------------------------
src/state/zookeeper.cpp <https://reviews.apache.org/r/17440/#comment62023> But we aren't connecting! The 'zk' handle is set to NULL on the line above. What was the problem with DISCONNECTED? That was used originally in GroupProcess as well. src/state/zookeeper.cpp <https://reviews.apache.org/r/17440/#comment62024> This couple of lines of code are hard to follow. Basically, I can get a 'connected' callback and be in the state CONNECTING, which makes me CONNECTED, but also be in lots of other states and do nothing? Can I be in READY and then get a 'connected' callback? Why don't I transition to CONNECTED if I'm in READY? We need to be much more explicit about what states we can be in here please. src/state/zookeeper.cpp <https://reviews.apache.org/r/17440/#comment62021> We should really capture the bad state due to failed authentication rather than keep us in the CONNECTED state and also add a new READY state. src/zookeeper/watcher.hpp <https://reviews.apache.org/r/17440/#comment62025> Please document why having shared state here doesn't work due to concurrency issues. - Benjamin Hindman On Jan. 28, 2014, 1:59 a.m., Jiang Yan Xu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/17440/ > ----------------------------------------------------------- > > (Updated Jan. 28, 2014, 1:59 a.m.) > > > Review request for mesos, Benjamin Hindman, Ben Mahler, and Vinod Kone. > > > Bugs: MESOS-937 > https://issues.apache.org/jira/browse/MESOS-937 > > > Repository: mesos-git > > > Description > ------- > > Description in JIRA: "Between the execution of > ProcessWatcher::~ProcessWatcher() and its base class destructor > Watcher::~Watcher(), the pure virtual method Watcher::process() can be > invoked by WatcherProcess::event()." > > By eliminating WatcherProcess this problem is resolved. > > > Diffs > ----- > > src/state/zookeeper.hpp d1d1fedf27987aeaf9fbdee678d3b3848d05620a > src/state/zookeeper.cpp 09b63d44e9349cab2d73659c939de3d8e96fbcc5 > src/zookeeper/group.hpp e51ebb2cf5f09a633462c101f913ee8272be9a6c > src/zookeeper/group.cpp ecb6c002e8194b8d67e262826d988f747414f9f3 > src/zookeeper/watcher.hpp 1db0386719c2a675d29b47b417dc856993062326 > src/zookeeper/zookeeper.hpp f50aca6e7035c8084c3e76fd56b9d1ef7f9d9902 > src/zookeeper/zookeeper.cpp 5720f4c1cd51c4d998b52e7bf7f9f019ae80c5f8 > > Diff: https://reviews.apache.org/r/17440/diff/ > > > Testing > ------- > > make check. > Jenkins tests ongoing, will report the results. > > > Thanks, > > Jiang Yan Xu > >
