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


Fix it, then Ship it!




LGTM.

Mind adding a TODO in the re-registration path?

I also relized that if the slave crashes while some task is still queued, we'll 
send TASK_LOST during reregistration, but we won't try to shutdown the executor 
in that case. I guess you'are working on it?


src/slave/slave.cpp (line 2971)
<https://reviews.apache.org/r/47381/#comment197527>

    Wondering if we should only call shutdown if 
`executor->isCommandExecutor()` is true?


- Jie Yu


On May 14, 2016, 12:28 a.m., Vinod Kone wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47381/
> -----------------------------------------------------------
> 
> (Updated May 14, 2016, 12:28 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-5380
>     https://issues.apache.org/jira/browse/MESOS-5380
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> The agent now shuts down the executor during registration if it does not
> have any queued tasks (e.g., framework sent a killTask before
> registration).
> 
> Note that if the executor doesn't register at all, it will be cleaned up
> anyway after the registration timeout value.
> 
> Also, note that this doesn't handle the case where the agent restarts
> after processing the killTask() but before cleaning up the executor.
> 
> 
> Diffs
> -----
> 
>   src/slave/slave.cpp 7c870396b4d6804bfda6169d76d136e95cd839f5 
>   src/tests/slave_tests.cpp 3ec670aa75790417ec8b7b96cfdb787492b104e1 
> 
> Diff: https://reviews.apache.org/r/47381/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> The new test failed w/o the code change.
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>

Reply via email to