> On Dec. 2, 2015, 10:24 p.m., Vinod Kone wrote:
> > I see a very basic test in the next review. Are you planning to write more 
> > comprehensive tests? Is that plan to templatize slave recovery tests for 
> > both pid based and http executors?

For now, I added just a basic test for testing the subscribe->subscribed 
workflow but I would surely be adding more tests around recovery. Currently, 
the existing Slave recovery tests use the `Executor/ExecutorDriver` to do all 
the heavy lifting. Until, have a HTTP based executor library it won't be that 
easy to templatize the slave recovery tests. Hence, for now, I would go ahead 
and add some basic tests in `src/tests/executor_http_apt_tests.cpp`. Does that 
sound reasonable to you ?


> On Dec. 2, 2015, 10:24 p.m., Vinod Kone wrote:
> > src/slave/slave.cpp, lines 2414-2415
> > <https://reviews.apache.org/r/38877/diff/10/?file=1148138#file1148138line2414>
> >
> >     I see that 'RECOVERING' is a possible state in `registerExecutor()`. Is 
> > that not possible for http executors?

Yes, the agent cannot be in `state == RECOVERING` for HTTP based executors 
since we already check it inside the `executor(...)` call back here:
https://github.com/apache/mesos/blob/master/src/slave/http.cpp#L218

Hence, it should be fine to fail the assert and crash here.


- Anand


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


On Dec. 2, 2015, 10:41 p.m., Anand Mazumdar wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38877/
> -----------------------------------------------------------
> 
> (Updated Dec. 2, 2015, 10:41 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Isabel Jimenez, and Vinod Kone.
> 
> 
> Bugs: MESOS-3515
>     https://issues.apache.org/jira/browse/MESOS-3515
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This change adds the functionality for executors to `Subscribe` via the 
> `api/v1/executor` endpoint. It also stores a marker file as part of the 
> `Subscribe` call if framework `checkpointing` is enabled. This can then be 
> used by the agent when recovering to wait for reconnecting back with the 
> executor.
> 
> 
> Diffs
> -----
> 
>   src/slave/http.cpp c3247f17e9faed32a46d3ab9ee83c399cd2c8d5e 
>   src/slave/slave.hpp 5ee133ae52998d05c8afabb2ade7095e363c75c5 
>   src/slave/slave.cpp 9055f2a789cb19f3579c15a379ea505dfef0578c 
> 
> Diff: https://reviews.apache.org/r/38877/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Reply via email to