Re: Review Request 50750: Removed CgroupsNetClsIsolatorProcess.

[haosdent huang]

> On Aug. 17, 2016, 8:06 a.m., Qian Zhang wrote:
> > src/tests/environment.cpp, line 438
> > 
> >
> > Here can we change it to the following?
> > ```
> > if (matches(test, "ROOT_CGROUPS_Isolate") ||
> > matches(test, "ROOT_CGROUPS_ContainerStatus") {
> > ```
> > In this way, I think you can still name the test class as 
> > `CgroupsIsolatorTest` which will be consistent with other cgroup's related 
> > tests.
> 
> haosdent huang wrote:
> I found `ROOT_CGROUPS_Isolate` and `ROOT_CGROUPS_ContainerStatus` are not 
> exactly enough here because it only test net_cls. Should we use 
> `ROOT_CGROUPS_NetSubsystemIsolate` and 
> `ROOT_CGROUPS_NetSubsystemContainerStatus` respectively.
> 
> Qian Zhang wrote:
> I think they should be `ROOT_CGROUPS_NetClsIsolate` (rather than 
> `ROOT_CGROUPS_Isolate`) and `ROOT_CGROUPS_ContainerStatus` which are the 
> original test names. So can you please let me know if 
> `ROOT_CGROUPS_NetClsIsolate` and `ROOT_CGROUPS_ContainerStatus` are enough? I 
> think it should be OK since we are doing the similar thing in 
> `PerfCPUCyclesFilter::disable()`, right?

`CgroupsIsolatorTest.ROOT_CGROUPS_ContainerStatus` sounds not exactly, how 
about `CgroupsIsolatorTest.ROOT_CGROUPS_ContainerNetClsStatus`?


- haosdent


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


