> On April 17, 2014, 4:56 p.m., Ben Mahler wrote:
> > src/zookeeper/group.cpp, lines 884-886
> > <https://reviews.apache.org/r/20470/diff/2/?file=561887#file561887line884>
> >
> >     Another question here:
> >     
> >     This says we can be in CONNECTING, and then we immediately call sync(). 
> > sync() will fail a CHECK if in state CONNECTING?

It's guarded by 'retrying' but yeah this statement can be confusing.

I guess I can move it up to the top of the method:

  CHECK(state == CONNECTING || state == CONNECTED || state == AUTHENTICATED ||
      state == READY) << state;
  if (!retrying) {
    // Retry could be cancelled before it is scheduled.
    return;
  }

Or leave it here but just have 
    CHECK(state == CONNECTED || state == AUTHENTICATED || state == READY) << 
state;

The latter looks better. 


But then immediately inside sync() we do the same CHECK again...

I guess checking it twice won't hurt and sync() is called also by connected(). 
I'll just leave the CHECK statement in both places.


- Jiang Yan


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


On April 17, 2014, 4:53 p.m., Jiang Yan Xu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20470/
> -----------------------------------------------------------
> 
> (Updated April 17, 2014, 4:53 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-1214
>     https://issues.apache.org/jira/browse/MESOS-1214
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> - Retry can be invoked when the Group is in any state but DISCONNECTED.
> 
> 
> Diffs
> -----
> 
>   src/zookeeper/group.cpp cff59976259499ea03db8f35cb2b40674b5e4b53 
> 
> Diff: https://reviews.apache.org/r/20470/diff/
> 
> 
> Testing
> -------
> 
> make check.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>

Reply via email to