> On Feb. 17, 2015, 7:09 p.m., Vinod Kone wrote: > > src/slave/slave.cpp, line 1695 > > <https://reviews.apache.org/r/31024/diff/1/?file=863796#file863796line1695> > > > > Not yours, but can you add a comment here: > > > > // Send terminal update which removes the task from 'queuedTasks'.
I think there is a NOTE above already. I'll move it down to here. > On Feb. 17, 2015, 7:09 p.m., Vinod Kone wrote: > > src/slave/slave.cpp, lines 1738-1747 > > <https://reviews.apache.org/r/31024/diff/1/?file=863796#file863796line1738> > > > > Hmmm. I don't think we want to do this. If an executor has been running > > for a while, and all its launched tasks have terminated, we want it to keep > > running and wait for more tasks. With the new semantics, it's possible that > > we kill the executor if a kill task comes for a queued task. > > > > We did this optimization for unregistered executor as a stop gap > > because there was no shutdown executor api call. Per our offline discussion, this is a stop gap for those single task executors that do not have a proper logic for self terminating if no task is received for an extended period of time. I added a TODO here to remove this logic because looks like a bug of the executor. > On Feb. 17, 2015, 7:09 p.m., Vinod Kone wrote: > > src/slave/slave.cpp, lines 2441-2451 > > <https://reviews.apache.org/r/31024/diff/1/?file=863796#file863796line2441> > > > > Shouldn't the monitoring be done after the containerizer update > > completes? I'll leave a TODO here. If 'update' fails, the container will be terminated and thus stopping the monitor. - Jie ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31024/#review72747 ----------------------------------------------------------- On Feb. 13, 2015, 10:43 p.m., Jie Yu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/31024/ > ----------------------------------------------------------- > > (Updated Feb. 13, 2015, 10:43 p.m.) > > > Review request for mesos, Ian Downes and Vinod Kone. > > > Bugs: MESOS-998 > https://issues.apache.org/jira/browse/MESOS-998 > > > Repository: mesos > > > Description > ------- > > Changed the slave logic so that it always waits for containerizer update to > finish. > > The key idea is to leverage `executor->queuedTasks` when > containerizer->update is in progress. > > > Diffs > ----- > > include/mesos/type_utils.hpp 32dc6ed489f43ba3695507f59c4c2d94028c8df1 > src/slave/slave.hpp 7a399f6df50c69b7e1e12d74f076fa57b6edb1b3 > src/slave/slave.cpp f39a876cdd6b580a7a75fd053e6923761bee7635 > src/tests/master_tests.cpp c678527942506ef23fa4f1aebf9664e4cc21b561 > src/tests/registrar_zookeeper_tests.cpp > 30d3cd8b1e7d56fbb326c9feccd4c3cbae2015de > src/tests/scheduler_tests.cpp 080ec7236246794de6c8cc7356a06331c5e48cd5 > src/tests/slave_tests.cpp a02e335576bf68b449a6286fa5cf5093b1b7182a > > Diff: https://reviews.apache.org/r/31024/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Jie Yu > >
