----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21379/#review44563 -----------------------------------------------------------
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? src/slave/slave.cpp <https://reviews.apache.org/r/21379/#comment78980> 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. src/slave/slave.cpp <https://reviews.apache.org/r/21379/#comment78984> Do you need the 'string()' here? src/slave/slave.cpp <https://reviews.apache.org/r/21379/#comment78981> Let's add the error string here: EXIT(1) << "Failed to set sigaction: " << sterror(errno); - Ben Mahler On May 30, 2014, 9:39 p.m., Alexandra Sava wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/21379/ > ----------------------------------------------------------- > > (Updated May 30, 2014, 9:39 p.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 > ----- > > src/slave/slave.hpp 34687e555e6ba07863c45840aa6d07717388cf62 > src/slave/slave.cpp c5c05132874b581829dfd191b8a553971fb8f3df > > Diff: https://reviews.apache.org/r/21379/diff/ > > > Testing > ------- > > > Thanks, > > Alexandra Sava > >