On Aug. 17, 2016, 5:50 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50750/
> ---
> 
> (Updated Aug. 17, 2016, 5:50 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.
> 
> 
> Bugs: MESOS-5976
> https://issues.apache.org/jira/browse/MESOS-5976
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed CgroupsNetClsIsolatorProcess.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 4362d472707193aa09343ef69f070e2d3efda324 
>   src/Makefile.am d389e02584cfb1a00067cd30de1364118d1c46c7 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 77a502f853e3e04ea8e274419544601778be9421 
>   src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp 
> b3fd8c85476bf46368bd79f052b7923ad9d32199 
>   src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp 
> bebefbba7153d4b0e9a8d7179cfb642e6e802bea 
>   src/tests/containerizer/isolator_tests.cpp 
> 05620d2411d464593cdb5aeaea10cb147047569b 
>   src/tests/environment.cpp 7f144f1763320aef1657a60b293d585b74a83367 
> 
> Diff: https://reviews.apache.org/r/50750/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 50750: Removed CgroupsNetClsIsolatorProcess.

[haosdent huang]

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

(Updated Aug. 18, 2016, 1:50 p.m.)


Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.


Changes
---

Address @qianzhang's comments.


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


Repository: mesos


Description
---

Removed CgroupsNetClsIsolatorProcess.


Diffs (updated)
-

  src/CMakeLists.txt 4362d472707193aa09343ef69f070e2d3efda324 
  src/Makefile.am d389e02584cfb1a00067cd30de1364118d1c46c7 
  src/slave/containerizer/mesos/containerizer.cpp 
77a502f853e3e04ea8e274419544601778be9421 
  src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp 
b3fd8c85476bf46368bd79f052b7923ad9d32199 
  src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp 
bebefbba7153d4b0e9a8d7179cfb642e6e802bea 
  src/tests/containerizer/isolator_tests.cpp 
05620d2411d464593cdb5aeaea10cb147047569b 
  src/tests/environment.cpp 7f144f1763320aef1657a60b293d585b74a83367 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 51031: Added non-recursive version of `cgroups::get`.

[haosdent huang]

> On Aug. 17, 2016, 7:55 p.m., Jiang Yan Xu wrote:
> > Mind adding a test? We already have 
> > `CgroupsAnyHierarchyTest.ROOT_CGROUPS_Get` but we can add another 
> > `ROOT_CGROUPS_Nonresursive_Get` and change the current one into a 
> > `ROOT_CGROUPS_Recursive_Get`.
> > 
> > The argument `bool recursive` defaults to true today because it's the 
> > current behavior. However I am not sure if we actually want this as the 
> > default. This is another disucssion to have but as least let's make the 
> > tests explicit, i.e., `ROOT_CGROUPS_Nonresursive_Get` and 
> > `ROOT_CGROUPS_Recursive_Get` for now.

I think we could change the default value to `false` which match the default 
value in `cgroups::create`. And update `cgroups::get` to use `recursive=true` 
in `cgroups::remove` and `cgroups::destroy`.


- haosdent


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


On Aug. 17, 2016, 5:38 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51031/
> ---
> 
> (Updated Aug. 17, 2016, 5:38 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.
> 
> 
> Bugs: MESOS-6035
> https://issues.apache.org/jira/browse/MESOS-6035
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added non-recursive version of `cgroups::get`.
> 
> 
> Diffs
> -
> 
>   src/linux/cgroups.hpp 047d3ea596660e704447c0f0c7b914a43c4a2187 
>   src/linux/cgroups.cpp f3232c009d04bb82701c0407b12abf999ab60a73 
> 
> Diff: https://reviews.apache.org/r/51031/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 51031: Added non-recursive version of `cgroups::get`.

[haosdent huang]

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

(Updated Aug. 18, 2016, 4:05 p.m.)


Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.


Changes
---

Address @xujyan's comments.


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


Repository: mesos


Description (updated)
---

In most cases, we just want to get the children cgroups instead of
retrieve descendant cgroups recursively. We added an argument to
`cgroups::get` to indicate whether retrieve cgroups recursively and
make non-recursive retrieve become the default behaviour. This patch
fixed some incorrect `TEST_CGROUPS_ROOT` checks as well.


Diffs (updated)
-

  src/linux/cgroups.hpp 047d3ea596660e704447c0f0c7b914a43c4a2187 
  src/linux/cgroups.cpp f3232c009d04bb82701c0407b12abf999ab60a73 
  src/tests/containerizer/cgroups_tests.cpp 
821c7860ab424bcfadbf597902e046bce9ba3044 
  src/tests/mesos.cpp 62e8fcc6fa7bd856aab6148ca6e6cad66b436f04 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 51185: Removed the expired TODO about non-recursive version `cgroups::get`.

[haosdent huang]

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

(Updated Aug. 18, 2016, 4:09 p.m.)


Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, Zhengju Sha, and 
Jiang Yan Xu.


Changes
---

Rebase.


Summary (updated)
-

Removed the expired TODO about non-recursive version `cgroups::get`.


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


Repository: mesos


Description (updated)
---

Removed the expired TODO about non-recursive version `cgroups::get`.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
7e205a3eea4ad89faccf564bdb2ad0041fdee249 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 49852: Implemented `NetClsSubsystem`.

[haosdent huang]

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

(Updated Aug. 18, 2016, 4:09 p.m.)


Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Implemented `NetClsSubsystem`.


Diffs (updated)
-

  src/CMakeLists.txt 4362d472707193aa09343ef69f070e2d3efda324 
  src/Makefile.am d389e02584cfb1a00067cd30de1364118d1c46c7 
  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
7e205a3eea4ad89faccf564bdb2ad0041fdee249 
  src/slave/containerizer/mesos/isolators/cgroups/constants.hpp 
c45d88092f3fe497373dfeaa8346aef9126c7b8b 
  src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp 
b3fd8c85476bf46368bd79f052b7923ad9d32199 
  src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp 
bebefbba7153d4b0e9a8d7179cfb642e6e802bea 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp 
a30ecafcbecc9d3b6eeea2b04dcb4d278750af41 
  src/slave/containerizer/mesos/isolators/cgroups/subsystems/net_cls.hpp 
PRE-CREATION 
  src/slave/containerizer/mesos/isolators/cgroups/subsystems/net_cls.cpp 
PRE-CREATION 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 49855: Enabled cgroups unified isolator in isolation.

[haosdent huang]

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

(Updated Aug. 18, 2016, 4:09 p.m.)


Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Enabled cgroups unified isolator in isolation.


Diffs (updated)
-

  src/slave/containerizer/mesos/containerizer.cpp 
77a502f853e3e04ea8e274419544601778be9421 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 49828: Added default methods implementations for `Subsystem` base class.

[haosdent huang]

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

(Updated Aug. 18, 2016, 4:09 p.m.)


Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Added default methods implementations for `Subsystem` base class.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp 
5f52a076a1fa3a21d886cb961ddeed5046a38d7c 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp 
a30ecafcbecc9d3b6eeea2b04dcb4d278750af41 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 49854: Implemented `DevicesSubsystem`.

[haosdent huang]

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

(Updated Aug. 18, 2016, 4:09 p.m.)


Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Implemented `DevicesSubsystem`.


Diffs (updated)
-

  src/CMakeLists.txt 4362d472707193aa09343ef69f070e2d3efda324 
  src/Makefile.am d389e02584cfb1a00067cd30de1364118d1c46c7 
  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
7e205a3eea4ad89faccf564bdb2ad0041fdee249 
  src/slave/containerizer/mesos/isolators/cgroups/constants.hpp 
c45d88092f3fe497373dfeaa8346aef9126c7b8b 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp 
a30ecafcbecc9d3b6eeea2b04dcb4d278750af41 
  src/slave/containerizer/mesos/isolators/cgroups/subsystems/devices.hpp 
PRE-CREATION 
  src/slave/containerizer/mesos/isolators/cgroups/subsystems/devices.cpp 
PRE-CREATION 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 49849: Implemented `CpuSubsystem`.

[haosdent huang]

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

(Updated Aug. 18, 2016, 4:09 p.m.)


Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Implemented `CpuSubsystem`.


Diffs (updated)
-

  src/CMakeLists.txt 4362d472707193aa09343ef69f070e2d3efda324 
  src/Makefile.am d389e02584cfb1a00067cd30de1364118d1c46c7 
  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
7e205a3eea4ad89faccf564bdb2ad0041fdee249 
  src/slave/containerizer/mesos/isolators/cgroups/constants.hpp 
c45d88092f3fe497373dfeaa8346aef9126c7b8b 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp 
5f52a076a1fa3a21d886cb961ddeed5046a38d7c 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp 
a30ecafcbecc9d3b6eeea2b04dcb4d278750af41 
  src/slave/containerizer/mesos/isolators/cgroups/subsystems/cpu.hpp 
PRE-CREATION 
  src/slave/containerizer/mesos/isolators/cgroups/subsystems/cpu.cpp 
PRE-CREATION 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 50733: Removed CgroupsCpushareIsolatorProcess.

[haosdent huang]

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

(Updated Aug. 18, 2016, 4:09 p.m.)


Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Removed CgroupsCpushareIsolatorProcess.


Diffs (updated)
-

  src/CMakeLists.txt 4362d472707193aa09343ef69f070e2d3efda324 
  src/Makefile.am d389e02584cfb1a00067cd30de1364118d1c46c7 
  src/docker/docker.cpp e07c4aa723f57db7f8bc31d4af5d32a30ebf98f2 
  src/slave/containerizer/mesos/containerizer.cpp 
77a502f853e3e04ea8e274419544601778be9421 
  src/slave/containerizer/mesos/isolators/cgroups/cpushare.hpp 
74982a610b6c0a74734165a0c6aa8c9f72f54deb 
  src/slave/containerizer/mesos/isolators/cgroups/cpushare.cpp 
221e814a448c4b5df9dab98de597451a24e2b89c 
  src/tests/containerizer/isolator_tests.cpp 
05620d2411d464593cdb5aeaea10cb147047569b 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 50751: Removed CgroupsPerfEventIsolatorProcess.

[haosdent huang]

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

(Updated Aug. 18, 2016, 4:10 p.m.)


Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Removed CgroupsPerfEventIsolatorProcess.


Diffs (updated)
-

  src/CMakeLists.txt 4362d472707193aa09343ef69f070e2d3efda324 
  src/Makefile.am d389e02584cfb1a00067cd30de1364118d1c46c7 
  src/slave/containerizer/mesos/containerizer.cpp 
77a502f853e3e04ea8e274419544601778be9421 
  src/slave/containerizer/mesos/isolators/cgroups/perf_event.hpp 
4abde12af68a26b94b3706cdb38bf9890d811039 
  src/slave/containerizer/mesos/isolators/cgroups/perf_event.cpp 
31f35385691681ef5da14be747edfb5f57c5d05a 
  src/slave/containerizer/mesos/isolators/cgroups/subsystems/perf_event.cpp 
PRE-CREATION 
  src/tests/containerizer/isolator_tests.cpp 
05620d2411d464593cdb5aeaea10cb147047569b 
  src/tests/environment.cpp 7f144f1763320aef1657a60b293d585b74a83367 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 50748: Removed CgroupsMemIsolatorProcess.

[haosdent huang]

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

(Updated Aug. 18, 2016, 4:09 p.m.)


Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Removed CgroupsMemIsolatorProcess.


Diffs (updated)
-

  src/CMakeLists.txt 4362d472707193aa09343ef69f070e2d3efda324 
  src/Makefile.am d389e02584cfb1a00067cd30de1364118d1c46c7 
  src/docker/docker.cpp e07c4aa723f57db7f8bc31d4af5d32a30ebf98f2 
  src/slave/containerizer/mesos/containerizer.cpp 
77a502f853e3e04ea8e274419544601778be9421 
  src/slave/containerizer/mesos/isolators/cgroups/mem.hpp 
b3ce6ed2505312bdd2d800164c2f57cd7625c9fa 
  src/slave/containerizer/mesos/isolators/cgroups/mem.cpp 
0a4f38ded5cffd438e2a3b1e7d066c3077557f0d 
  src/tests/containerizer/isolator_tests.cpp 
05620d2411d464593cdb5aeaea10cb147047569b 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 49851: Implemented `MemorySubsystem`.

[haosdent huang]

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

(Updated Aug. 18, 2016, 4:09 p.m.)


Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Implemented `MemorySubsystem`.


Diffs (updated)
-

  src/CMakeLists.txt 4362d472707193aa09343ef69f070e2d3efda324 
  src/Makefile.am d389e02584cfb1a00067cd30de1364118d1c46c7 
  src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp 
9b2d33ec3b023058d00c6671464cc9cd092f653b 
  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
7e205a3eea4ad89faccf564bdb2ad0041fdee249 
  src/slave/containerizer/mesos/isolators/cgroups/constants.hpp 
c45d88092f3fe497373dfeaa8346aef9126c7b8b 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp 
5f52a076a1fa3a21d886cb961ddeed5046a38d7c 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp 
a30ecafcbecc9d3b6eeea2b04dcb4d278750af41 
  src/slave/containerizer/mesos/isolators/cgroups/subsystems/memory.hpp 
PRE-CREATION 
  src/slave/containerizer/mesos/isolators/cgroups/subsystems/memory.cpp 
PRE-CREATION 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 50749: Removed CgroupsDevicesIsolatorProcess.

[haosdent huang]

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

(Updated Aug. 18, 2016, 4:09 p.m.)


Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Removed CgroupsDevicesIsolatorProcess.


Diffs (updated)
-

  src/CMakeLists.txt 4362d472707193aa09343ef69f070e2d3efda324 
  src/Makefile.am d389e02584cfb1a00067cd30de1364118d1c46c7 
  src/slave/containerizer/mesos/containerizer.cpp 
77a502f853e3e04ea8e274419544601778be9421 
  src/slave/containerizer/mesos/isolators/cgroups/devices.hpp 
7ad96440b21a380c5d9af27b0168e9abf47769af 
  src/slave/containerizer/mesos/isolators/cgroups/devices.cpp 
f1b5e75d23780c0e1d53487852422b02c88de9e8 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 49853: Implemented `PerfEventSubsystem`.

[haosdent huang]

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

(Updated Aug. 18, 2016, 4:09 p.m.)


Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Implemented `PerfEventSubsystem`.


Diffs (updated)
-

  src/CMakeLists.txt 4362d472707193aa09343ef69f070e2d3efda324 
  src/Makefile.am d389e02584cfb1a00067cd30de1364118d1c46c7 
  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
7e205a3eea4ad89faccf564bdb2ad0041fdee249 
  src/slave/containerizer/mesos/isolators/cgroups/constants.hpp 
c45d88092f3fe497373dfeaa8346aef9126c7b8b 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp 
a30ecafcbecc9d3b6eeea2b04dcb4d278750af41 
  src/slave/containerizer/mesos/isolators/cgroups/subsystems/perf_event.hpp 
PRE-CREATION 
  src/slave/containerizer/mesos/isolators/cgroups/subsystems/perf_event.cpp 
PRE-CREATION 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 50750: Removed CgroupsNetClsIsolatorProcess.

[haosdent huang]

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

(Updated Aug. 18, 2016, 4:10 p.m.)


Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Removed CgroupsNetClsIsolatorProcess.


Diffs (updated)
-

  src/CMakeLists.txt 4362d472707193aa09343ef69f070e2d3efda324 
  src/Makefile.am d389e02584cfb1a00067cd30de1364118d1c46c7 
  src/slave/containerizer/mesos/containerizer.cpp 
77a502f853e3e04ea8e274419544601778be9421 
  src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp 
b3fd8c85476bf46368bd79f052b7923ad9d32199 
  src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp 
bebefbba7153d4b0e9a8d7179cfb642e6e802bea 
  src/tests/containerizer/isolator_tests.cpp 
05620d2411d464593cdb5aeaea10cb147047569b 
  src/tests/environment.cpp 7f144f1763320aef1657a60b293d585b74a83367 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 50758: Updated `UserCgroupIsolatorTest` to use `CgroupsIsolatorProcess`.

[haosdent huang]

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

(Updated Aug. 18, 2016, 4:09 p.m.)


Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.


Changes
---

Rebase.


Repository: mesos


Description
---

Updated `UserCgroupIsolatorTest` to use `CgroupsIsolatorProcess`.


Diffs (updated)
-

  src/tests/containerizer/isolator_tests.cpp 
05620d2411d464593cdb5aeaea10cb147047569b 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 49850: Implemented `CpuacctSubsystem`.

[haosdent huang]

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

(Updated Aug. 18, 2016, 4:09 p.m.)


Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Implemented `CpuacctSubsystem`.


Diffs (updated)
-

  src/CMakeLists.txt 4362d472707193aa09343ef69f070e2d3efda324 
  src/Makefile.am d389e02584cfb1a00067cd30de1364118d1c46c7 
  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
7e205a3eea4ad89faccf564bdb2ad0041fdee249 
  src/slave/containerizer/mesos/isolators/cgroups/constants.hpp 
c45d88092f3fe497373dfeaa8346aef9126c7b8b 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp 
a30ecafcbecc9d3b6eeea2b04dcb4d278750af41 
  src/slave/containerizer/mesos/isolators/cgroups/subsystems/cpuacct.hpp 
PRE-CREATION 
  src/slave/containerizer/mesos/isolators/cgroups/subsystems/cpuacct.cpp 
PRE-CREATION 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 51091: Disallowed unknown cgroups isolator.

[haosdent huang]

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

(Updated Aug. 18, 2016, 4:25 p.m.)


Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.


Changes
---

Rebase.


Repository: mesos


Description
---

Disallowed unknown cgroups isolator.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
7e205a3eea4ad89faccf564bdb2ad0041fdee249 

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


Testing
---


Thanks,

haosdent huang

Review Request 51215: Moved memory test helper to the common test helper.

[Jie Yu]

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

Review request for mesos and Benjamin Bannier.


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


Repository: mesos


Description
---

Moved memory test helper to the common test helper.


Diffs
-

  src/Makefile.am d389e02584cfb1a00067cd30de1364118d1c46c7 
  src/tests/CMakeLists.txt 1ea8b2102753ae294bd75706ffaf08308e928acd 
  src/tests/cmake/MesosTestsConfigure.cmake 
361032082c3a5f76b949dc7981f75b53e02d84f5 
  src/tests/containerizer/CMakeLists.txt 
2c52e43a9deee90fa32693731d6ebedb5201bb1f 
  src/tests/containerizer/memory_test_helper.hpp 
86a9b57c1872b2efe799b0dc61f3d3d73e4d134b 
  src/tests/containerizer/memory_test_helper.cpp 
e850b376ac61e2dedb07ea418cfeaf400cbba172 
  src/tests/test_helper_main.cpp PRE-CREATION 

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


Testing
---


Thanks,

Jie Yu

Re: Review Request 51031: Added non-recursive version of `cgroups::get`.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [51091, 51031]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker_build.sh

- Mesos ReviewBot


On Aug. 18, 2016, 4:05 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51031/
> ---
> 
> (Updated Aug. 18, 2016, 4:05 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.
> 
> 
> Bugs: MESOS-6035
> https://issues.apache.org/jira/browse/MESOS-6035
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In most cases, we just want to get the children cgroups instead of
> retrieve descendant cgroups recursively. We added an argument to
> `cgroups::get` to indicate whether retrieve cgroups recursively and
> make non-recursive retrieve become the default behaviour. This patch
> fixed some incorrect `TEST_CGROUPS_ROOT` checks as well.
> 
> 
> Diffs
> -
> 
>   src/linux/cgroups.hpp 047d3ea596660e704447c0f0c7b914a43c4a2187 
>   src/linux/cgroups.cpp f3232c009d04bb82701c0407b12abf999ab60a73 
>   src/tests/containerizer/cgroups_tests.cpp 
> 821c7860ab424bcfadbf597902e046bce9ba3044 
>   src/tests/mesos.cpp 62e8fcc6fa7bd856aab6148ca6e6cad66b436f04 
> 
> Diff: https://reviews.apache.org/r/51031/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Review Request 51210: Update mesos-executor name for Windows.

[Daniel Pravat]

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

Review request for mesos, Alex Naparu, Artem Harutyunyan, Alex Clemmer, Joseph 
Wu, and Michael Park.


Repository: mesos


Description
---

Update mesos-executor name for Windows.


Diffs
-

  src/slave/slave.cpp 3688f420e71ce9c79b7dda221f3f5c0042a9b3a1 

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


Testing
---


Thanks,

Daniel Pravat

Review Request 51213: Used `os::execlp` instead of `::execlp`.

[Daniel Pravat]

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

Review request for mesos, Alex Naparu, Artem Harutyunyan, Alex Clemmer, Joseph 
Wu, and Michael Park.


Repository: mesos


Description
---

Used `os::execlp` instead of `::execlp`.


Diffs
-

  src/slave/containerizer/mesos/launch.cpp 
2db8db5ac2f4614d406b950a1b4e8098a0d90e7b 

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


Testing
---


Thanks,

Daniel Pravat

Review Request 51211: Eumlate execlp functionality on Windows.

[Daniel Pravat]

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

Review request for mesos, Alex Naparu, Artem Harutyunyan, Alex Clemmer, Joseph 
Wu, and Michael Park.


Repository: mesos


Description
---

Eumlate execlp functionality on Windows.


Diffs
-

  3rdparty/stout/include/stout/os/posix/shell.hpp 
1d73ae5fa7182ba433eaee0cc0034af6b36d77db 
  3rdparty/stout/include/stout/os/windows/shell.hpp 
c6e141aba0abe2c7fe5410e867f7db47d632e765 
  src/slave/containerizer/mesos/launch.cpp 
2db8db5ac2f4614d406b950a1b4e8098a0d90e7b 

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


Testing
---


Thanks,

Daniel Pravat

Review Request 51217: Added `os::execlp' in `stout` library.

[Daniel Pravat]

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

Review request for mesos, Alex Naparu, Artem Harutyunyan, Alex Clemmer, Joseph 
Wu, and Michael Park.


Repository: mesos


Description
---

Added `os::execlp' in `stout` library.


Diffs
-

  3rdparty/stout/include/stout/os/posix/shell.hpp 
1d73ae5fa7182ba433eaee0cc0034af6b36d77db 
  3rdparty/stout/include/stout/os/windows/shell.hpp 
c6e141aba0abe2c7fe5410e867f7db47d632e765 

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


Testing
---


Thanks,

Daniel Pravat

Re: Review Request 51217: Added `os::execlp' in `stout` library.

[Daniel Pravat]

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

(Updated Aug. 18, 2016, 6:20 p.m.)


Review request for mesos, Alex Naparu, Artem Harutyunyan, Alex Clemmer, Joseph 
Wu, and Michael Park.


Repository: mesos


Description
---

Added `os::execlp' in `stout` library.


Diffs (updated)
-

  3rdparty/stout/include/stout/os/posix/shell.hpp 
1d73ae5fa7182ba433eaee0cc0034af6b36d77db 
  3rdparty/stout/include/stout/os/windows/shell.hpp 
c6e141aba0abe2c7fe5410e867f7db47d632e765 

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


Testing
---


Thanks,

Daniel Pravat

Re: Review Request 50751: Removed CgroupsPerfEventIsolatorProcess.

[Mesos ReviewBot]

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



Bad review!

Reviews applied: [50751, 50750, 50748, 50749, 50733, 50758, 49855, 49853, 
49852, 49851, 49854, 49850, 49849, 49828, 51185]

Error:
Circular dependency detected for review 51185.Please fix the 'depends_on' field.

- Mesos ReviewBot


On Aug. 18, 2016, 4:10 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50751/
> ---
> 
> (Updated Aug. 18, 2016, 4:10 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.
> 
> 
> Bugs: MESOS-5975
> https://issues.apache.org/jira/browse/MESOS-5975
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed CgroupsPerfEventIsolatorProcess.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 4362d472707193aa09343ef69f070e2d3efda324 
>   src/Makefile.am d389e02584cfb1a00067cd30de1364118d1c46c7 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 77a502f853e3e04ea8e274419544601778be9421 
>   src/slave/containerizer/mesos/isolators/cgroups/perf_event.hpp 
> 4abde12af68a26b94b3706cdb38bf9890d811039 
>   src/slave/containerizer/mesos/isolators/cgroups/perf_event.cpp 
> 31f35385691681ef5da14be747edfb5f57c5d05a 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/perf_event.cpp 
> PRE-CREATION 
>   src/tests/containerizer/isolator_tests.cpp 
> 05620d2411d464593cdb5aeaea10cb147047569b 
>   src/tests/environment.cpp 7f144f1763320aef1657a60b293d585b74a83367 
> 
> Diff: https://reviews.apache.org/r/50751/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 51215: Moved memory test helper to the common test helper.

[Mesos ReviewBot]

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



Bad patch!

Reviews applied: [51215]

Failed command: ./support/apply-review.sh -n -r 51215

Error:
2016-08-18 19:47:08 URL:https://reviews.apache.org/r/51215/diff/raw/ 
[13529/13529] -> "51215.patch" [1]
error: patch failed: src/Makefile.am:1929
error: src/Makefile.am: patch does not apply
error: patch failed: src/tests/CMakeLists.txt:20
error: src/tests/CMakeLists.txt: patch does not apply
error: patch failed: src/tests/cmake/MesosTestsConfigure.cmake:15
error: src/tests/cmake/MesosTestsConfigure.cmake: patch does not apply
error: src/tests/test_helper_main.cpp: does not exist in index

Full log: https://builds.apache.org/job/mesos-reviewbot/14856/console

- Mesos ReviewBot


On Aug. 18, 2016, 5:39 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51215/
> ---
> 
> (Updated Aug. 18, 2016, 5:39 p.m.)
> 
> 
> Review request for mesos and Benjamin Bannier.
> 
> 
> Bugs: MESOS-6053
> https://issues.apache.org/jira/browse/MESOS-6053
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Moved memory test helper to the common test helper.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am d389e02584cfb1a00067cd30de1364118d1c46c7 
>   src/tests/CMakeLists.txt 1ea8b2102753ae294bd75706ffaf08308e928acd 
>   src/tests/cmake/MesosTestsConfigure.cmake 
> 361032082c3a5f76b949dc7981f75b53e02d84f5 
>   src/tests/containerizer/CMakeLists.txt 
> 2c52e43a9deee90fa32693731d6ebedb5201bb1f 
>   src/tests/containerizer/memory_test_helper.hpp 
> 86a9b57c1872b2efe799b0dc61f3d3d73e4d134b 
>   src/tests/containerizer/memory_test_helper.cpp 
> e850b376ac61e2dedb07ea418cfeaf400cbba172 
>   src/tests/test_helper_main.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/51215/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Review Request 51220: Moved setns test helper to the common test helper.

[Jie Yu]

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

Review request for mesos and Benjamin Bannier.


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


Repository: mesos


Description
---

Moved setns test helper to the common test helper.


Diffs
-

  src/Makefile.am d389e02584cfb1a00067cd30de1364118d1c46c7 
  src/tests/CMakeLists.txt 1ea8b2102753ae294bd75706ffaf08308e928acd 
  src/tests/cmake/MesosTestsConfigure.cmake 
361032082c3a5f76b949dc7981f75b53e02d84f5 
  src/tests/containerizer/CMakeLists.txt 
2c52e43a9deee90fa32693731d6ebedb5201bb1f 
  src/tests/containerizer/ns_tests.cpp cd668ebb3b9461bee00dc338c288e5df6eb8fe31 
  src/tests/containerizer/setns_test_helper.hpp 
bd1a985856126faec07a8c1829b99443c8ec6f2b 
  src/tests/containerizer/setns_test_helper.cpp 
b7788ab7e476785c1181d5edfa0a70e084dd4e3c 
  src/tests/containerizer/setns_test_helper_main.cpp 
b8f20c3c046d7c40351ad89263e40e5e87e2f1f1 
  src/tests/test_helper_main.cpp 9eeba9cdef547770db7dc868377901631c884a47 

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


Testing
---

make check


Thanks,

Jie Yu

Re: Review Request 51171: Supported image provisioner singleton methods for global pointer.

[Jie Yu]

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



It was my bad. After looking at the code closely, I realized that we should 
probably use dependency injection for the new isolator. In other words,

`ImageVolumeIsolator::create` will take a flags as well as a 
`Shared`. Take a look at the GPU isolator see how to inject 
provisioner.


src/slave/containerizer/mesos/provisioner/provisioner.hpp (line 119)


No need for this. You can just save it using a file local global variable.



src/tests/containerizer/mesos_containerizer_tests.cpp (lines 774 - 775)


Fix indentation?



src/tests/containerizer/mesos_containerizer_tests.cpp (lines 778 - 780)


Ditto.


- Jie Yu


On Aug. 17, 2016, 8:44 a.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51171/
> ---
> 
> (Updated Aug. 17, 2016, 8:44 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Benjamin Mahler, Artem 
> Harutyunyan, Jie Yu, Kevin Klues, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Currently, the provisioner is constructed in mesos containerizer create
> as an 'owned' pointer. However, if we want to support image volumes as
> a isolator, the volume/image isolator needs the provisioner object. We
> have to make sure the provisioner is singliton and reachable in both
> mesos containerizer and the isolator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/provisioner.hpp 
> 8f378f6fea3fa4b6d1b7d079c7f280790b30589e 
>   src/slave/containerizer/mesos/provisioner/provisioner.cpp 
> ad19feccf08112f94d6514a134963c54ca541e88 
>   src/tests/containerizer/mesos_containerizer_tests.cpp 
> 4a49247a8b8c6995535a865266f85f2aa31fb4de 
> 
> Diff: https://reviews.apache.org/r/51171/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 44332: Fixed a typo in the HierarchicalAllocatorTest.CoarseGrained test.

[Alexander Rukletsov]

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

(Updated Aug. 18, 2016, 10:15 p.m.)


Review request for mesos, Benjamin Mahler and Joris Van Remoortere.


Repository: mesos


Description
---

See summary.


Diffs
-

  src/tests/hierarchical_allocator_tests.cpp 
3e4ad31925e1b815a74d67fa3962d23fa5bc89d1 

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


Testing
---

On Mac OS 10.10.4:

`make check`

`GTEST_FILTER="HierarchicalAllocatorTest.*" ./bin/mesos-tests.sh 
--gtest_repeat=100 --gtest_break_on_failure --gtest_shuffle`


Thanks,

Alexander Rukletsov

Re: Review Request 49063: Updating formatting of some lambda calls for readability.

[Michael Park]

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


Fix it, then Ship it!





src/master/http.cpp (lines 2838 - 2839)


Could you fix the alignment here?



src/slave/http.cpp (lines 924 - 960)


Let's leave this as-is.


- Michael Park


On June 27, 2016, 11:23 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49063/
> ---
> 
> (Updated June 27, 2016, 11:23 a.m.)
> 
> 
> Review request for mesos, Joerg Schad and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 88a11afeda6b96c337ab84fa1b32ab49724beabc 
>   src/slave/http.cpp 30006c2255ebfc6e2b150d518bc758eb3e94dbca 
> 
> Diff: https://reviews.apache.org/r/49063/diff/
> 
> 
> Testing
> ---
> 
> make check on Mac OS 10.10.4
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 49849: Implemented `CpuSubsystem`.

[Jie Yu]

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


Fix it, then Ship it!




Made some adjustment while committing. Please take a look and adjust the 
following patches. Thanks!


src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp (lines 34 - 36)


No need to prepend with leading underscore for these variables.



src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp (line 46)


I meant to have a static `create` method for each subsystem.



src/slave/containerizer/mesos/isolators/cgroups/subsystems/cpu.hpp (line 44)


I'd hide this constructor.


- Jie Yu


On Aug. 18, 2016, 4:09 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49849/
> ---
> 
> (Updated Aug. 18, 2016, 4:09 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.
> 
> 
> Bugs: MESOS-5042
> https://issues.apache.org/jira/browse/MESOS-5042
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented `CpuSubsystem`.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 4362d472707193aa09343ef69f070e2d3efda324 
>   src/Makefile.am d389e02584cfb1a00067cd30de1364118d1c46c7 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
> 7e205a3eea4ad89faccf564bdb2ad0041fdee249 
>   src/slave/containerizer/mesos/isolators/cgroups/constants.hpp 
> c45d88092f3fe497373dfeaa8346aef9126c7b8b 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp 
> 5f52a076a1fa3a21d886cb961ddeed5046a38d7c 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp 
> a30ecafcbecc9d3b6eeea2b04dcb4d278750af41 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/cpu.hpp 
> PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/cpu.cpp 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/49849/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 51217: Added `os::execlp' in `stout` library.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [51210, 51217]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker_build.sh

- Mesos ReviewBot


On Aug. 18, 2016, 6:20 p.m., Daniel Pravat wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51217/
> ---
> 
> (Updated Aug. 18, 2016, 6:20 p.m.)
> 
> 
> Review request for mesos, Alex Naparu, Artem Harutyunyan, Alex Clemmer, 
> Joseph Wu, and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added `os::execlp' in `stout` library.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/posix/shell.hpp 
> 1d73ae5fa7182ba433eaee0cc0034af6b36d77db 
>   3rdparty/stout/include/stout/os/windows/shell.hpp 
> c6e141aba0abe2c7fe5410e867f7db47d632e765 
> 
> Diff: https://reviews.apache.org/r/51217/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Daniel Pravat
> 
>

Re: Review Request 50540: Add systemd watchdog support.

[Lawrence Wu]

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

(Updated Aug. 18, 2016, 11:19 p.m.)


Review request for mesos, David Robinson, Ian Downes, and Jie Yu.


Changes
---

address issues, write second test


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


Repository: mesos


Description
---

Add systemd watchdog support.


Diffs (updated)
-

  configure.ac d2136909b7305498ae901a5ea00133142b77f9e6 
  src/Makefile.am 599ebbef6d164fb2a530b55427ddabb5cd607634 
  src/linux/systemd.hpp 91134f1d4b100759e45931bd09ca4e1e1aeaaf8a 
  src/linux/systemd.cpp 619aa2778da5f99d3a078a8e1208bdaa9dc77581 
  src/slave/main.cpp 4624392d30cf391015dcd63f447fe2414a47a16a 
  src/tests/linux/systemd_test_helper.hpp PRE-CREATION 
  src/tests/linux/systemd_test_helper.cpp PRE-CREATION 
  src/tests/linux/systemd_test_helper_main.cpp PRE-CREATION 
  src/tests/linux/systemd_tests.cpp PRE-CREATION 

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


Testing (updated)
---

Tested by sending SIGSTOP to running mesos and verifying via journalctl that it 
was killed by the watchdog.

The test I wrote for this does the following:
- build up a unit file as a string and create a unit file in 
/etc/systemd/system/systemd-test-helper.service
- reload the systemd daemon and start the newly discovered helper service
- wait a bit (30s) to make sure the watchdog has had a chance to kill the 
service
- use systemctl status systemd-test-helper to check that the service is still 
running
- clean up the unit file.


Thanks,

Lawrence Wu

Re: Review Request 50540: Add systemd watchdog support.

[Lawrence Wu]

> On Aug. 17, 2016, 12:44 a.m., Jie Yu wrote:
> > High level question: is this useful? I imagine watchdog logic for agent is 
> > to call sd_notify from `Slave` actor (similar to Master) so that we know 
> > `Slave` is still functional. I think this is the right way to detect 
> > hanging.
> > 
> > The systemd library code should provide primitive to the Agent code so that 
> > it does not call raw `sd_notify` directly.
> 
> Jie Yu wrote:
> Scratch that. I misunderstood what Watchdog is for. But in general, i 
> don't get why we need that. Sound like not a very effective way to detect 
> hanging.

We (Twitter) currently use monit to monitor mesos, but we'd like to use the 
systemd watchdog to do the monitoring and detect hangs instead. Could you 
elaborate on why you think it is not very effective? The way I see it, there is 
very little extra code and performance overhead (sd_notify is a trivial 
operation), and we gain reliability on the Mesos side.


> On Aug. 17, 2016, 12:44 a.m., Jie Yu wrote:
> > src/linux/systemd.cpp, line 207
> > 
> >
> > Who will delete the Watchdog? I think you should use a `managed` spawn. 
> > See `spawn` definition.

Reading the definition for a managed spawn, it looks like we do not want a 
managed process -- that will get garbage collected. We want the watchdog to be 
alive for the entire lifetime of the process. I added a comment clarifying this.


> On Aug. 17, 2016, 12:44 a.m., Jie Yu wrote:
> > src/linux/systemd.cpp, line 208
> > 
> >
> > Why create the watch dog actor if WATCHDOG_USEC is not set?

this should be fixed now.


> On Aug. 17, 2016, 12:44 a.m., Jie Yu wrote:
> > src/linux/systemd.hpp, line 32
> > 
> >
> > We now does not use `using namespace` any more. Please include them one 
> > by one using `using xxx;` clauses.

Done.


> On Aug. 17, 2016, 12:44 a.m., Jie Yu wrote:
> > src/linux/systemd.cpp, line 203
> > 
> >
> > Hum, this also could be the case where WATCHDOG_USEC is not set, right? 
> > The error here sounds unintuitive.
> > 
> > I'd suggest we check env exists or not, and then parse it.
> > 
> > ```
> > if (watchdogUsec.isNone()) {
> > } else if (watchDog.isError()) {
> > }
> > ```

Done.


> On Aug. 17, 2016, 12:44 a.m., Jie Yu wrote:
> > configure.ac, lines 615-616
> > 
> >
> > Looks like you haven't checked headers. You need dev headers, right?

I'm not too sure, but I think we just need this lib for sd_notify. I tried 
building this on a machine without the systemd dev headers and it worked, so I 
don't think dev headers are necessary.


- Lawrence


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


On Aug. 18, 2016, 11:19 p.m., Lawrence Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50540/
> ---
> 
> (Updated Aug. 18, 2016, 11:19 p.m.)
> 
> 
> Review request for mesos, David Robinson, Ian Downes, and Jie Yu.
> 
> 
> Bugs: MESOS-5376
> https://issues.apache.org/jira/browse/MESOS-5376
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add systemd watchdog support.
> 
> 
> Diffs
> -
> 
>   configure.ac d2136909b7305498ae901a5ea00133142b77f9e6 
>   src/Makefile.am 599ebbef6d164fb2a530b55427ddabb5cd607634 
>   src/linux/systemd.hpp 91134f1d4b100759e45931bd09ca4e1e1aeaaf8a 
>   src/linux/systemd.cpp 619aa2778da5f99d3a078a8e1208bdaa9dc77581 
>   src/slave/main.cpp 4624392d30cf391015dcd63f447fe2414a47a16a 
>   src/tests/linux/systemd_test_helper.hpp PRE-CREATION 
>   src/tests/linux/systemd_test_helper.cpp PRE-CREATION 
>   src/tests/linux/systemd_test_helper_main.cpp PRE-CREATION 
>   src/tests/linux/systemd_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/50540/diff/
> 
> 
> Testing
> ---
> 
> Tested by sending SIGSTOP to running mesos and verifying via journalctl that 
> it was killed by the watchdog.
> 
> The test I wrote for this does the following:
> - build up a unit file as a string and create a unit file in 
> /etc/systemd/system/systemd-test-helper.service
> - reload the systemd daemon and start the newly discovered helper service
> - wait a bit (30s) to make sure the watchdog has had a chance to kill the 
> service
> - use systemctl status systemd-test-helper to check that the service is still 
> running
> - clean up the unit f

Re: Review Request 51042: Added high-level function to set capabilties on process exec'd next.

[Jie Yu]

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



Looking at what runC does:
https://github.com/opencontainers/runc/blob/master/libcontainer/init_linux.go#L119-L144

This sounds pretty clean, why we need such a helper method?

- Jie Yu


On Aug. 12, 2016, 4:49 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51042/
> ---
> 
> (Updated Aug. 12, 2016, 4:49 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-5051
> https://issues.apache.org/jira/browse/MESOS-5051
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/linux/capabilities.hpp fd7a571a62e3bbdd8a0453f800bf622457891bf8 
>   src/linux/capabilities.cpp 5f21dd42dd94af907450aabb1900d1f73816b31c 
> 
> Diff: https://reviews.apache.org/r/51042/diff/
> 
> 
> Testing
> ---
> 
> make check and sudo make check (Debian jessie, gcc-4.9.2, w/o optimizations)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 51042: Added high-level function to set capabilties on process exec'd next.

[Jie Yu]

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




src/linux/capabilities.cpp (lines 297 - 313)


Should this just be a `set`?

Also, we should just assume we have SETPCAP as agent is typically running 
as root.


- Jie Yu


On Aug. 12, 2016, 4:49 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51042/
> ---
> 
> (Updated Aug. 12, 2016, 4:49 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-5051
> https://issues.apache.org/jira/browse/MESOS-5051
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/linux/capabilities.hpp fd7a571a62e3bbdd8a0453f800bf622457891bf8 
>   src/linux/capabilities.cpp 5f21dd42dd94af907450aabb1900d1f73816b31c 
> 
> Diff: https://reviews.apache.org/r/51042/diff/
> 
> 
> Testing
> ---
> 
> make check and sudo make check (Debian jessie, gcc-4.9.2, w/o optimizations)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 51172: Implemented the volume/image isolator.

[Guangya Liu]

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




src/slave/containerizer/mesos/isolators/volume/image.hpp (line 28)


s/docker runtime/volume image



src/slave/containerizer/mesos/isolators/volume/image.cpp (lines 49 - 50)


kill this



src/slave/containerizer/mesos/isolators/volume/image.cpp (line 94)


s/int/size_t



src/slave/containerizer/mesos/isolators/volume/image.cpp (lines 97 - 99)


I think that we also need to check if the volume has source here, and just 
ignore such volumes as the volume who has sources can only be handled by docker 
volume isolator.

```
if (volume.has_source()) {
  ...
}
```



src/slave/containerizer/mesos/isolators/volume/image.cpp (line 160)


kill this



src/slave/containerizer/mesos/isolators/volume/image.cpp (line 165)


kill this


- Guangya Liu


On 八月 17, 2016, 8:44 a.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51172/
> ---
> 
> (Updated 八月 17, 2016, 8:44 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Benjamin Mahler, Artem 
> Harutyunyan, Jie Yu, Kevin Klues, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented the volume/image isolator.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 4362d472707193aa09343ef69f070e2d3efda324 
>   src/Makefile.am d389e02584cfb1a00067cd30de1364118d1c46c7 
>   src/slave/containerizer/mesos/isolators/volume/image.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/volume/image.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/51172/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 49234: Code cleanup for docker/runtime isolator.

[Guangya Liu]

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

(Updated 八月 18, 2016, 11:38 p.m.)


Review request for mesos, Gilbert Song and Jie Yu.


Summary (updated)
-

Code cleanup for docker/runtime isolator.


Repository: mesos


Description (updated)
---

1) Removed DockerRuntimeIsolatorProcess::recover from docker/runtime.
2) Removed unused namespaces.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/docker/runtime.hpp 
ac0397f65bb2a675ad1eae0f7cfa95f10145fc63 
  src/slave/containerizer/mesos/isolators/docker/runtime.cpp 
b589cd691ae6aacd2dcd00878e43d58f15abfe11 

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


Testing
---

make
make check


Thanks,

Guangya Liu

Re: Review Request 51220: Moved setns test helper to the common test helper.

[Mesos ReviewBot]

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



Bad patch!

Reviews applied: [51220, 51215]

Failed command: ./support/apply-review.sh -n -r 51215

Error:
2016-08-18 23:43:25 URL:https://reviews.apache.org/r/51215/diff/raw/ 
[13529/13529] -> "51215.patch" [1]
error: patch failed: src/Makefile.am:1929
error: src/Makefile.am: patch does not apply
error: patch failed: src/tests/CMakeLists.txt:20
error: src/tests/CMakeLists.txt: patch does not apply
error: patch failed: src/tests/cmake/MesosTestsConfigure.cmake:15
error: src/tests/cmake/MesosTestsConfigure.cmake: patch does not apply
error: src/tests/test_helper_main.cpp: does not exist in index

Full log: https://builds.apache.org/job/mesos-reviewbot/14859/console

- Mesos ReviewBot


On Aug. 18, 2016, 8:47 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51220/
> ---
> 
> (Updated Aug. 18, 2016, 8:47 p.m.)
> 
> 
> Review request for mesos and Benjamin Bannier.
> 
> 
> Bugs: MESOS-6053
> https://issues.apache.org/jira/browse/MESOS-6053
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Moved setns test helper to the common test helper.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am d389e02584cfb1a00067cd30de1364118d1c46c7 
>   src/tests/CMakeLists.txt 1ea8b2102753ae294bd75706ffaf08308e928acd 
>   src/tests/cmake/MesosTestsConfigure.cmake 
> 361032082c3a5f76b949dc7981f75b53e02d84f5 
>   src/tests/containerizer/CMakeLists.txt 
> 2c52e43a9deee90fa32693731d6ebedb5201bb1f 
>   src/tests/containerizer/ns_tests.cpp 
> cd668ebb3b9461bee00dc338c288e5df6eb8fe31 
>   src/tests/containerizer/setns_test_helper.hpp 
> bd1a985856126faec07a8c1829b99443c8ec6f2b 
>   src/tests/containerizer/setns_test_helper.cpp 
> b7788ab7e476785c1181d5edfa0a70e084dd4e3c 
>   src/tests/containerizer/setns_test_helper_main.cpp 
> b8f20c3c046d7c40351ad89263e40e5e87e2f1f1 
>   src/tests/test_helper_main.cpp 9eeba9cdef547770db7dc868377901631c884a47 
> 
> Diff: https://reviews.apache.org/r/51220/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 50540: Add systemd watchdog support.

[Lawrence Wu]

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

(Updated Aug. 19, 2016, 12:09 a.m.)


Review request for mesos, David Robinson, Ian Downes, and Jie Yu.


Changes
---

use os::rm instead of subprocess unlink


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


Repository: mesos


Description
---

Add systemd watchdog support.


Diffs (updated)
-

  configure.ac d2136909b7305498ae901a5ea00133142b77f9e6 
  src/Makefile.am 599ebbef6d164fb2a530b55427ddabb5cd607634 
  src/linux/systemd.hpp 91134f1d4b100759e45931bd09ca4e1e1aeaaf8a 
  src/linux/systemd.cpp 619aa2778da5f99d3a078a8e1208bdaa9dc77581 
  src/slave/main.cpp 4624392d30cf391015dcd63f447fe2414a47a16a 
  src/tests/linux/systemd_test_helper.hpp PRE-CREATION 
  src/tests/linux/systemd_test_helper.cpp PRE-CREATION 
  src/tests/linux/systemd_test_helper_main.cpp PRE-CREATION 
  src/tests/linux/systemd_tests.cpp PRE-CREATION 

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


Testing
---

Tested by sending SIGSTOP to running mesos and verifying via journalctl that it 
was killed by the watchdog.

The test I wrote for this does the following:
- build up a unit file as a string and create a unit file in 
/etc/systemd/system/systemd-test-helper.service
- reload the systemd daemon and start the newly discovered helper service
- wait a bit (30s) to make sure the watchdog has had a chance to kill the 
service
- use systemctl status systemd-test-helper to check that the service is still 
running
- clean up the unit file.


Thanks,

Lawrence Wu

Re: Review Request 49063: Updating formatting of some lambda calls for readability.

[Mesos ReviewBot]

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



Bad patch!

Reviews applied: [49063]

Failed command: ./support/apply-review.sh -n -r 49063

Error:
2016-08-19 00:40:11 URL:https://reviews.apache.org/r/49063/diff/raw/ 
[9666/9666] -> "49063.patch" [1]
error: patch failed: src/master/http.cpp:2830
error: src/master/http.cpp: patch does not apply
error: patch failed: src/slave/http.cpp:857
error: src/slave/http.cpp: patch does not apply

Full log: https://builds.apache.org/job/mesos-reviewbot/14860/console

- Mesos ReviewBot


On June 27, 2016, 11:23 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49063/
> ---
> 
> (Updated June 27, 2016, 11:23 a.m.)
> 
> 
> Review request for mesos, Joerg Schad and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 88a11afeda6b96c337ab84fa1b32ab49724beabc 
>   src/slave/http.cpp 30006c2255ebfc6e2b150d518bc758eb3e94dbca 
> 
> Diff: https://reviews.apache.org/r/49063/diff/
> 
> 
> Testing
> ---
> 
> make check on Mac OS 10.10.4
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 51027: Track allocation candidates to bound allocator.

[Jiang Yan Xu]

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




src/master/allocator/mesos/hierarchical.hpp (line 216)


Change the comment to:

```
// Allocate resources from the allocation candidates.
```



src/master/allocator/mesos/hierarchical.hpp (lines 219 - 220)


Kill this. We don't define this method anymore.



src/master/allocator/mesos/hierarchical.hpp (lines 222 - 223)


We need to change this per the comments below.



src/master/allocator/mesos/hierarchical.hpp (lines 382 - 383)


We should add comments for these members.



src/master/allocator/mesos/hierarchical.cpp (lines 782 - 786)


Here it's appropriate, particularly given the comment above, that we run 
allocation synchronously.

Add a comment similar to the one in `setQuota` on why we do it 
synchronously.



src/master/allocator/mesos/hierarchical.cpp (line 1107)


s/explicitly/synchronously/



src/master/allocator/mesos/hierarchical.cpp (lines 1109 - 1113)


Here it's appropriate, particularly given the comment above, that we run 
allocation synchronously.



src/master/allocator/mesos/hierarchical.cpp (line 1136)


s/explicitly/synchronously/



src/master/allocator/mesos/hierarchical.cpp (lines 1138 - 1142)


Ditto.



src/master/allocator/mesos/hierarchical.cpp (lines 1177 - 1181)


Ditto.



src/master/allocator/mesos/hierarchical.cpp (line 1209)


We need some comments here.

```
We run the allocation synchronously here instead of dispatching it so we 
don't delay a batched allocation if the allocator is backed up.

TODO: This still does not guarantee that at least one allocation is run per 
batch interval. Consider having the allocator run allocations synchronously 
when handling events if no allocation is run for the past batch interval. 
```



src/master/allocator/mesos/hierarchical.cpp (line 1214)






src/master/allocator/mesos/hierarchical.cpp (line 1227)


Here we can just rename the method we call as: `_allocate()`. Note that now 
that we keep track of a member `allocationCandidates` we don't have to pass 
them around. If we don't pass them around, we can't use the argument as what 
distinguishes the overloaded `allocate()` methods.

I think it's pretty natural to have `void _allocate();` as a private helper.



src/master/allocator/mesos/hierarchical.cpp (line 1582)


Similarly here, we don't need the argument in `deallocate()`.


- Jiang Yan Xu


On Aug. 17, 2016, 11:12 a.m., Jacob Janco wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51027/
> ---
> 
> (Updated Aug. 17, 2016, 11:12 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, James Peach, Klaus Ma, and Jiang 
> Yan Xu.
> 
> 
> Bugs: MESOS-3157
> https://issues.apache.org/jira/browse/MESOS-3157
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> - Triggered allocations dispatch allocate() only
>   if there is no pending allocation in the queue.
> - Allocation candidates are accumulated and only
>   cleared when enqueued allocations are processed.
>   
>   - carrying over work from https://reviews.apache.org/r/41658/ and added the 
> previous reviewers
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.hpp 
> bdbc6d3b5b959990538f4e3b7b1a3b031d9aea05 
>   src/master/allocator/mesos/hierarchical.cpp 
> 234ef98529964a0b6d3f132426a6c8ccbb1263ee 
> 
> Diff: https://reviews.apache.org/r/51027/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> note: check without filters depends on https://reviews.apache.org/r/51028
> 
> With new benchmark https://reviews.apache.org/r/49617: 
> Sample output without 51027:
> [ RUN  ] 
> SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.FrameworkFailover/22
> Using 1 agents and 3000 frameworks
> Added 3000 frameworks in 57251us
> Added 1 agents in 3.2134535333mins
> allocator settled after  1.6123603833mins
> [   O

Re: Review Request 51027: Track allocation candidates to bound allocator.

[Jiang Yan Xu]

> On Aug. 17, 2016, 5:38 p.m., Guangya Liu wrote:
> > Can you please also clarify some difference with 
> > https://reviews.apache.org/r/41658/, as here the work is carry from 
> > https://reviews.apache.org/r/41658/ , I think that this is more simple and 
> > clear compared with counter in https://reviews.apache.org/r/41658/ with a 
> > boolean here.

This is exactly a rework based on comments in 
https://reviews.apache.org/r/41658/.


- Jiang Yan


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


On Aug. 17, 2016, 11:12 a.m., Jacob Janco wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51027/
> ---
> 
> (Updated Aug. 17, 2016, 11:12 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, James Peach, Klaus Ma, and Jiang 
> Yan Xu.
> 
> 
> Bugs: MESOS-3157
> https://issues.apache.org/jira/browse/MESOS-3157
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> - Triggered allocations dispatch allocate() only
>   if there is no pending allocation in the queue.
> - Allocation candidates are accumulated and only
>   cleared when enqueued allocations are processed.
>   
>   - carrying over work from https://reviews.apache.org/r/41658/ and added the 
> previous reviewers
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.hpp 
> bdbc6d3b5b959990538f4e3b7b1a3b031d9aea05 
>   src/master/allocator/mesos/hierarchical.cpp 
> 234ef98529964a0b6d3f132426a6c8ccbb1263ee 
> 
> Diff: https://reviews.apache.org/r/51027/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> note: check without filters depends on https://reviews.apache.org/r/51028
> 
> With new benchmark https://reviews.apache.org/r/49617: 
> Sample output without 51027:
> [ RUN  ] 
> SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.FrameworkFailover/22
> Using 1 agents and 3000 frameworks
> Added 3000 frameworks in 57251us
> Added 1 agents in 3.2134535333mins
> allocator settled after  1.6123603833mins
> [   OK ] 
> SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.FrameworkFailover/22
>  (290578 ms)
> 
> Sample output with 51027:
> [ RUN  ] 
> SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.FrameworkFailover/22
> Using 1 agents and 3000 frameworks
> Added 3000 frameworks in 39817us
> Added 1 agents in 3.2286054167mins
> allocator settled after  25.525654secs
> [   OK ] 
> SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.FrameworkFailover/22
>  (220137 ms)
> 
> 
> Thanks,
> 
> Jacob Janco
> 
>

Re: Review Request 51027: Track allocation candidates to bound allocator.

[Jiang Yan Xu]

> On Aug. 17, 2016, 5:36 p.m., Guangya Liu wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 29
> > 
> >
> > move this right before `#include `

Also move `#include ` with it.


- Jiang Yan


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


On Aug. 17, 2016, 11:12 a.m., Jacob Janco wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51027/
> ---
> 
> (Updated Aug. 17, 2016, 11:12 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, James Peach, Klaus Ma, and Jiang 
> Yan Xu.
> 
> 
> Bugs: MESOS-3157
> https://issues.apache.org/jira/browse/MESOS-3157
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> - Triggered allocations dispatch allocate() only
>   if there is no pending allocation in the queue.
> - Allocation candidates are accumulated and only
>   cleared when enqueued allocations are processed.
>   
>   - carrying over work from https://reviews.apache.org/r/41658/ and added the 
> previous reviewers
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.hpp 
> bdbc6d3b5b959990538f4e3b7b1a3b031d9aea05 
>   src/master/allocator/mesos/hierarchical.cpp 
> 234ef98529964a0b6d3f132426a6c8ccbb1263ee 
> 
> Diff: https://reviews.apache.org/r/51027/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> note: check without filters depends on https://reviews.apache.org/r/51028
> 
> With new benchmark https://reviews.apache.org/r/49617: 
> Sample output without 51027:
> [ RUN  ] 
> SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.FrameworkFailover/22
> Using 1 agents and 3000 frameworks
> Added 3000 frameworks in 57251us
> Added 1 agents in 3.2134535333mins
> allocator settled after  1.6123603833mins
> [   OK ] 
> SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.FrameworkFailover/22
>  (290578 ms)
> 
> Sample output with 51027:
> [ RUN  ] 
> SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.FrameworkFailover/22
> Using 1 agents and 3000 frameworks
> Added 3000 frameworks in 39817us
> Added 1 agents in 3.2286054167mins
> allocator settled after  25.525654secs
> [   OK ] 
> SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.FrameworkFailover/22
>  (220137 ms)
> 
> 
> Thanks,
> 
> Jacob Janco
> 
>

Re: Review Request 50750: Removed CgroupsNetClsIsolatorProcess.

[Qian Zhang]

> On Aug. 17, 2016, 4:06 p.m., Qian Zhang wrote:
> > src/tests/environment.cpp, line 438
> > 
> >
> > Here can we change it to the following?
> > ```
> > if (matches(test, "ROOT_CGROUPS_Isolate") ||
> > matches(test, "ROOT_CGROUPS_ContainerStatus") {
> > ```
> > In this way, I think you can still name the test class as 
> > `CgroupsIsolatorTest` which will be consistent with other cgroup's related 
> > tests.
> 
> haosdent huang wrote:
> I found `ROOT_CGROUPS_Isolate` and `ROOT_CGROUPS_ContainerStatus` are not 
> exactly enough here because it only test net_cls. Should we use 
> `ROOT_CGROUPS_NetSubsystemIsolate` and 
> `ROOT_CGROUPS_NetSubsystemContainerStatus` respectively.
> 
> Qian Zhang wrote:
> I think they should be `ROOT_CGROUPS_NetClsIsolate` (rather than 
> `ROOT_CGROUPS_Isolate`) and `ROOT_CGROUPS_ContainerStatus` which are the 
> original test names. So can you please let me know if 
> `ROOT_CGROUPS_NetClsIsolate` and `ROOT_CGROUPS_ContainerStatus` are enough? I 
> think it should be OK since we are doing the similar thing in 
> `PerfCPUCyclesFilter::disable()`, right?
> 
> haosdent huang wrote:
> `CgroupsIsolatorTest.ROOT_CGROUPS_ContainerStatus` sounds not exactly, 
> how about `CgroupsIsolatorTest.ROOT_CGROUPS_ContainerNetClsStatus`?

What about name it as `ROOT_CGROUPS_NetClsContainerStatus` which seems more 
consistent with `ROOT_CGROUPS_NetClsIsolate`?


- Qian


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


On Aug. 19, 2016, 12:10 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50750/
> ---
> 
> (Updated Aug. 19, 2016, 12:10 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.
> 
> 
> Bugs: MESOS-5976
> https://issues.apache.org/jira/browse/MESOS-5976
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed CgroupsNetClsIsolatorProcess.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 4362d472707193aa09343ef69f070e2d3efda324 
>   src/Makefile.am d389e02584cfb1a00067cd30de1364118d1c46c7 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 77a502f853e3e04ea8e274419544601778be9421 
>   src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp 
> b3fd8c85476bf46368bd79f052b7923ad9d32199 
>   src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp 
> bebefbba7153d4b0e9a8d7179cfb642e6e802bea 
>   src/tests/containerizer/isolator_tests.cpp 
> 05620d2411d464593cdb5aeaea10cb147047569b 
>   src/tests/environment.cpp 7f144f1763320aef1657a60b293d585b74a83367 
> 
> Diff: https://reviews.apache.org/r/50750/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 49234: Code cleanup for docker/runtime isolator.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [49234]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker_build.sh

- Mesos ReviewBot


On Aug. 18, 2016, 11:38 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49234/
> ---
> 
> (Updated Aug. 18, 2016, 11:38 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> 1) Removed DockerRuntimeIsolatorProcess::recover from docker/runtime.
> 2) Removed unused namespaces.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/docker/runtime.hpp 
> ac0397f65bb2a675ad1eae0f7cfa95f10145fc63 
>   src/slave/containerizer/mesos/isolators/docker/runtime.cpp 
> b589cd691ae6aacd2dcd00878e43d58f15abfe11 
> 
> Diff: https://reviews.apache.org/r/49234/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 50750: Removed CgroupsNetClsIsolatorProcess.

[haosdent huang]

> On Aug. 17, 2016, 8:06 a.m., Qian Zhang wrote:
> > src/tests/environment.cpp, line 438
> > 
> >
> > Here can we change it to the following?
> > ```
> > if (matches(test, "ROOT_CGROUPS_Isolate") ||
> > matches(test, "ROOT_CGROUPS_ContainerStatus") {
> > ```
> > In this way, I think you can still name the test class as 
> > `CgroupsIsolatorTest` which will be consistent with other cgroup's related 
> > tests.
> 
> haosdent huang wrote:
> I found `ROOT_CGROUPS_Isolate` and `ROOT_CGROUPS_ContainerStatus` are not 
> exactly enough here because it only test net_cls. Should we use 
> `ROOT_CGROUPS_NetSubsystemIsolate` and 
> `ROOT_CGROUPS_NetSubsystemContainerStatus` respectively.
> 
> Qian Zhang wrote:
> I think they should be `ROOT_CGROUPS_NetClsIsolate` (rather than 
> `ROOT_CGROUPS_Isolate`) and `ROOT_CGROUPS_ContainerStatus` which are the 
> original test names. So can you please let me know if 
> `ROOT_CGROUPS_NetClsIsolate` and `ROOT_CGROUPS_ContainerStatus` are enough? I 
> think it should be OK since we are doing the similar thing in 
> `PerfCPUCyclesFilter::disable()`, right?
> 
> haosdent huang wrote:
> `CgroupsIsolatorTest.ROOT_CGROUPS_ContainerStatus` sounds not exactly, 
> how about `CgroupsIsolatorTest.ROOT_CGROUPS_ContainerNetClsStatus`?
> 
> Qian Zhang wrote:
> What about name it as `ROOT_CGROUPS_NetClsContainerStatus` which seems 
> more consistent with `ROOT_CGROUPS_NetClsIsolate`?

Make sense, let me update.


- haosdent


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


On Aug. 18, 2016, 4:10 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50750/
> ---
> 
> (Updated Aug. 18, 2016, 4:10 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.
> 
> 
> Bugs: MESOS-5976
> https://issues.apache.org/jira/browse/MESOS-5976
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed CgroupsNetClsIsolatorProcess.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 4362d472707193aa09343ef69f070e2d3efda324 
>   src/Makefile.am d389e02584cfb1a00067cd30de1364118d1c46c7 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 77a502f853e3e04ea8e274419544601778be9421 
>   src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp 
> b3fd8c85476bf46368bd79f052b7923ad9d32199 
>   src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp 
> bebefbba7153d4b0e9a8d7179cfb642e6e802bea 
>   src/tests/containerizer/isolator_tests.cpp 
> 05620d2411d464593cdb5aeaea10cb147047569b 
>   src/tests/environment.cpp 7f144f1763320aef1657a60b293d585b74a83367 
> 
> Diff: https://reviews.apache.org/r/50750/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 51068: Prevent memory leaks

[Aaron Wood]

> On Aug. 16, 2016, 11:35 p.m., Benjamin Mahler wrote:
> > Hey Aaron, I was not able to identify the leaks you were addressing. Also 
> > there seems to be some object lifetime issues introduced with this patch.

It looks like I was wrong about this, I had thought the copy constructor of 
tuple was getting called when it wasn't the copy constructor at all. I did some 
more testing with this and found that the original version behaves properly.
I think we can close this out.


- Aaron


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


On Aug. 17, 2016, 6:21 p.m., Aaron Wood wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51068/
> ---
> 
> (Updated Aug. 17, 2016, 6:21 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This should prevent any of the promises that are created in the various 
> ZookeeperProcess class methods from leaking memory.
> 
> 
> Diffs
> -
> 
>   docs/contributors.yaml 3f06000 
>   src/zookeeper/zookeeper.cpp e105377 
> 
> Diff: https://reviews.apache.org/r/51068/diff/
> 
> 
> Testing
> ---
> 
> make && make check
> 
> 
> Thanks,
> 
> Aaron Wood
> 
>