----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/15706/#review29615 -----------------------------------------------------------
src/zookeeper/group.cpp <https://reviews.apache.org/r/15706/#comment57028> Is printing the pid useful? s/)/) / s/to/ to/ src/zookeeper/group.cpp <https://reviews.apache.org/r/15706/#comment57019> GroupProcess::retry calls sync without ensuring we're in a CONNECTING state, is that ok? src/zookeeper/group.cpp <https://reviews.apache.org/r/15706/#comment57015> Ah, so now this is possible because we set retrying to false? Is there any harm to eliminating setting it to false in expired() and reconnecting()? Before retrying was used as a guard against duplicate retries, and only set false inside retry(), which is really easy to think about and allows you to have a CHECK(retrying) here, right? Now retrying is used as a cancellation mechanism, but an imperfect one because we could trigger a second retry (setting retrying=true) before the first retry executes and realizes retrying is false. In this case, the first one would win if successful. If unsuccessful, both will continue to sync() and backoff. src/zookeeper/group.cpp <https://reviews.apache.org/r/15706/#comment57018> Would it be more clear if sync() returned a Try to differentiate retry-able errors? This would be consistent with the rest of the code here and would avoid having us look at state to figure this out. Checking state != CONNECTING seems to be a very implicit way to check for non-recoverable error? - Ben Mahler On Dec. 2, 2013, 10:39 p.m., Jiang Yan Xu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/15706/ > ----------------------------------------------------------- > > (Updated Dec. 2, 2013, 10:39 p.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 > >
