> On June 2, 2014, 11:09 p.m., Ben Mahler wrote:
> > Looks much simpler, thanks!
> > 
> > It's a bit scary to have a Slave::signaled member function executed inside 
> > the signal handler, given that this means we can have more than one Slave 
> > member function executing concurrently. We try to avoid this in general. 
> > What if we lambda::bind 'self()' instead 'this'? That way, we could make 
> > Slave::signaled a static function!
> > 
> > Are you planning to follow up with tests in a separate review?

The test will be in a separate review.


> On June 2, 2014, 11:09 p.m., Ben Mahler wrote:
> > src/slave/slave.cpp, line 154
> > <https://reviews.apache.org/r/21379/diff/4/?file=600103#file600103line154>
> >
> >     We should probably use getpwuid_r to be thread safe.
> >     
> >     It might be cleaner to add an optional 'uid' argument to os::user and 
> > re-use that, but feel free to leave a TODO here for now.

I added an optional 'uid' because it definitely looks cleaner using os::user 
instead of getpwuid_r.


- Alexandra


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


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