> On Jan. 25, 2016, 9:58 p.m., Vinod Kone wrote: > > src/executor/executor.cpp, line 361 > > <https://reviews.apache.org/r/41283/diff/9/?file=1205244#file1205244line361> > > > > dont you want to set `subscribe` and `nonSubscribe` to None()?
As per our discussion, we don't set them to `None()` since we need them later when `_recoveryTimeout` is invoked. > On Jan. 25, 2016, 9:58 p.m., Vinod Kone wrote: > > src/executor/executor.cpp, line 368 > > <https://reviews.apache.org/r/41283/diff/9/?file=1205244#file1205244line368> > > > > Why is this outside the `if (state == CONNECTED)` block? I'm guessing > > you don't want to fire multiple delays one for subscribe disconnection and > > nonSubscribe disconnection for example? > > > > If yes, you might just want to do the below at the top of this method. > > > > ``` > > if (state == DISCONNECTED) { > > return; > > } > > ``` > > > > and merge this if block with the one at #352. Fixed. Previously, we used to fire 2 retries for both the subscribe/non-subscribe connections. Thanks for noticing it. As per our discussion, now we retry inside the `backoff` function instead of `disconnected` only once per disconnection. > On Jan. 25, 2016, 9:58 p.m., Vinod Kone wrote: > > src/executor/executor.cpp, line 299 > > <https://reviews.apache.org/r/41283/diff/9/?file=1205244#file1205244line299> > > > > Maybe mention when state transitions from/to CONNECTED/DISCONNECTED? We mention them when we define the `enum`. Let me know if that is not enough. - Anand ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41283/#review116179 ----------------------------------------------------------- On Jan. 22, 2016, 1:36 a.m., Anand Mazumdar wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/41283/ > ----------------------------------------------------------- > > (Updated Jan. 22, 2016, 1:36 a.m.) > > > Review request for mesos, Ben Mahler and Vinod Kone. > > > Bugs: MESOS-3550 > https://issues.apache.org/jira/browse/MESOS-3550 > > > Repository: mesos > > > Description > ------- > > This change introduces the implementation for the executor library. > > This uses the new HTTP Connection interface to ensure calls are properly > pipelined. > > `connected` -> Callback invoked when a TCP connection is established with the > agent for the `Subscribe` call. > `disconnected` -> When the `Subscribe` TCP connection is interrupted possibly > due to an agent restart. > `received` -> When the executor receives events from the agent upon > subscribing. > > > Diffs > ----- > > src/Makefile.am d23e35001078a86775bd9b76baa207ecb9dab7e1 > src/executor/executor.cpp PRE-CREATION > > Diff: https://reviews.apache.org/r/41283/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Anand Mazumdar > >
