You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by haosdent huang <ha...@gmail.com> on 2016/04/01 04:08:23 UTC
Re: Review Request 45084: Add `Subsystem` abstraction for cgroups.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45084/
-----------------------------------------------------------
(Updated April 1, 2016, 2:08 a.m.)
Review request for mesos and Jie Yu.
Changes
-------
Rebase.
Bugs: MESOS-5039
https://issues.apache.org/jira/browse/MESOS-5039
Repository: mesos
Description
-------
Add `Subsystem` abstraction for cgroups.
Diffs (updated)
-----
src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp PRE-CREATION
src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/45084/diff/
Testing
-------
Thanks,
haosdent huang
Re: Review Request 45084: Added stubs for the `Subsystem` abstraction
of cgroups unified isolator.
Posted by haosdent huang <ha...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45084/
-----------------------------------------------------------
(Updated April 16, 2016, 10:13 a.m.)
Review request for mesos, Gilbert Song, Guangya Liu, Ian Downes, Jie Yu, and Kevin Klues.
Changes
-------
Rebase.
Bugs: MESOS-5039
https://issues.apache.org/jira/browse/MESOS-5039
Repository: mesos
Description
-------
Added stubs for the `Subsystem` abstraction of cgroups unified isolator.
Diffs (updated)
-----
src/CMakeLists.txt 06f58c4a88e3c527df727df1efe11ed3ab77efa8
src/Makefile.am ec855263d620e4723c8ba9cd056c40a3a2e9ca99
src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp PRE-CREATION
src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/45084/diff/
Testing
-------
Thanks,
haosdent huang
Re: Review Request 45084: Added stubs for the `Subsystem` abstraction
of cgroups unified isolator.
Posted by haosdent huang <ha...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45084/
-----------------------------------------------------------
(Updated April 13, 2016, 6:45 p.m.)
Review request for mesos, Gilbert Song, Guangya Liu, Ian Downes, Jie Yu, and Kevin Klues.
Changes
-------
Rebase.
Bugs: MESOS-5039
https://issues.apache.org/jira/browse/MESOS-5039
Repository: mesos
Description
-------
Added stubs for the `Subsystem` abstraction of cgroups unified isolator.
Diffs (updated)
-----
src/CMakeLists.txt ca59a1824352423f4db6ef8bb41acc6fe602c041
src/Makefile.am a8f68316c5d4d4c82d99363535a97fa5b1caafc5
src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp PRE-CREATION
src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/45084/diff/
Testing
-------
Thanks,
haosdent huang
Re: Review Request 45084: Added stubs for the `Subsystem` abstraction
of cgroups unified isolator.
Posted by haosdent huang <ha...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45084/
-----------------------------------------------------------
(Updated April 11, 2016, 6:26 p.m.)
Review request for mesos, Gilbert Song, Guangya Liu, Ian Downes, Jie Yu, and Kevin Klues.
Changes
-------
Address @jieyu's comments.
Summary (updated)
-----------------
Added stubs for the `Subsystem` abstraction of cgroups unified isolator.
Bugs: MESOS-5039
https://issues.apache.org/jira/browse/MESOS-5039
Repository: mesos
Description (updated)
-------
Added stubs for the `Subsystem` abstraction of cgroups unified isolator.
Diffs (updated)
-----
src/CMakeLists.txt ca59a1824352423f4db6ef8bb41acc6fe602c041
src/Makefile.am dc8f8e31c5ea229e20c4f7e4d15db1ae756938aa
src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp PRE-CREATION
src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/45084/diff/
Testing
-------
Thanks,
haosdent huang
Re: Review Request 45084: Add `Subsystem` abstraction for cgroups.
Posted by haosdent huang <ha...@gmail.com>.
> On April 9, 2016, 12:11 a.m., Jie Yu wrote:
> > Does this patch compile on its own? I'd like each patch to be 'atomic' so that we can commit some of them if they look good.
Thanks for your review! Current patch chain are not atomic, let me change the dependencies.
- haosdent
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45084/#review127895
-----------------------------------------------------------
On April 7, 2016, 10:36 a.m., haosdent huang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45084/
> -----------------------------------------------------------
>
> (Updated April 7, 2016, 10:36 a.m.)
>
>
> Review request for mesos, Gilbert Song, Guangya Liu, Ian Downes, Jie Yu, and Kevin Klues.
>
>
> Bugs: MESOS-5039
> https://issues.apache.org/jira/browse/MESOS-5039
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Add `Subsystem` abstraction for cgroups.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp PRE-CREATION
> src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/45084/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> haosdent huang
>
>
Re: Review Request 45084: Add `Subsystem` abstraction for cgroups.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45084/#review127895
-----------------------------------------------------------
Does this patch compile on its own? I'd like each patch to be 'atomic' so that we can commit some of them if they look good.
src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp (lines 24 - 25)
<https://reviews.apache.org/r/45084/#comment191252>
Instead of that, can you just include <mesos/slave/isolator.hpp>?
src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp (line 32)
<https://reviews.apache.org/r/45084/#comment191253>
Why do you need this header?
src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp (line 46)
<https://reviews.apache.org/r/45084/#comment191264>
Could you please put `flags` as the first parameter?
src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp (line 48)
<https://reviews.apache.org/r/45084/#comment191262>
Why return a raw pointer, instead of an Owned pointer here?
src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp (line 53)
<https://reviews.apache.org/r/45084/#comment191263>
Could you please put `flags` as the first parameter?
src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp (line 57)
<https://reviews.apache.org/r/45084/#comment191310>
`virtual std::string name() const = 0;`
We typically do not return const string.
src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp (line 28)
<https://reviews.apache.org/r/45084/#comment191255>
Please use explicit using clauses here.
src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp (lines 68 - 83)
<https://reviews.apache.org/r/45084/#comment191257>
Since you're just adding stubs in this patch, can you introduce such logics later? It's hard to review if you stick in logics like this without any context.
- Jie Yu
On April 7, 2016, 10:36 a.m., haosdent huang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45084/
> -----------------------------------------------------------
>
> (Updated April 7, 2016, 10:36 a.m.)
>
>
> Review request for mesos, Gilbert Song, Guangya Liu, Ian Downes, Jie Yu, and Kevin Klues.
>
>
> Bugs: MESOS-5039
> https://issues.apache.org/jira/browse/MESOS-5039
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Add `Subsystem` abstraction for cgroups.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp PRE-CREATION
> src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/45084/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> haosdent huang
>
>
Re: Review Request 45084: Add `Subsystem` abstraction for cgroups.
Posted by haosdent huang <ha...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45084/
-----------------------------------------------------------
(Updated April 7, 2016, 10:36 a.m.)
Review request for mesos, Gilbert Song, Guangya Liu, Ian Downes, Jie Yu, and Kevin Klues.
Changes
-------
Rebase.
Bugs: MESOS-5039
https://issues.apache.org/jira/browse/MESOS-5039
Repository: mesos
Description
-------
Add `Subsystem` abstraction for cgroups.
Diffs (updated)
-----
src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp PRE-CREATION
src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/45084/diff/
Testing
-------
Thanks,
haosdent huang
Re: Review Request 45084: Add `Subsystem` abstraction for cgroups.
Posted by haosdent huang <ha...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45084/
-----------------------------------------------------------
(Updated April 4, 2016, 5:12 p.m.)
Review request for mesos, Gilbert Song, Guangya Liu, Ian Downes, Jie Yu, and Kevin Klues.
Changes
-------
Rebase and update reviewers.
Bugs: MESOS-5039
https://issues.apache.org/jira/browse/MESOS-5039
Repository: mesos
Description
-------
Add `Subsystem` abstraction for cgroups.
Diffs (updated)
-----
src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp PRE-CREATION
src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/45084/diff/
Testing
-------
Thanks,
haosdent huang