----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/15706/#review29666 -----------------------------------------------------------
Ship it! Looks good to me! Just a few cleanup bits below. src/zookeeper/group.cpp <https://reviews.apache.org/r/15706/#comment57093> Shouldn't we be using the cache return value instead like you did below? } else if (!cached.get()) { This is the explicit indication of retry and we have a nice CHECK_SOME(memberships) below. src/zookeeper/group.cpp <https://reviews.apache.org/r/15706/#comment57094> I think you want this comment above the sync() call. src/zookeeper/group.cpp <https://reviews.apache.org/r/15706/#comment57095> Maybe you'll want this CHECK above where you call cache as well? src/zookeeper/group.cpp <https://reviews.apache.org/r/15706/#comment57096> This one can likely stay as is since count on a map is only ever 1 or 0, we're essentially just doing a contains() operation. (We don't have a hashmap so we have to use count() instead). - Ben Mahler On Dec. 3, 2013, 5:01 a.m., Jiang Yan Xu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/15706/ > ----------------------------------------------------------- > > (Updated Dec. 3, 2013, 5:01 a.m.) > > > Review request for mesos, Benjamin Hindman, Ben Mahler, and Vinod Kone. > > > Bugs: MESOS-814 > https://issues.apache.org/jira/browse/MESOS-814 > > > Repository: mesos-git > > > Description > ------- > > See summary. > > > Diffs > ----- > > src/tests/group_tests.cpp fb4c2f103ab7436b0fa2d6276c1ec1011a9405f0 > src/zookeeper/group.hpp 04068e357cec95457d1f24c166d0b60f86d997d2 > src/zookeeper/group.cpp 5218286bb6ab989ba16cead1483851cc8adba5ca > > Diff: https://reviews.apache.org/r/15706/diff/ > > > Testing > ------- > > make check & mesos-tests.sh > --gtest_filter=GroupTest*:ZooKeeperTest*:ZooKeeperMasterContenderDetectorTest* > with 100 iterations. > > Added a test to exercise authenticate() and create() retries. > > > Thanks, > > Jiang Yan Xu > >
