You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Klaus Ma <kl...@gmail.com> on 2015/12/06 16:35:56 UTC

Re: Review Request 40759: [WIP] Command executor can overcommit the slave

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

(Updated Dec. 6, 2015, 11:35 p.m.)


Review request for mesos, Ben Mahler, Ian Downes, and Vinod Kone.


Changes
-------

Update the draft code diff to move `getExecutorInfo` from slave to master; but there's still some failed cases on slave's registry. Will discuss the solution for it in JIRA.


Bugs: MESOS-1718
    https://issues.apache.org/jira/browse/MESOS-1718


Repository: mesos


Description
-------

Currently we give a small amount of resources to the command executor, in addition to resources used by the command task: https://github.com/apache/mesos/blob/0.20.0-rc1/src/slave/slave.cpp#L2448

```
ExecutorInfo Slave::getExecutorInfo(
    const FrameworkID& frameworkId,
    const TaskInfo& task)
{
  ...
    // Add an allowance for the command executor. This does lead to a
    // small overcommit of resources.
    executor.mutable_resources()->MergeFrom(
        Resources::parse(
          "cpus:" + stringify(DEFAULT_EXECUTOR_CPUS) + ";" +
          "mem:" + stringify(DEFAULT_EXECUTOR_MEM.megabytes())).get());
  ...
}
```

This leads to an overcommit of the slave. Ideally, for command tasks we can "transfer" all of the task resources to the executor at the slave / isolation level.


Diffs (updated)
-----

  include/mesos/mesos.proto 3a10c8a 
  src/docker/executor.cpp 4042cec 
  src/launcher/executor.cpp f90ea01 
  src/master/constants.hpp cc38dfc 
  src/master/master.hpp 4683fa5 
  src/master/master.cpp 953fa4f 
  src/slave/constants.hpp bcbb140 
  src/slave/slave.hpp b7586ce 
  src/slave/slave.cpp 9bd86e1 
  src/tests/containerizer/docker_containerizer_tests.cpp 3f199e6 
  src/tests/master_tests.cpp 865fa4a 
  src/tests/monitor_tests.cpp a848d14 
  src/tests/slave_recovery_tests.cpp c0e4ff7 
  src/tests/slave_tests.cpp 4975bea 

Diff: https://reviews.apache.org/r/40759/diff/


Testing
-------

only `make` passwd for now.


Thanks,

Klaus Ma