-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15706/#review29414
-----------------------------------------------------------



src/tests/group_tests.cpp
<https://reviews.apache.org/r/15706/#comment56645>

    Is this guaranteed to cause retries?



src/zookeeper/group.cpp
<https://reviews.apache.org/r/15706/#comment56646>

    Can you CHECK that state is ready here before you call sync()?
    
    Since this ignores the result of sync(), the assumption here is that if 
sync returns false, we must be imminently getting a callback from ZK, does this 
assumption hold? Can you add a note?



src/zookeeper/group.cpp
<https://reviews.apache.org/r/15706/#comment56641>

    Can you add braces?
    
    if (...) {
      return true;
    }
    
    Should this instead be a CHECK that we're in CONNECTING?



src/zookeeper/group.cpp
<https://reviews.apache.org/r/15706/#comment56642>

    Ditto about adding braces.
    
    What if we're in connecting? Should this instead be a CHECK that we're in 
AUTHENTICATED?



src/zookeeper/group.cpp
<https://reviews.apache.org/r/15706/#comment56647>

    We may want CHECK_EQ for these kinds of CHECKs, so that a failure prints 
the actual state.



src/zookeeper/group.cpp
<https://reviews.apache.org/r/15706/#comment56643>

    "if needed" or "if not already authenticated"?



src/zookeeper/group.cpp
<https://reviews.apache.org/r/15706/#comment56644>

    if not already created?


- Ben Mahler


On Nov. 25, 2013, 7:59 p.m., Jiang Yan Xu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15706/
> -----------------------------------------------------------
> 
> (Updated Nov. 25, 2013, 7:59 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 12c781b29f4300ca8a29660adc3f1e55e03d5d04 
> 
> Diff: https://reviews.apache.org/r/15706/diff/
> 
> 
> Testing
> -------
> 
> make check & mesos-tests.sh 
> --gtest_filter=GroupTest*:ZooKeeperTest*:ZooKeeperMasterContenderDetectorTest*
>  with high iterations.
> 
> Added a test to exercise authenticate() and create() retries.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>

Reply via email to