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