----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/15706/#review29198 -----------------------------------------------------------
Looks good, I was looking at how retry() can be initiated from several locations in the code but it appears relatively harmless: we will just do redundant sync() calls on occasion AFAICT. src/zookeeper/group.cpp <https://reviews.apache.org/r/15706/#comment56341> Can these be on the same line now? src/zookeeper/group.cpp <https://reviews.apache.org/r/15706/#comment56340> Result<Nothing> seems a little strange, what is the difference between None() and Some(Nothing())? Can we make this Try<bool>, returning whether setup was completed? This looks similar to how sync() is used as well. - Ben Mahler On Nov. 20, 2013, 12:05 a.m., Jiang Yan Xu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/15706/ > ----------------------------------------------------------- > > (Updated Nov. 20, 2013, 12:05 a.m.) > > > Review request for mesos, Benjamin Hindman, Ben Mahler, Ian Downes, Jie Yu, > and Vinod Kone. > > > Bugs: MESOS-814 > https://issues.apache.org/jira/browse/MESOS-814 > > > Repository: mesos-git > > > Description > ------- > > See summary. > > > Diffs > ----- > > src/zookeeper/group.hpp 04068e357cec95457d1f24c166d0b60f86d997d2 > src/zookeeper/group.cpp 12c781b29f4300ca8a29660adc3f1e55e03d5d04 > > Diff: https://reviews.apache.org/r/15706/diff/ > > > Testing > ------- > > make check & mesos-tests.sh > --gtest_filter=GroupTest*:ZooKeeperTest*:ZooKeeperMasterContenderDetectorTest* > with high iterations. > > This fix is for a problem not easy to expose through unit tests so no new > tests were written. > > > Thanks, > > Jiang Yan Xu > >
