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

Ship it!



src/slave/slave.cpp
<https://reviews.apache.org/r/10233/#comment39940>

    move this above and place the loop in the else statement



src/slave/slave.cpp
<https://reviews.apache.org/r/10233/#comment39936>

    Can you move this into an else on the TERMINATED check?



src/slave/slave.cpp
<https://reviews.apache.org/r/10233/#comment39937>

    Can you move this into an else on the TERMINATED check?
    
    shutdownExecutor should enforce its expected states, and then callers must 
only call it accordingly.



src/slave/slave.cpp
<https://reviews.apache.org/r/10233/#comment39941>

    Can you add a TODO to kill this, which means pushing Futures through and 
using collect() to terminate the slave after it's all finished.



src/tests/slave_recovery_tests.cpp
<https://reviews.apache.org/r/10233/#comment39942>

    // TODO base this on the time variable of the reaper?


- Ben Mahler


On April 16, 2013, 12:39 a.m., Vinod Kone wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10233/
> -----------------------------------------------------------
> 
> (Updated April 16, 2013, 12:39 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Ben Mahler.
> 
> 
> Description
> -------
> 
> See summary.
> 
> The main crux is two fold
> 
> 1) Shutdown a slave after all executors have terminated
> 2) Delete the "latest" symlink under /path/to/meta/slaves/ to make sure a 
> shutdown slave comes up as new slave when restarted.
> 
> I had to refactor cleanup(), because turns out there are quite a few edge 
> cases to guard against.
> 
> 
> Diffs
> -----
> 
>   src/slave/process_isolator.cpp 210ea10ad97e08c7a303249da97e70b438dfe11d 
>   src/slave/slave.hpp 2529bf500a3265b10ad4cddde10c2d62a6cdb4a0 
>   src/slave/slave.cpp 325231458a6883019436e7cc5a37f85f0f5735fa 
>   src/slave/state.cpp e5c32257978d8407535e05ed73f8a50bdc2f651d 
>   src/tests/slave_recovery_tests.cpp d0ff9b73e06e89a5409f038be2766333e0a0689e 
> 
> Diff: https://reviews.apache.org/r/10233/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> sudo GLOG_v=1 ./bin/mesos-tests.sh --gtest_filter="*ShutdownSlave*" --verbose 
> --gtest_repeat=100 --gtest_break_on_failure
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>

Reply via email to