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


Just a small amount of cleanup and we should be all set! Please send your test 
change out for review so I can help debug it. :)


3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp
<https://reviews.apache.org/r/21379/#comment80310>

    We shouldn't have this LOG(FATAL) when the user cannot be found, it will 
mean the slave crashes before it had a chance to shutdown, which can render the 
shutdown ineffective.
    
    Let's remove the change here and just leave a TODO to follow up with a 
separate change to print the user that signaled the slave.



src/slave/slave.cpp
<https://reviews.apache.org/r/21379/#comment80312>

    I think we can clean this up a bit further:
    
    (1) We can remove signaledWrapper, it's a global variable and it's a bit 
hard to understand it's lifecycle and how it behaves when multiple slaves are 
running.
    
    (2) 'callWrapper' can just be called 'handler'. Instead of invoking a 
function object it can just 'dispatch' to Slave::signaled.
    
    (3) Per (2), Slave::signaled can be a non-static member function that 
directly calls shutdown(). As it stands, we're doing a LOG(WARNING) inside the 
signal handler, which is not safe!


- Ben Mahler


On June 8, 2014, 10:42 a.m., Alexandra Sava wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21379/
> -----------------------------------------------------------
> 
> (Updated June 8, 2014, 10:42 a.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-544
>     https://issues.apache.org/jira/browse/MESOS-544
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> First phase: Mesos-slave support for "node drain" 
>                                                                               
>                   
> This phase handles the shutdown of the slave, triggered by SIGUSR1 signal:
> * implement a signal handler which is invoked when SIGUSR1 signal is issued
> * the signal handler will dispatch the 'shutdown' method of the Slave class, 
> which  shuts down all the frameworks and executors that run on the slave.
> 
> In the next phase, the slave will send an unregistration request to the 
> master in order to overcome the lag of the health check timer (75 sec).  
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 
> 72f3e70ccc189d0a7dd613ccaf79537d3587c49c 
>   src/slave/slave.hpp 34687e555e6ba07863c45840aa6d07717388cf62 
>   src/slave/slave.cpp 643c0882a4bab1b612b3fb6fd1004e09edf5f368 
> 
> Diff: https://reviews.apache.org/r/21379/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Alexandra Sava
> 
>

Reply via email to