----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/2669/#review3209 -----------------------------------------------------------
src/slave/slave.cpp <https://reviews.apache.org/r/2669/#comment7128> s/start/start. src/slave/slave.cpp <https://reviews.apache.org/r/2669/#comment7126> Let's keep this down by the actual RunTaskMessage for consistency with the other RunTaskMessage usage. src/slave/slave.cpp <https://reviews.apache.org/r/2669/#comment7130> This is great. But three things: (1) let's tweak this comment to include the fact that we are changing the resources _after_ we have accounted for the tasks; (2) possibly include a TODO or NOTE that says that given the asynchronous nature of the dispatch this still might not be sufficient for guaranteeing that the resource limits have been changed before the executor starts running the task; and (3) also change the other place in the code where we send a RunTaskMessage to a slave to dispatch to the isolation module before we send the message. src/tests/master_tests.cpp <https://reviews.apache.org/r/2669/#comment7132> Unless I'm mistaken, because of the async nature of the isolation module there is no happens-before relationship in the code that guarantees that the isolation module will have gotten a "resourcesChanged" before the slave gets a status update from the executor that it sends back to the scheduler. I'd rather you add a TODO in the code to try and capture the fact that resources have been updated then doing it this way for now. - Benjamin On 2011-11-02 05:51:36, Charles Reiss wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/2669/ > ----------------------------------------------------------- > > (Updated 2011-11-02 05:51:36) > > > Review request for mesos. > > > Summary > ------- > > This calls Executor::addTask to accumulate the usage of queued tasks before > calling resourcesChanged() to ensure the executor has enough resources to run > those queued tasks. > > > This addresses bug MESOS-56. > https://issues.apache.org/jira/browse/MESOS-56 > > > Diffs > ----- > > src/slave/slave.cpp de5716c > src/tests/master_tests.cpp 2438114 > src/tests/utils.hpp 02772f7 > > Diff: https://reviews.apache.org/r/2669/diff > > > Testing > ------- > > Modification to MasterTest included. > > > Thanks, > > Charles > >
