> On April 15, 2013, 1:06 a.m., Ben Mahler wrote:
> > src/slave/slave.hpp, line 292
> > <https://reviews.apache.org/r/10233/diff/3/?file=281061#file281061line292>
> >
> >     Why this TODO if you've introduced the states in another change?

I used that to keep track.


> On April 15, 2013, 1:06 a.m., Ben Mahler wrote:
> > src/slave/slave.cpp, line 492
> > <https://reviews.apache.org/r/10233/diff/3/?file=281062#file281062line492>
> >
> >     Just curious whether you're planning to implement the fix in another 
> > review?

Not yet. I don't like the timeout solution to be honest. If this becomes a 
problem in production, I will spend more time on a proper solution. 


> On April 15, 2013, 1:06 a.m., Ben Mahler wrote:
> > src/slave/slave.cpp, line 520
> > <https://reviews.apache.org/r/10233/diff/3/?file=281062#file281062line520>
> >
> >     You'll need to remove this check. You can also kill the Future argument.

I fixed this in a downstream review.


> On April 15, 2013, 1:06 a.m., Ben Mahler wrote:
> > src/slave/slave.cpp, line 1715
> > <https://reviews.apache.org/r/10233/diff/3/?file=281062#file281062line1715>
> >
> >     What's the fix?

as you might have seen, this is fixed in a downstream review.


- Vinod


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


On April 15, 2013, 12:50 a.m., Vinod Kone wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10233/
> -----------------------------------------------------------
> 
> (Updated April 15, 2013, 12:50 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