mesos-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Ben Mahler" <benjamin.mah...@gmail.com>
Subject Re: Review Request 15706: Fixed Group to retry when authentication failed due to retryable errors.
Date Tue, 03 Dec 2013 19:08:41 GMT

-----------------------------------------------------------
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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message