----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31024/#review72747 -----------------------------------------------------------
src/slave/slave.hpp <https://reviews.apache.org/r/31024/#comment118807> s/has/have/ src/slave/slave.hpp <https://reviews.apache.org/r/31024/#comment118809> should this be a public function? src/slave/slave.cpp <https://reviews.apache.org/r/31024/#comment118813> How about: // Queue task until the containerizer is updated with new resource limits. src/slave/slave.cpp <https://reviews.apache.org/r/31024/#comment118815> s/limits/resources/ src/slave/slave.cpp <https://reviews.apache.org/r/31024/#comment118823> Not yours, but can you add a comment here: // Send terminal update which removes the task from 'queuedTasks'. src/slave/slave.cpp <https://reviews.apache.org/r/31024/#comment118824> s/cases/case/ src/slave/slave.cpp <https://reviews.apache.org/r/31024/#comment118827> 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. src/slave/slave.cpp <https://reviews.apache.org/r/31024/#comment118828> s/task/tasks/ src/slave/slave.cpp <https://reviews.apache.org/r/31024/#comment118829> s/limits/resources/ src/slave/slave.cpp <https://reviews.apache.org/r/31024/#comment118830> Shouldn't the monitoring be done after the containerizer update completes? src/tests/slave_tests.cpp <https://reviews.apache.org/r/31024/#comment118834> Use LaucnhTasks action? src/tests/slave_tests.cpp <https://reviews.apache.org/r/31024/#comment118835> Can you also add/update tests to test the correct case; container resources are updated before task reaches executor? - Vinod Kone 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 > >
