You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Ian Downes <ia...@gmail.com> on 2014/06/05 01:28:05 UTC
Review Request 22251: Give the command executor some cpu and memory.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22251/
-----------------------------------------------------------
Review request for mesos, Ben Mahler and Vinod Kone.
Bugs: MESOS-1417
https://issues.apache.org/jira/browse/MESOS-1417
Repository: mesos-git
Description
-------
Give the command executor some cpu and memory.
This is necessary because after a terminal task status update the slave updates the container's resources, which would be zero if the executor didn't have its own resources.
This does lead to a small overcommit but the command executor is only (mainly?) used for tests.
Diffs
-----
src/slave/slave.cpp 643c0882a4bab1b612b3fb6fd1004e09edf5f368
Diff: https://reviews.apache.org/r/22251/diff/
Testing
-------
make check
Thanks,
Ian Downes
Re: Review Request 22251: Give the command executor some cpu and memory.
Posted by Ian Downes <ia...@gmail.com>.
> On June 4, 2014, 4:37 p.m., Vinod Kone wrote:
> > src/slave/slave.cpp, line 2304
> > <https://reviews.apache.org/r/22251/diff/1/?file=603699#file603699line2304>
> >
> > pull these out into constants.
> >
> > DEFAULT_EXECUTOR_CPUS
> > DEFAULT_EXECUTOR_MEM
> >
> > Also these values seem to be too high? what is the minimum value allowed by cgroups?
Technically, zero is valid for both but it needs to be something sensible to avoid starving or OOMing the executor. I looked at a command executor running in a memory cgroup and its memory usage was ~ 4 MB.
The numbers don't really mean much because the slave doesn't account for them; they are upper bounds to ensure a borked executor doesn't overtake the host.
- Ian
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22251/#review44776
-----------------------------------------------------------
On June 4, 2014, 4:36 p.m., Ian Downes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22251/
> -----------------------------------------------------------
>
> (Updated June 4, 2014, 4:36 p.m.)
>
>
> Review request for mesos, Ben Mahler, Jie Yu, and Vinod Kone.
>
>
> Bugs: MESOS-1417
> https://issues.apache.org/jira/browse/MESOS-1417
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Give the command executor some cpu and memory.
>
> This is necessary because after a terminal task status update the slave updates the container's resources, which would be zero if the executor didn't have its own resources.
>
> This does lead to a small overcommit but the command executor is only (mainly?) used for tests.
>
>
> Diffs
> -----
>
> src/slave/slave.cpp 643c0882a4bab1b612b3fb6fd1004e09edf5f368
>
> Diff: https://reviews.apache.org/r/22251/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Ian Downes
>
>
Re: Review Request 22251: Give the command executor some cpu and memory.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22251/#review44776
-----------------------------------------------------------
src/slave/slave.cpp
<https://reviews.apache.org/r/22251/#comment79315>
command executor is actually used by some real frameworks (e.g., jenkins on mesos).
src/slave/slave.cpp
<https://reviews.apache.org/r/22251/#comment79316>
pull these out into constants.
DEFAULT_EXECUTOR_CPUS
DEFAULT_EXECUTOR_MEM
Also these values seem to be too high? what is the minimum value allowed by cgroups?
- Vinod Kone
On June 4, 2014, 11:36 p.m., Ian Downes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22251/
> -----------------------------------------------------------
>
> (Updated June 4, 2014, 11:36 p.m.)
>
>
> Review request for mesos, Ben Mahler, Jie Yu, and Vinod Kone.
>
>
> Bugs: MESOS-1417
> https://issues.apache.org/jira/browse/MESOS-1417
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Give the command executor some cpu and memory.
>
> This is necessary because after a terminal task status update the slave updates the container's resources, which would be zero if the executor didn't have its own resources.
>
> This does lead to a small overcommit but the command executor is only (mainly?) used for tests.
>
>
> Diffs
> -----
>
> src/slave/slave.cpp 643c0882a4bab1b612b3fb6fd1004e09edf5f368
>
> Diff: https://reviews.apache.org/r/22251/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Ian Downes
>
>
Re: Review Request 22251: Give the command executor some cpu and memory.
Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22251/#review45329
-----------------------------------------------------------
Patch looks great!
Reviews applied: [22251]
All tests passed.
- Mesos ReviewBot
On June 10, 2014, 5:57 p.m., Ian Downes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22251/
> -----------------------------------------------------------
>
> (Updated June 10, 2014, 5:57 p.m.)
>
>
> Review request for mesos, Ben Mahler, Jie Yu, and Vinod Kone.
>
>
> Bugs: MESOS-1417
> https://issues.apache.org/jira/browse/MESOS-1417
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Give the command executor some cpu and memory.
>
> This is necessary because after a terminal task status update the slave updates the container's resources, which would be zero if the executor didn't have its own resources.
>
> This does lead to a small overcommit.
>
>
> Diffs
> -----
>
> src/slave/constants.hpp ace459071d7b1906fe8665878a60ae9edf7d0022
> src/slave/slave.cpp 643c0882a4bab1b612b3fb6fd1004e09edf5f368
>
> Diff: https://reviews.apache.org/r/22251/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Ian Downes
>
>
Re: Review Request 22251: Give the command executor some cpu and memory.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22251/#review45391
-----------------------------------------------------------
Ship it!
Ship It!
- Jie Yu
On June 10, 2014, 5:57 p.m., Ian Downes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22251/
> -----------------------------------------------------------
>
> (Updated June 10, 2014, 5:57 p.m.)
>
>
> Review request for mesos, Ben Mahler, Jie Yu, and Vinod Kone.
>
>
> Bugs: MESOS-1417
> https://issues.apache.org/jira/browse/MESOS-1417
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Give the command executor some cpu and memory.
>
> This is necessary because after a terminal task status update the slave updates the container's resources, which would be zero if the executor didn't have its own resources.
>
> This does lead to a small overcommit.
>
>
> Diffs
> -----
>
> src/slave/constants.hpp ace459071d7b1906fe8665878a60ae9edf7d0022
> src/slave/slave.cpp 643c0882a4bab1b612b3fb6fd1004e09edf5f368
>
> Diff: https://reviews.apache.org/r/22251/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Ian Downes
>
>
Re: Review Request 22251: Give the command executor some cpu and memory.
Posted by Ian Downes <ia...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22251/
-----------------------------------------------------------
(Updated June 10, 2014, 10:57 a.m.)
Review request for mesos, Ben Mahler, Jie Yu, and Vinod Kone.
Bugs: MESOS-1417
https://issues.apache.org/jira/browse/MESOS-1417
Repository: mesos-git
Description (updated)
-------
Give the command executor some cpu and memory.
This is necessary because after a terminal task status update the slave updates the container's resources, which would be zero if the executor didn't have its own resources.
This does lead to a small overcommit.
Diffs
-----
src/slave/constants.hpp ace459071d7b1906fe8665878a60ae9edf7d0022
src/slave/slave.cpp 643c0882a4bab1b612b3fb6fd1004e09edf5f368
Diff: https://reviews.apache.org/r/22251/diff/
Testing
-------
make check
Thanks,
Ian Downes
Re: Review Request 22251: Give the command executor some cpu and memory.
Posted by Ian Downes <ia...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22251/
-----------------------------------------------------------
(Updated June 10, 2014, 10:56 a.m.)
Review request for mesos, Ben Mahler, Jie Yu, and Vinod Kone.
Changes
-------
Moved constants.
Bugs: MESOS-1417
https://issues.apache.org/jira/browse/MESOS-1417
Repository: mesos-git
Description
-------
Give the command executor some cpu and memory.
This is necessary because after a terminal task status update the slave updates the container's resources, which would be zero if the executor didn't have its own resources.
This does lead to a small overcommit but the command executor is only (mainly?) used for tests.
Diffs (updated)
-----
src/slave/constants.hpp ace459071d7b1906fe8665878a60ae9edf7d0022
src/slave/slave.cpp 643c0882a4bab1b612b3fb6fd1004e09edf5f368
Diff: https://reviews.apache.org/r/22251/diff/
Testing
-------
make check
Thanks,
Ian Downes
Re: Review Request 22251: Give the command executor some cpu and memory.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22251/#review44829
-----------------------------------------------------------
src/slave/slave.hpp
<https://reviews.apache.org/r/22251/#comment79384>
Move it to src/slave/constants.hpp?
- Jie Yu
On June 5, 2014, 12:15 a.m., Ian Downes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22251/
> -----------------------------------------------------------
>
> (Updated June 5, 2014, 12:15 a.m.)
>
>
> Review request for mesos, Ben Mahler, Jie Yu, and Vinod Kone.
>
>
> Bugs: MESOS-1417
> https://issues.apache.org/jira/browse/MESOS-1417
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Give the command executor some cpu and memory.
>
> This is necessary because after a terminal task status update the slave updates the container's resources, which would be zero if the executor didn't have its own resources.
>
> This does lead to a small overcommit but the command executor is only (mainly?) used for tests.
>
>
> Diffs
> -----
>
> src/slave/slave.hpp 34687e555e6ba07863c45840aa6d07717388cf62
> src/slave/slave.cpp 643c0882a4bab1b612b3fb6fd1004e09edf5f368
>
> Diff: https://reviews.apache.org/r/22251/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Ian Downes
>
>
Re: Review Request 22251: Give the command executor some cpu and memory.
Posted by Ian Downes <ia...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22251/
-----------------------------------------------------------
(Updated June 4, 2014, 5:15 p.m.)
Review request for mesos, Ben Mahler, Jie Yu, and Vinod Kone.
Changes
-------
Addressed Vinod's comments.
Bugs: MESOS-1417
https://issues.apache.org/jira/browse/MESOS-1417
Repository: mesos-git
Description
-------
Give the command executor some cpu and memory.
This is necessary because after a terminal task status update the slave updates the container's resources, which would be zero if the executor didn't have its own resources.
This does lead to a small overcommit but the command executor is only (mainly?) used for tests.
Diffs (updated)
-----
src/slave/slave.hpp 34687e555e6ba07863c45840aa6d07717388cf62
src/slave/slave.cpp 643c0882a4bab1b612b3fb6fd1004e09edf5f368
Diff: https://reviews.apache.org/r/22251/diff/
Testing
-------
make check
Thanks,
Ian Downes
Re: Review Request 22251: Give the command executor some cpu and memory.
Posted by Ian Downes <ia...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22251/
-----------------------------------------------------------
(Updated June 4, 2014, 4:36 p.m.)
Review request for mesos, Ben Mahler, Jie Yu, and Vinod Kone.
Changes
-------
Add Jie
Bugs: MESOS-1417
https://issues.apache.org/jira/browse/MESOS-1417
Repository: mesos-git
Description
-------
Give the command executor some cpu and memory.
This is necessary because after a terminal task status update the slave updates the container's resources, which would be zero if the executor didn't have its own resources.
This does lead to a small overcommit but the command executor is only (mainly?) used for tests.
Diffs
-----
src/slave/slave.cpp 643c0882a4bab1b612b3fb6fd1004e09edf5f368
Diff: https://reviews.apache.org/r/22251/diff/
Testing
-------
make check
Thanks,
Ian Downes