> On March 11, 2016, 10:44 p.m., Vinod Kone wrote:
> > src/executor/executor.cpp, line 462
> > <https://reviews.apache.org/r/44580/diff/1/?file=1293349#file1293349line462>
> >
> >     This if statement sounds like all combinations of "checkpoint", 
> > "connecting" and "recoveryTimeout" are possible? I don't think that's the 
> > case?
> >     
> >     Should this be?
> >     
> >     ```
> >     if (recoveryTimer.isSome()) {
> >       CHECK(checkpoint);
> >       CHECK_EQ(CONNECTING, state);
> >       
> >       return;
> >     }
> >     ```

Good catch. Looks much simpler this way.

Since the state changes inside `disconnected()`, I removed the 
`CHECK_EQ(CONNECTING, state)` check.


> On March 11, 2016, 10:44 p.m., Vinod Kone wrote:
> > src/executor/executor.cpp, line 527
> > <https://reviews.apache.org/r/44580/diff/1/?file=1293349#file1293349line527>
> >
> >     kill this?

I moved it to doing the `CHECK` right before we do a `get()` for 
`recoveryTimeout`.


- Anand


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


On March 9, 2016, 6:03 p.m., Anand Mazumdar wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44580/
> -----------------------------------------------------------
> 
> (Updated March 9, 2016, 6:03 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-4858
>     https://issues.apache.org/jira/browse/MESOS-4858
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This change makes the following modifications to the library:
> 
> - Removes passing connection objects to `defer` callbacks as it can
> sometimes lead to deadlocks around destruction in the same execution
> context.
> - Introduced 3 additional states `CONNECTING`, `SUBSCRIBING` and
> `SUBSCRIBED`. The `CONNECTING` state helps us in identifying if a
> connection attempt is in progress while the latter two states allows
> us to drop subscribe calls if one is already is in progress.
> - Creates a random `connectionID` to demarcate a new connection
> instance and  allowing to discard a state connection attempt.
> - Changes around setting the recovery timeout timer only once.
> This allows us to later discard the recoveryTimeout callback
> if we connected with the agent at a later point of time.
> 
> 
> Diffs
> -----
> 
>   src/executor/executor.cpp c3e95ea7e4edf78f2a65ddc15e213aba66e69db2 
> 
> Diff: https://reviews.apache.org/r/44580/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Reply via email to