You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Patrick Reilly <pa...@gmail.com> on 2014/08/27 19:58:46 UTC
Review Request 25105: Explore disk io isolation in cgroups
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25105/
-----------------------------------------------------------
Review request for mesos, Adam B and Benjamin Hindman.
Repository: mesos-git
Description
-------
Currently there is no disk isolation in place and this affects an executor to be starved of disk when another disk heavy operation such as copying a multi gigabyte file is being performed by another executor.
Diffs
-----
include/mesos/mesos.proto dea51f9
src/Makefile.am 40b9f6b
src/slave/containerizer/isolators/cgroups/blkio.hpp PRE-CREATION
src/slave/containerizer/isolators/cgroups/blkio.cpp PRE-CREATION
src/slave/containerizer/mesos/containerizer.cpp 5116b14
Diff: https://reviews.apache.org/r/25105/diff/
Testing
-------
In progress.
Thanks,
Patrick Reilly
Re: Review Request 25105: Explore disk io isolation in cgroups
Posted by Ian Downes <ia...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25105/#review51678
-----------------------------------------------------------
Please close issues once you've addressed them.
src/slave/containerizer/isolators/cgroups/blkio.cpp
<https://reviews.apache.org/r/25105/#comment90201>
Correct alphabetical ordering.
src/slave/containerizer/isolators/cgroups/blkio.cpp
<https://reviews.apache.org/r/25105/#comment90203>
This shouldn't be a check. Just return a Failure("Pid ... for container ... has already been isolated")
src/slave/containerizer/isolators/cgroups/blkio.cpp
<https://reviews.apache.org/r/25105/#comment90206>
Can you push this into linux/cgroups.cpp and namespace appropriately. See the memory and cpu for guidance.
src/slave/containerizer/isolators/cgroups/blkio.cpp
<https://reviews.apache.org/r/25105/#comment90207>
Push into linux/cgroups.{c,h}pp
- Ian Downes
On Aug. 27, 2014, 11:50 a.m., Patrick Reilly wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25105/
> -----------------------------------------------------------
>
> (Updated Aug. 27, 2014, 11:50 a.m.)
>
>
> Review request for mesos, Adam B and Benjamin Hindman.
>
>
> Bugs: MESOS-350
> https://issues.apache.org/jira/browse/MESOS-350
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Currently there is no disk isolation in place and this affects an executor to be starved of disk when another disk heavy operation such as copying a multi gigabyte file is being performed by another executor.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto dea51f9
> src/Makefile.am 40b9f6b
> src/slave/containerizer/isolators/cgroups/blkio.hpp PRE-CREATION
> src/slave/containerizer/isolators/cgroups/blkio.cpp PRE-CREATION
> src/slave/containerizer/mesos/containerizer.cpp 5116b14
>
> Diff: https://reviews.apache.org/r/25105/diff/
>
>
> Testing
> -------
>
> In progress.
>
>
> Thanks,
>
> Patrick Reilly
>
>
Re: Review Request 25105: Explore disk io isolation in cgroups
Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25105/#review51728
-----------------------------------------------------------
src/slave/containerizer/isolators/cgroups/blkio.cpp
<https://reviews.apache.org/r/25105/#comment90284>
Pad this with 4 more spaces.
- Timothy Chen
On Aug. 28, 2014, 12:27 a.m., Patrick Reilly wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25105/
> -----------------------------------------------------------
>
> (Updated Aug. 28, 2014, 12:27 a.m.)
>
>
> Review request for mesos, Adam B and Benjamin Hindman.
>
>
> Bugs: MESOS-350
> https://issues.apache.org/jira/browse/MESOS-350
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Currently there is no disk isolation in place and this affects an executor to be starved of disk when another disk heavy operation such as copying a multi gigabyte file is being performed by another executor.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto dea51f9
> src/Makefile.am 40b9f6b
> src/linux/cgroups.hpp abf31df
> src/linux/cgroups.cpp 989e307
> src/slave/containerizer/isolators/cgroups/blkio.hpp PRE-CREATION
> src/slave/containerizer/isolators/cgroups/blkio.cpp PRE-CREATION
> src/slave/containerizer/mesos/containerizer.cpp 5116b14
>
> Diff: https://reviews.apache.org/r/25105/diff/
>
>
> Testing
> -------
>
> In progress.
>
>
> Thanks,
>
> Patrick Reilly
>
>
Re: Review Request 25105: Explore disk io isolation in cgroups
Posted by Patrick Reilly <pa...@gmail.com>.
> On Aug. 28, 2014, 12:56 a.m., Timothy Chen wrote:
> > src/slave/containerizer/isolators/cgroups/blkio.hpp, line 36
> > <https://reviews.apache.org/r/25105/diff/8/?file=670662#file670662line36>
> >
> > Are you working on unit tests?
[~tnachen] they are currently being worked on.
- Patrick
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25105/#review51730
-----------------------------------------------------------
On Aug. 28, 2014, 1:12 a.m., Patrick Reilly wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25105/
> -----------------------------------------------------------
>
> (Updated Aug. 28, 2014, 1:12 a.m.)
>
>
> Review request for mesos, Adam B and Benjamin Hindman.
>
>
> Bugs: MESOS-350
> https://issues.apache.org/jira/browse/MESOS-350
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Currently there is no disk isolation in place and this affects an executor to be starved of disk when another disk heavy operation such as copying a multi gigabyte file is being performed by another executor.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto dea51f9
> src/Makefile.am 40b9f6b
> src/linux/cgroups.hpp abf31df
> src/linux/cgroups.cpp 989e307
> src/slave/containerizer/isolators/cgroups/blkio.hpp PRE-CREATION
> src/slave/containerizer/isolators/cgroups/blkio.cpp PRE-CREATION
> src/slave/containerizer/mesos/containerizer.cpp 5116b14
>
> Diff: https://reviews.apache.org/r/25105/diff/
>
>
> Testing
> -------
>
> In progress.
>
>
> Thanks,
>
> Patrick Reilly
>
>
Re: Review Request 25105: Explore disk io isolation in cgroups
Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25105/#review51730
-----------------------------------------------------------
src/slave/containerizer/isolators/cgroups/blkio.hpp
<https://reviews.apache.org/r/25105/#comment90294>
Are you working on unit tests?
src/slave/containerizer/isolators/cgroups/blkio.cpp
<https://reviews.apache.org/r/25105/#comment90288>
Spacing between > >
src/slave/containerizer/isolators/cgroups/blkio.cpp
<https://reviews.apache.org/r/25105/#comment90290>
2 more spaces
src/slave/containerizer/isolators/cgroups/blkio.cpp
<https://reviews.apache.org/r/25105/#comment90293>
plus on previous line
src/slave/containerizer/isolators/cgroups/blkio.cpp
<https://reviews.apache.org/r/25105/#comment90289>
ditto
src/slave/containerizer/isolators/cgroups/blkio.cpp
<https://reviews.apache.org/r/25105/#comment90291>
ditto
src/slave/containerizer/isolators/cgroups/blkio.cpp
<https://reviews.apache.org/r/25105/#comment90292>
+ on previous line
- Timothy Chen
On Aug. 28, 2014, 12:48 a.m., Patrick Reilly wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25105/
> -----------------------------------------------------------
>
> (Updated Aug. 28, 2014, 12:48 a.m.)
>
>
> Review request for mesos, Adam B and Benjamin Hindman.
>
>
> Bugs: MESOS-350
> https://issues.apache.org/jira/browse/MESOS-350
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Currently there is no disk isolation in place and this affects an executor to be starved of disk when another disk heavy operation such as copying a multi gigabyte file is being performed by another executor.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto dea51f9
> src/Makefile.am 40b9f6b
> src/linux/cgroups.hpp abf31df
> src/linux/cgroups.cpp 989e307
> src/slave/containerizer/isolators/cgroups/blkio.hpp PRE-CREATION
> src/slave/containerizer/isolators/cgroups/blkio.cpp PRE-CREATION
> src/slave/containerizer/mesos/containerizer.cpp 5116b14
>
> Diff: https://reviews.apache.org/r/25105/diff/
>
>
> Testing
> -------
>
> In progress.
>
>
> Thanks,
>
> Patrick Reilly
>
>
Re: Review Request 25105: Explore disk io isolation in cgroups
Posted by Nikita Vetoshkin <ni...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25105/#review52429
-----------------------------------------------------------
src/tests/isolator_tests.cpp
<https://reviews.apache.org/r/25105/#comment91221>
A bit offtopic, but there are issues with close() and EINTR at least on Linux: http://ewontfix.com/4/.
TL;DR: EINTR returns with fd already closed and retrying could close another fd opened by any other thread.
- Nikita Vetoshkin
On Sept. 4, 2014, 11:01 p.m., Patrick Reilly wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25105/
> -----------------------------------------------------------
>
> (Updated Sept. 4, 2014, 11:01 p.m.)
>
>
> Review request for mesos, Adam B and Benjamin Hindman.
>
>
> Bugs: MESOS-350
> https://issues.apache.org/jira/browse/MESOS-350
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Currently there is no disk isolation in place and this affects an executor to be starved of disk when another disk heavy operation such as copying a multi gigabyte file is being performed by another executor.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto dea51f9
> include/mesos/resources.hpp 0e37170
> src/Makefile.am 5526189
> src/common/resources.cpp 29fc765
> src/linux/cgroups.hpp abf31df
> src/linux/cgroups.cpp 989e307
> src/slave/containerizer/isolators/cgroups/blkio.hpp PRE-CREATION
> src/slave/containerizer/isolators/cgroups/blkio.cpp PRE-CREATION
> src/slave/containerizer/mesos/containerizer.cpp 5116b14
> src/tests/isolator_tests.cpp c38f876
>
> Diff: https://reviews.apache.org/r/25105/diff/
>
>
> Testing
> -------
>
> This now implements isolation at the read / write level, using both iops or kbps. Tests for both are also included.
>
>
> Thanks,
>
> Patrick Reilly
>
>
Re: Review Request 25105: Explore disk io isolation in cgroups
Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25105/#review52372
-----------------------------------------------------------
src/linux/cgroups.cpp
<https://reviews.apache.org/r/25105/#comment91144>
Single line comments.
src/linux/cgroups.cpp
<https://reviews.apache.org/r/25105/#comment91145>
two spaces between methods. And all other places.
src/slave/containerizer/isolators/cgroups/blkio.cpp
<https://reviews.apache.org/r/25105/#comment91146>
previous line
src/slave/containerizer/isolators/cgroups/blkio.cpp
<https://reviews.apache.org/r/25105/#comment91152>
Format log.
LOG(INFO) << "....."
<< "......";
src/tests/isolator_tests.cpp
<https://reviews.apache.org/r/25105/#comment91159>
I'm not familiar with boost concept checks, but are you using it in your tests?
src/tests/isolator_tests.cpp
<https://reviews.apache.org/r/25105/#comment91153>
pad with 4 spaces.
src/tests/isolator_tests.cpp
<https://reviews.apache.org/r/25105/#comment91156>
previous line
src/common/resources.cpp
<https://reviews.apache.org/r/25105/#comment91137>
Use single line comments, and this already exceeds 70 char.
src/common/resources.cpp
<https://reviews.apache.org/r/25105/#comment91142>
two spaces in between methods.
src/common/resources.cpp
<https://reviews.apache.org/r/25105/#comment91141>
formatting?
src/common/resources.cpp
<https://reviews.apache.org/r/25105/#comment91140>
two spaces between methods.
src/common/resources.cpp
<https://reviews.apache.org/r/25105/#comment91139>
two spaces between methods.
src/common/resources.cpp
<https://reviews.apache.org/r/25105/#comment91138>
formatting.
src/common/resources.cpp
<https://reviews.apache.org/r/25105/#comment91143>
two spaces in between methods.
- Timothy Chen
On Sept. 4, 2014, 11:01 p.m., Patrick Reilly wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25105/
> -----------------------------------------------------------
>
> (Updated Sept. 4, 2014, 11:01 p.m.)
>
>
> Review request for mesos, Adam B and Benjamin Hindman.
>
>
> Bugs: MESOS-350
> https://issues.apache.org/jira/browse/MESOS-350
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Currently there is no disk isolation in place and this affects an executor to be starved of disk when another disk heavy operation such as copying a multi gigabyte file is being performed by another executor.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto dea51f9
> include/mesos/resources.hpp 0e37170
> src/Makefile.am 5526189
> src/common/resources.cpp 29fc765
> src/linux/cgroups.hpp abf31df
> src/linux/cgroups.cpp 989e307
> src/slave/containerizer/isolators/cgroups/blkio.hpp PRE-CREATION
> src/slave/containerizer/isolators/cgroups/blkio.cpp PRE-CREATION
> src/slave/containerizer/mesos/containerizer.cpp 5116b14
> src/tests/isolator_tests.cpp c38f876
>
> Diff: https://reviews.apache.org/r/25105/diff/
>
>
> Testing
> -------
>
> This now implements isolation at the read / write level, using both iops or kbps. Tests for both are also included.
>
>
> Thanks,
>
> Patrick Reilly
>
>
Re: Review Request 25105: Explore disk io isolation in cgroups
Posted by Nikita Vetoshkin <ni...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25105/#review52428
-----------------------------------------------------------
src/linux/cgroups.cpp
<https://reviews.apache.org/r/25105/#comment91219>
How about returning ErrnoError (seems it's the only way to get exact error reason from ifstream)?
src/linux/cgroups.cpp
<https://reviews.apache.org/r/25105/#comment91220>
Shouldn't error message start with a capital letter? Same in other places down the lines.
- Nikita Vetoshkin
On Sept. 4, 2014, 11:01 p.m., Patrick Reilly wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25105/
> -----------------------------------------------------------
>
> (Updated Sept. 4, 2014, 11:01 p.m.)
>
>
> Review request for mesos, Adam B and Benjamin Hindman.
>
>
> Bugs: MESOS-350
> https://issues.apache.org/jira/browse/MESOS-350
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Currently there is no disk isolation in place and this affects an executor to be starved of disk when another disk heavy operation such as copying a multi gigabyte file is being performed by another executor.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto dea51f9
> include/mesos/resources.hpp 0e37170
> src/Makefile.am 5526189
> src/common/resources.cpp 29fc765
> src/linux/cgroups.hpp abf31df
> src/linux/cgroups.cpp 989e307
> src/slave/containerizer/isolators/cgroups/blkio.hpp PRE-CREATION
> src/slave/containerizer/isolators/cgroups/blkio.cpp PRE-CREATION
> src/slave/containerizer/mesos/containerizer.cpp 5116b14
> src/tests/isolator_tests.cpp c38f876
>
> Diff: https://reviews.apache.org/r/25105/diff/
>
>
> Testing
> -------
>
> This now implements isolation at the read / write level, using both iops or kbps. Tests for both are also included.
>
>
> Thanks,
>
> Patrick Reilly
>
>
Re: Review Request 25105: Explore disk io isolation in cgroups
Posted by Patrick Reilly <pa...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25105/
-----------------------------------------------------------
(Updated Sept. 4, 2014, 11:01 p.m.)
Review request for mesos, Adam B and Benjamin Hindman.
Changes
-------
Updating testing.
Bugs: MESOS-350
https://issues.apache.org/jira/browse/MESOS-350
Repository: mesos-git
Description
-------
Currently there is no disk isolation in place and this affects an executor to be starved of disk when another disk heavy operation such as copying a multi gigabyte file is being performed by another executor.
Diffs
-----
include/mesos/mesos.proto dea51f9
include/mesos/resources.hpp 0e37170
src/Makefile.am 5526189
src/common/resources.cpp 29fc765
src/linux/cgroups.hpp abf31df
src/linux/cgroups.cpp 989e307
src/slave/containerizer/isolators/cgroups/blkio.hpp PRE-CREATION
src/slave/containerizer/isolators/cgroups/blkio.cpp PRE-CREATION
src/slave/containerizer/mesos/containerizer.cpp 5116b14
src/tests/isolator_tests.cpp c38f876
Diff: https://reviews.apache.org/r/25105/diff/
Testing (updated)
-------
This now implements isolation at the read / write level, using both iops or kbps. Tests for both are also included.
Thanks,
Patrick Reilly
Re: Review Request 25105: Explore disk io isolation in cgroups
Posted by Patrick Reilly <pa...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25105/
-----------------------------------------------------------
(Updated Sept. 4, 2014, 10:49 p.m.)
Review request for mesos, Adam B and Benjamin Hindman.
Changes
-------
Fixing cosmetics.
Bugs: MESOS-350
https://issues.apache.org/jira/browse/MESOS-350
Repository: mesos-git
Description
-------
Currently there is no disk isolation in place and this affects an executor to be starved of disk when another disk heavy operation such as copying a multi gigabyte file is being performed by another executor.
Diffs (updated)
-----
include/mesos/mesos.proto dea51f9
include/mesos/resources.hpp 0e37170
src/Makefile.am 5526189
src/common/resources.cpp 29fc765
src/linux/cgroups.hpp abf31df
src/linux/cgroups.cpp 989e307
src/slave/containerizer/isolators/cgroups/blkio.hpp PRE-CREATION
src/slave/containerizer/isolators/cgroups/blkio.cpp PRE-CREATION
src/slave/containerizer/mesos/containerizer.cpp 5116b14
src/tests/isolator_tests.cpp c38f876
Diff: https://reviews.apache.org/r/25105/diff/
Testing
-------
In progress.
Thanks,
Patrick Reilly
Re: Review Request 25105: Explore disk io isolation in cgroups
Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25105/#review51914
-----------------------------------------------------------
src/slave/containerizer/isolators/cgroups/blkio.cpp
<https://reviews.apache.org/r/25105/#comment90597>
pad 4 spaces.
src/tests/isolator_tests.cpp
<https://reviews.apache.org/r/25105/#comment90598>
end period.
src/tests/isolator_tests.cpp
<https://reviews.apache.org/r/25105/#comment90599>
Mesos Tests creates a temporary sandbox for each test run, you should write it into your os::getcwd directory instead.
src/tests/isolator_tests.cpp
<https://reviews.apache.org/r/25105/#comment90601>
Why sleep?
- Timothy Chen
On Aug. 29, 2014, 8:36 p.m., Patrick Reilly wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25105/
> -----------------------------------------------------------
>
> (Updated Aug. 29, 2014, 8:36 p.m.)
>
>
> Review request for mesos, Adam B and Benjamin Hindman.
>
>
> Bugs: MESOS-350
> https://issues.apache.org/jira/browse/MESOS-350
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Currently there is no disk isolation in place and this affects an executor to be starved of disk when another disk heavy operation such as copying a multi gigabyte file is being performed by another executor.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto dea51f9
> src/Makefile.am 40b9f6b
> src/linux/cgroups.hpp abf31df
> src/linux/cgroups.cpp 989e307
> src/slave/containerizer/isolators/cgroups/blkio.hpp PRE-CREATION
> src/slave/containerizer/isolators/cgroups/blkio.cpp PRE-CREATION
> src/slave/containerizer/mesos/containerizer.cpp 5116b14
> src/tests/isolator_tests.cpp c38f876
>
> Diff: https://reviews.apache.org/r/25105/diff/
>
>
> Testing
> -------
>
> In progress.
>
>
> Thanks,
>
> Patrick Reilly
>
>
Re: Review Request 25105: Explore disk io isolation in cgroups
Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25105/#review51948
-----------------------------------------------------------
Bad patch!
Reviews applied: [25105]
Failed command: ./support/mesos-style.py
Error:
Checking 506 files using filter --filter=-,+build/class,+build/deprecated,+build/endif_comment,+readability/todo,+readability/namespace,+runtime/vlog,+whitespace/blank_line,+whitespace/comma,+whitespace/ending_newline,+whitespace/forcolon,+whitespace/indent,+whitespace/line_length,+whitespace/tab,+whitespace/todo
src/tests/isolator_tests.cpp:903: Could not find a newline character at the end of the file. [whitespace/ending_newline] [5]
Total errors found: 1
- Mesos ReviewBot
On Aug. 29, 2014, 8:36 p.m., Patrick Reilly wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25105/
> -----------------------------------------------------------
>
> (Updated Aug. 29, 2014, 8:36 p.m.)
>
>
> Review request for mesos, Adam B and Benjamin Hindman.
>
>
> Bugs: MESOS-350
> https://issues.apache.org/jira/browse/MESOS-350
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Currently there is no disk isolation in place and this affects an executor to be starved of disk when another disk heavy operation such as copying a multi gigabyte file is being performed by another executor.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto dea51f9
> src/Makefile.am 40b9f6b
> src/linux/cgroups.hpp abf31df
> src/linux/cgroups.cpp 989e307
> src/slave/containerizer/isolators/cgroups/blkio.hpp PRE-CREATION
> src/slave/containerizer/isolators/cgroups/blkio.cpp PRE-CREATION
> src/slave/containerizer/mesos/containerizer.cpp 5116b14
> src/tests/isolator_tests.cpp c38f876
>
> Diff: https://reviews.apache.org/r/25105/diff/
>
>
> Testing
> -------
>
> In progress.
>
>
> Thanks,
>
> Patrick Reilly
>
>
Re: Review Request 25105: Explore disk io isolation in cgroups
Posted by Patrick Reilly <pa...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25105/
-----------------------------------------------------------
(Updated Aug. 29, 2014, 8:36 p.m.)
Review request for mesos, Adam B and Benjamin Hindman.
Changes
-------
Fixes character limit aesthetic
Bugs: MESOS-350
https://issues.apache.org/jira/browse/MESOS-350
Repository: mesos-git
Description
-------
Currently there is no disk isolation in place and this affects an executor to be starved of disk when another disk heavy operation such as copying a multi gigabyte file is being performed by another executor.
Diffs (updated)
-----
include/mesos/mesos.proto dea51f9
src/Makefile.am 40b9f6b
src/linux/cgroups.hpp abf31df
src/linux/cgroups.cpp 989e307
src/slave/containerizer/isolators/cgroups/blkio.hpp PRE-CREATION
src/slave/containerizer/isolators/cgroups/blkio.cpp PRE-CREATION
src/slave/containerizer/mesos/containerizer.cpp 5116b14
src/tests/isolator_tests.cpp c38f876
Diff: https://reviews.apache.org/r/25105/diff/
Testing
-------
In progress.
Thanks,
Patrick Reilly
Re: Review Request 25105: Explore disk io isolation in cgroups
Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25105/#review51860
-----------------------------------------------------------
Patch looks great!
Reviews applied: [25105]
All tests passed.
- Mesos ReviewBot
On Aug. 28, 2014, 1:12 a.m., Patrick Reilly wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25105/
> -----------------------------------------------------------
>
> (Updated Aug. 28, 2014, 1:12 a.m.)
>
>
> Review request for mesos, Adam B and Benjamin Hindman.
>
>
> Bugs: MESOS-350
> https://issues.apache.org/jira/browse/MESOS-350
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Currently there is no disk isolation in place and this affects an executor to be starved of disk when another disk heavy operation such as copying a multi gigabyte file is being performed by another executor.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto dea51f9
> src/Makefile.am 40b9f6b
> src/linux/cgroups.hpp abf31df
> src/linux/cgroups.cpp 989e307
> src/slave/containerizer/isolators/cgroups/blkio.hpp PRE-CREATION
> src/slave/containerizer/isolators/cgroups/blkio.cpp PRE-CREATION
> src/slave/containerizer/mesos/containerizer.cpp 5116b14
>
> Diff: https://reviews.apache.org/r/25105/diff/
>
>
> Testing
> -------
>
> In progress.
>
>
> Thanks,
>
> Patrick Reilly
>
>
Re: Review Request 25105: Explore disk io isolation in cgroups
Posted by Patrick Reilly <pa...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25105/
-----------------------------------------------------------
(Updated Aug. 28, 2014, 1:12 a.m.)
Review request for mesos, Adam B and Benjamin Hindman.
Changes
-------
Various fixes for mesos style
Bugs: MESOS-350
https://issues.apache.org/jira/browse/MESOS-350
Repository: mesos-git
Description
-------
Currently there is no disk isolation in place and this affects an executor to be starved of disk when another disk heavy operation such as copying a multi gigabyte file is being performed by another executor.
Diffs (updated)
-----
include/mesos/mesos.proto dea51f9
src/Makefile.am 40b9f6b
src/linux/cgroups.hpp abf31df
src/linux/cgroups.cpp 989e307
src/slave/containerizer/isolators/cgroups/blkio.hpp PRE-CREATION
src/slave/containerizer/isolators/cgroups/blkio.cpp PRE-CREATION
src/slave/containerizer/mesos/containerizer.cpp 5116b14
Diff: https://reviews.apache.org/r/25105/diff/
Testing
-------
In progress.
Thanks,
Patrick Reilly
Re: Review Request 25105: Explore disk io isolation in cgroups
Posted by Patrick Reilly <pa...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25105/
-----------------------------------------------------------
(Updated Aug. 28, 2014, 12:48 a.m.)
Review request for mesos, Adam B and Benjamin Hindman.
Changes
-------
Pad with 4 more spaces
Bugs: MESOS-350
https://issues.apache.org/jira/browse/MESOS-350
Repository: mesos-git
Description
-------
Currently there is no disk isolation in place and this affects an executor to be starved of disk when another disk heavy operation such as copying a multi gigabyte file is being performed by another executor.
Diffs (updated)
-----
include/mesos/mesos.proto dea51f9
src/Makefile.am 40b9f6b
src/linux/cgroups.hpp abf31df
src/linux/cgroups.cpp 989e307
src/slave/containerizer/isolators/cgroups/blkio.hpp PRE-CREATION
src/slave/containerizer/isolators/cgroups/blkio.cpp PRE-CREATION
src/slave/containerizer/mesos/containerizer.cpp 5116b14
Diff: https://reviews.apache.org/r/25105/diff/
Testing
-------
In progress.
Thanks,
Patrick Reilly
Re: Review Request 25105: Explore disk io isolation in cgroups
Posted by Patrick Reilly <pa...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25105/
-----------------------------------------------------------
(Updated Aug. 28, 2014, 12:27 a.m.)
Review request for mesos, Adam B and Benjamin Hindman.
Changes
-------
Various fixes for reported issues.
Bugs: MESOS-350
https://issues.apache.org/jira/browse/MESOS-350
Repository: mesos-git
Description
-------
Currently there is no disk isolation in place and this affects an executor to be starved of disk when another disk heavy operation such as copying a multi gigabyte file is being performed by another executor.
Diffs (updated)
-----
include/mesos/mesos.proto dea51f9
src/Makefile.am 40b9f6b
src/linux/cgroups.hpp abf31df
src/linux/cgroups.cpp 989e307
src/slave/containerizer/isolators/cgroups/blkio.hpp PRE-CREATION
src/slave/containerizer/isolators/cgroups/blkio.cpp PRE-CREATION
src/slave/containerizer/mesos/containerizer.cpp 5116b14
Diff: https://reviews.apache.org/r/25105/diff/
Testing
-------
In progress.
Thanks,
Patrick Reilly
Re: Review Request 25105: Explore disk io isolation in cgroups
Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25105/#review51724
-----------------------------------------------------------
src/slave/containerizer/isolators/cgroups/blkio.cpp
<https://reviews.apache.org/r/25105/#comment90273>
We usually put the + in the previous line, and pad this 2nd line with 4 spaces.
src/slave/containerizer/isolators/cgroups/blkio.cpp
<https://reviews.apache.org/r/25105/#comment90275>
Not trying to nitpick, but if you don't plan to implement this, please put period in the end.
src/slave/containerizer/isolators/cgroups/blkio.cpp
<https://reviews.apache.org/r/25105/#comment90274>
return Nothing();
src/slave/containerizer/isolators/cgroups/blkio.cpp
<https://reviews.apache.org/r/25105/#comment90276>
4 spaces pad.
src/linux/cgroups.hpp
<https://reviews.apache.org/r/25105/#comment90270>
Need to space out the closing > for compatibility reasons.
Try<std::pair<uint64_t, uint64_t> >
src/linux/cgroups.cpp
<https://reviews.apache.org/r/25105/#comment90271>
ditto
src/linux/cgroups.cpp
<https://reviews.apache.org/r/25105/#comment90272>
End with period.
- Timothy Chen
On Aug. 27, 2014, 9:49 p.m., Patrick Reilly wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25105/
> -----------------------------------------------------------
>
> (Updated Aug. 27, 2014, 9:49 p.m.)
>
>
> Review request for mesos, Adam B and Benjamin Hindman.
>
>
> Bugs: MESOS-350
> https://issues.apache.org/jira/browse/MESOS-350
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Currently there is no disk isolation in place and this affects an executor to be starved of disk when another disk heavy operation such as copying a multi gigabyte file is being performed by another executor.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto dea51f9
> src/Makefile.am 40b9f6b
> src/linux/cgroups.hpp abf31df
> src/linux/cgroups.cpp 989e307
> src/slave/containerizer/isolators/cgroups/blkio.hpp PRE-CREATION
> src/slave/containerizer/isolators/cgroups/blkio.cpp PRE-CREATION
> src/slave/containerizer/mesos/containerizer.cpp 5116b14
>
> Diff: https://reviews.apache.org/r/25105/diff/
>
>
> Testing
> -------
>
> In progress.
>
>
> Thanks,
>
> Patrick Reilly
>
>
Re: Review Request 25105: Explore disk io isolation in cgroups
Posted by Patrick Reilly <pa...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25105/
-----------------------------------------------------------
(Updated Aug. 27, 2014, 9:49 p.m.)
Review request for mesos, Adam B and Benjamin Hindman.
Changes
-------
Fix to become a single line since it fits 80 char width.
Bugs: MESOS-350
https://issues.apache.org/jira/browse/MESOS-350
Repository: mesos-git
Description
-------
Currently there is no disk isolation in place and this affects an executor to be starved of disk when another disk heavy operation such as copying a multi gigabyte file is being performed by another executor.
Diffs (updated)
-----
include/mesos/mesos.proto dea51f9
src/Makefile.am 40b9f6b
src/linux/cgroups.hpp abf31df
src/linux/cgroups.cpp 989e307
src/slave/containerizer/isolators/cgroups/blkio.hpp PRE-CREATION
src/slave/containerizer/isolators/cgroups/blkio.cpp PRE-CREATION
src/slave/containerizer/mesos/containerizer.cpp 5116b14
Diff: https://reviews.apache.org/r/25105/diff/
Testing
-------
In progress.
Thanks,
Patrick Reilly
Re: Review Request 25105: Explore disk io isolation in cgroups
Posted by Patrick Reilly <pa...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25105/
-----------------------------------------------------------
(Updated Aug. 27, 2014, 9:38 p.m.)
Review request for mesos, Adam B and Benjamin Hindman.
Changes
-------
Various small fixes still a total work in progress, (WIP)
Bugs: MESOS-350
https://issues.apache.org/jira/browse/MESOS-350
Repository: mesos-git
Description
-------
Currently there is no disk isolation in place and this affects an executor to be starved of disk when another disk heavy operation such as copying a multi gigabyte file is being performed by another executor.
Diffs (updated)
-----
include/mesos/mesos.proto dea51f9
src/Makefile.am 40b9f6b
src/linux/cgroups.hpp abf31df
src/linux/cgroups.cpp 989e307
src/slave/containerizer/isolators/cgroups/blkio.hpp PRE-CREATION
src/slave/containerizer/isolators/cgroups/blkio.cpp PRE-CREATION
src/slave/containerizer/mesos/containerizer.cpp 5116b14
Diff: https://reviews.apache.org/r/25105/diff/
Testing
-------
In progress.
Thanks,
Patrick Reilly
Re: Review Request 25105: Explore disk io isolation in cgroups
Posted by Patrick Reilly <pa...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25105/
-----------------------------------------------------------
(Updated Aug. 27, 2014, 6:50 p.m.)
Review request for mesos, Adam B and Benjamin Hindman.
Changes
-------
Fix silly error.
Bugs: MESOS-350
https://issues.apache.org/jira/browse/MESOS-350
Repository: mesos-git
Description
-------
Currently there is no disk isolation in place and this affects an executor to be starved of disk when another disk heavy operation such as copying a multi gigabyte file is being performed by another executor.
Diffs (updated)
-----
include/mesos/mesos.proto dea51f9
src/Makefile.am 40b9f6b
src/slave/containerizer/isolators/cgroups/blkio.hpp PRE-CREATION
src/slave/containerizer/isolators/cgroups/blkio.cpp PRE-CREATION
src/slave/containerizer/mesos/containerizer.cpp 5116b14
Diff: https://reviews.apache.org/r/25105/diff/
Testing
-------
In progress.
Thanks,
Patrick Reilly
Re: Review Request 25105: Explore disk io isolation in cgroups
Posted by Patrick Reilly <pa...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25105/
-----------------------------------------------------------
(Updated Aug. 27, 2014, 6:40 p.m.)
Review request for mesos, Adam B and Benjamin Hindman.
Changes
-------
A small fix for the style issues
Bugs: MESOS-350
https://issues.apache.org/jira/browse/MESOS-350
Repository: mesos-git
Description
-------
Currently there is no disk isolation in place and this affects an executor to be starved of disk when another disk heavy operation such as copying a multi gigabyte file is being performed by another executor.
Diffs (updated)
-----
include/mesos/mesos.proto dea51f9
src/Makefile.am 40b9f6b
src/slave/containerizer/isolators/cgroups/blkio.hpp PRE-CREATION
src/slave/containerizer/isolators/cgroups/blkio.cpp PRE-CREATION
src/slave/containerizer/mesos/containerizer.cpp 5116b14
Diff: https://reviews.apache.org/r/25105/diff/
Testing
-------
In progress.
Thanks,
Patrick Reilly
Re: Review Request 25105: Explore disk io isolation in cgroups
Posted by Patrick Reilly <pa...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25105/
-----------------------------------------------------------
(Updated Aug. 27, 2014, 6:37 p.m.)
Review request for mesos, Adam B and Benjamin Hindman.
Changes
-------
Fix some style issues
Bugs: MESOS-350
https://issues.apache.org/jira/browse/MESOS-350
Repository: mesos-git
Description
-------
Currently there is no disk isolation in place and this affects an executor to be starved of disk when another disk heavy operation such as copying a multi gigabyte file is being performed by another executor.
Diffs (updated)
-----
include/mesos/mesos.proto dea51f9
src/Makefile.am 40b9f6b
src/slave/containerizer/isolators/cgroups/blkio.hpp PRE-CREATION
src/slave/containerizer/isolators/cgroups/blkio.cpp PRE-CREATION
src/slave/containerizer/mesos/containerizer.cpp 5116b14
Diff: https://reviews.apache.org/r/25105/diff/
Testing
-------
In progress.
Thanks,
Patrick Reilly
Re: Review Request 25105: Explore disk io isolation in cgroups
Posted by Patrick Reilly <pa...@gmail.com>.
> On Aug. 27, 2014, 6:32 p.m., Benjamin Hindman wrote:
> > include/mesos/mesos.proto, lines 440-441
> > <https://reviews.apache.org/r/25105/diff/1/?file=670315#file670315line440>
> >
> > Let's use put the "units" at the end. Also, we can pull out read vs write:
> >
> > disk_read_total_iops
> > disk_write_total_iops
> > disk_read_total_bytes
> > disk_write_total_bytes
> >
> > I like that the 'disk_' prefix matches the current resource name ("disk"), but I wonder if we want to use 'blk_' as the prefix instead, especially if we want to introduce new "disk" specific resources.
>
> Ian Downes wrote:
> We've got (or may have) multiple types of "disk" storage, these come to mind:
> 1) A directory on a shared filesystem.
> 2) A dedicated filesystem mounted somewhere in a container.
> 3) A raw block device exposed to the container.
>
> I second [~benhindman] that these IO statistics should keep the blk prefix; the statistics and throttling tunables are actually block device specific so I think we should preserve that particular naming while other disk types are worked on.
So to clarify [~benhindman] and [~idownes] we want to use 'blk' as the prefix instead?
- Patrick
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25105/#review51672
-----------------------------------------------------------
On Aug. 27, 2014, 9:49 p.m., Patrick Reilly wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25105/
> -----------------------------------------------------------
>
> (Updated Aug. 27, 2014, 9:49 p.m.)
>
>
> Review request for mesos, Adam B and Benjamin Hindman.
>
>
> Bugs: MESOS-350
> https://issues.apache.org/jira/browse/MESOS-350
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Currently there is no disk isolation in place and this affects an executor to be starved of disk when another disk heavy operation such as copying a multi gigabyte file is being performed by another executor.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto dea51f9
> src/Makefile.am 40b9f6b
> src/linux/cgroups.hpp abf31df
> src/linux/cgroups.cpp 989e307
> src/slave/containerizer/isolators/cgroups/blkio.hpp PRE-CREATION
> src/slave/containerizer/isolators/cgroups/blkio.cpp PRE-CREATION
> src/slave/containerizer/mesos/containerizer.cpp 5116b14
>
> Diff: https://reviews.apache.org/r/25105/diff/
>
>
> Testing
> -------
>
> In progress.
>
>
> Thanks,
>
> Patrick Reilly
>
>
Re: Review Request 25105: Explore disk io isolation in cgroups
Posted by Ian Downes <ia...@gmail.com>.
> On Aug. 27, 2014, 11:32 a.m., Benjamin Hindman wrote:
> > include/mesos/mesos.proto, lines 440-441
> > <https://reviews.apache.org/r/25105/diff/1/?file=670315#file670315line440>
> >
> > Let's use put the "units" at the end. Also, we can pull out read vs write:
> >
> > disk_read_total_iops
> > disk_write_total_iops
> > disk_read_total_bytes
> > disk_write_total_bytes
> >
> > I like that the 'disk_' prefix matches the current resource name ("disk"), but I wonder if we want to use 'blk_' as the prefix instead, especially if we want to introduce new "disk" specific resources.
We've got (or may have) multiple types of "disk" storage, these come to mind:
1) A directory on a shared filesystem.
2) A dedicated filesystem mounted somewhere in a container.
3) A raw block device exposed to the container.
I second [~benhindman] that these IO statistics should keep the blk prefix; the statistics and throttling tunables are actually block device specific so I think we should preserve that particular naming while other disk types are worked on.
- Ian
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25105/#review51672
-----------------------------------------------------------
On Aug. 27, 2014, 11:50 a.m., Patrick Reilly wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25105/
> -----------------------------------------------------------
>
> (Updated Aug. 27, 2014, 11:50 a.m.)
>
>
> Review request for mesos, Adam B and Benjamin Hindman.
>
>
> Bugs: MESOS-350
> https://issues.apache.org/jira/browse/MESOS-350
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Currently there is no disk isolation in place and this affects an executor to be starved of disk when another disk heavy operation such as copying a multi gigabyte file is being performed by another executor.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto dea51f9
> src/Makefile.am 40b9f6b
> src/slave/containerizer/isolators/cgroups/blkio.hpp PRE-CREATION
> src/slave/containerizer/isolators/cgroups/blkio.cpp PRE-CREATION
> src/slave/containerizer/mesos/containerizer.cpp 5116b14
>
> Diff: https://reviews.apache.org/r/25105/diff/
>
>
> Testing
> -------
>
> In progress.
>
>
> Thanks,
>
> Patrick Reilly
>
>
Re: Review Request 25105: Explore disk io isolation in cgroups
Posted by Patrick Reilly <pa...@gmail.com>.
> On Aug. 27, 2014, 6:32 p.m., Benjamin Hindman wrote:
> > src/slave/containerizer/isolators/cgroups/blkio.hpp, line 35
> > <https://reviews.apache.org/r/25105/diff/1/?file=670317#file670317line35>
> >
> > This doesn't compile. ;-)
Yeah I'm not quite sure how I ended up with an unescaped # causing the preprocessor to barf.
- Patrick
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25105/#review51672
-----------------------------------------------------------
On Aug. 28, 2014, 12:27 a.m., Patrick Reilly wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25105/
> -----------------------------------------------------------
>
> (Updated Aug. 28, 2014, 12:27 a.m.)
>
>
> Review request for mesos, Adam B and Benjamin Hindman.
>
>
> Bugs: MESOS-350
> https://issues.apache.org/jira/browse/MESOS-350
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Currently there is no disk isolation in place and this affects an executor to be starved of disk when another disk heavy operation such as copying a multi gigabyte file is being performed by another executor.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto dea51f9
> src/Makefile.am 40b9f6b
> src/linux/cgroups.hpp abf31df
> src/linux/cgroups.cpp 989e307
> src/slave/containerizer/isolators/cgroups/blkio.hpp PRE-CREATION
> src/slave/containerizer/isolators/cgroups/blkio.cpp PRE-CREATION
> src/slave/containerizer/mesos/containerizer.cpp 5116b14
>
> Diff: https://reviews.apache.org/r/25105/diff/
>
>
> Testing
> -------
>
> In progress.
>
>
> Thanks,
>
> Patrick Reilly
>
>
Re: Review Request 25105: Explore disk io isolation in cgroups
Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25105/#review51672
-----------------------------------------------------------
First pass looks great. See Tim Chen's review too!
include/mesos/mesos.proto
<https://reviews.apache.org/r/25105/#comment90198>
Let's use put the "units" at the end. Also, we can pull out read vs write:
disk_read_total_iops
disk_write_total_iops
disk_read_total_bytes
disk_write_total_bytes
I like that the 'disk_' prefix matches the current resource name ("disk"), but I wonder if we want to use 'blk_' as the prefix instead, especially if we want to introduce new "disk" specific resources.
src/slave/containerizer/isolators/cgroups/blkio.hpp
<https://reviews.apache.org/r/25105/#comment90199>
This doesn't compile. ;-)
src/slave/containerizer/mesos/containerizer.cpp
<https://reviews.apache.org/r/25105/#comment90191>
Is there a reason to #ifdef this away? Like (most of) the other isolators, once this is in the code base we should always build it and let the operator decide whether or not they want ot use it. The network isolator is a bit more special case because it does a lot more.
- Benjamin Hindman
On Aug. 27, 2014, 5:59 p.m., Patrick Reilly wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25105/
> -----------------------------------------------------------
>
> (Updated Aug. 27, 2014, 5:59 p.m.)
>
>
> Review request for mesos, Adam B and Benjamin Hindman.
>
>
> Bugs: MESOS-350
> https://issues.apache.org/jira/browse/MESOS-350
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Currently there is no disk isolation in place and this affects an executor to be starved of disk when another disk heavy operation such as copying a multi gigabyte file is being performed by another executor.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto dea51f9
> src/Makefile.am 40b9f6b
> src/slave/containerizer/isolators/cgroups/blkio.hpp PRE-CREATION
> src/slave/containerizer/isolators/cgroups/blkio.cpp PRE-CREATION
> src/slave/containerizer/mesos/containerizer.cpp 5116b14
>
> Diff: https://reviews.apache.org/r/25105/diff/
>
>
> Testing
> -------
>
> In progress.
>
>
> Thanks,
>
> Patrick Reilly
>
>
Re: Review Request 25105: Explore disk io isolation in cgroups
Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25105/#review51669
-----------------------------------------------------------
src/slave/containerizer/isolators/cgroups/blkio.hpp
<https://reviews.apache.org/r/25105/#comment90184>
End your comment with a period.
src/slave/containerizer/isolators/cgroups/blkio.hpp
<https://reviews.apache.org/r/25105/#comment90185>
We typically use single line comment for TODOs. // TODO(preilly):
src/slave/containerizer/isolators/cgroups/blkio.hpp
<https://reviews.apache.org/r/25105/#comment90186>
ditto
src/slave/containerizer/isolators/cgroups/blkio.hpp
<https://reviews.apache.org/r/25105/#comment90187>
Single line comments.
src/slave/containerizer/isolators/cgroups/blkio.cpp
<https://reviews.apache.org/r/25105/#comment90189>
These two lines can become a single line since it fits 80 char width.
src/slave/containerizer/isolators/cgroups/blkio.cpp
<https://reviews.apache.org/r/25105/#comment90190>
We never throw exceptions in Mesos code (so let's not make this the first). Return Failure("TODO: Implement") for the same intention.
However, I think instead of throwing an error you probably want to just return Nothing() and have a TODO comment.
src/slave/containerizer/isolators/cgroups/blkio.cpp
<https://reviews.apache.org/r/25105/#comment90192>
ditto.
src/slave/containerizer/isolators/cgroups/blkio.cpp
<https://reviews.apache.org/r/25105/#comment90193>
ditto
src/slave/containerizer/isolators/cgroups/blkio.cpp
<https://reviews.apache.org/r/25105/#comment90194>
ditto
src/slave/containerizer/isolators/cgroups/blkio.cpp
<https://reviews.apache.org/r/25105/#comment90195>
Not used?
- Timothy Chen
On Aug. 27, 2014, 5:59 p.m., Patrick Reilly wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25105/
> -----------------------------------------------------------
>
> (Updated Aug. 27, 2014, 5:59 p.m.)
>
>
> Review request for mesos, Adam B and Benjamin Hindman.
>
>
> Bugs: MESOS-350
> https://issues.apache.org/jira/browse/MESOS-350
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Currently there is no disk isolation in place and this affects an executor to be starved of disk when another disk heavy operation such as copying a multi gigabyte file is being performed by another executor.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto dea51f9
> src/Makefile.am 40b9f6b
> src/slave/containerizer/isolators/cgroups/blkio.hpp PRE-CREATION
> src/slave/containerizer/isolators/cgroups/blkio.cpp PRE-CREATION
> src/slave/containerizer/mesos/containerizer.cpp 5116b14
>
> Diff: https://reviews.apache.org/r/25105/diff/
>
>
> Testing
> -------
>
> In progress.
>
>
> Thanks,
>
> Patrick Reilly
>
>
Re: Review Request 25105: Explore disk io isolation in cgroups
Posted by Patrick Reilly <pa...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25105/
-----------------------------------------------------------
(Updated Aug. 27, 2014, 5:59 p.m.)
Review request for mesos, Adam B and Benjamin Hindman.
Bugs: MESOS-350
https://issues.apache.org/jira/browse/MESOS-350
Repository: mesos-git
Description
-------
Currently there is no disk isolation in place and this affects an executor to be starved of disk when another disk heavy operation such as copying a multi gigabyte file is being performed by another executor.
Diffs
-----
include/mesos/mesos.proto dea51f9
src/Makefile.am 40b9f6b
src/slave/containerizer/isolators/cgroups/blkio.hpp PRE-CREATION
src/slave/containerizer/isolators/cgroups/blkio.cpp PRE-CREATION
src/slave/containerizer/mesos/containerizer.cpp 5116b14
Diff: https://reviews.apache.org/r/25105/diff/
Testing
-------
In progress.
Thanks,
Patrick Reilly