You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Ian Downes <ia...@gmail.com> on 2015/03/03 19:17:41 UTC

Re: Review Request 31250: Expose the number of processes and threads in a container when cgroup is enabled.

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

Ship it!



src/slave/containerizer/isolators/cgroups/cpushare.cpp
<https://reviews.apache.org/r/31250/#comment121896>

    s/opeation/operation/



src/tests/isolator_tests.cpp
<https://reviews.apache.org/r/31250/#comment121617>

    give it some memory too?



src/tests/isolator_tests.cpp
<https://reviews.apache.org/r/31250/#comment121619>

    Can you use Subprocess::PIPE here so the process will terminate if the parent exits?



src/tests/isolator_tests.cpp
<https://reviews.apache.org/r/31250/#comment121620>

    Is the earlier test for pids=[], tids=[] redundant?



src/tests/isolator_tests.cpp
<https://reviews.apache.org/r/31250/#comment121890>

    s/Process/Process count/


- Ian Downes


On Feb. 23, 2015, 4:45 p.m., Chi Zhang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31250/
> -----------------------------------------------------------
> 
> (Updated Feb. 23, 2015, 4:45 p.m.)
> 
> 
> Review request for mesos and Ian Downes.
> 
> 
> Bugs: mesos-2103
>     https://issues.apache.org/jira/browse/mesos-2103
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Ian, as discussed, I put the logic in cpu isolator for now until linux launcher sees more and more need to report isolator-independent cgroup usage.
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto 9df972d750ce1e4a81d2e96cc508d6f83cad2fc8 
>   src/slave/containerizer/isolators/cgroups/cpushare.cpp b6df23963f20e5f14f484d449733655d46cd6884 
>   src/tests/isolator_tests.cpp f7323965b94543bfda78afc4f60fc677f4900ecd 
> 
> Diff: https://reviews.apache.org/r/31250/diff/
> 
> 
> Testing
> -------
> 
> added a new test. the test is end-to-end so that future replacement of the actual logic won't require a change.
> 
> 
> Thanks,
> 
> Chi Zhang
> 
>


Re: Review Request 31250: Expose the number of processes and threads in a container when cgroup is enabled.

Posted by Chi Zhang <ch...@gmail.com>.

> On March 3, 2015, 6:17 p.m., Ian Downes wrote:
> > src/tests/isolator_tests.cpp, lines 607-611
> > <https://reviews.apache.org/r/31250/diff/2/?file=873351#file873351line607>
> >
> >     Is the earlier test for pids=[], tids=[] redundant?

CpuIsolator creates a cgroup for the container in ::create, so the check above was intended to make sure it's empty right after creation. I've added a comment there.

This check could make sure nothing happens to the cgroup between prepare and isolate (the forking, and so on), and with the check right after isolate that isolate increases the number of pids and tids.


- Chi


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


On March 3, 2015, 7:37 p.m., Chi Zhang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31250/
> -----------------------------------------------------------
> 
> (Updated March 3, 2015, 7:37 p.m.)
> 
> 
> Review request for mesos and Ian Downes.
> 
> 
> Bugs: mesos-2103
>     https://issues.apache.org/jira/browse/mesos-2103
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Ian, as discussed, I put the logic in cpu isolator for now until linux launcher sees more and more need to report isolator-independent cgroup usage.
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto 9df972d750ce1e4a81d2e96cc508d6f83cad2fc8 
>   src/slave/containerizer/isolators/cgroups/cpushare.cpp d4967687241ddf50a85beb961d691a8974f24fea 
>   src/tests/isolator_tests.cpp f7323965b94543bfda78afc4f60fc677f4900ecd 
> 
> Diff: https://reviews.apache.org/r/31250/diff/
> 
> 
> Testing
> -------
> 
> added a new test. the test is end-to-end so that future replacement of the actual logic won't require a change.
> 
> 
> Thanks,
> 
> Chi Zhang
> 
>


Re: Review Request 31250: Expose the number of processes and threads in a container when cgroup is enabled.

Posted by Chi Zhang <ch...@gmail.com>.

> On March 3, 2015, 6:17 p.m., Ian Downes wrote:
> > src/tests/isolator_tests.cpp, lines 592-594
> > <https://reviews.apache.org/r/31250/diff/2/?file=873351#file873351line592>
> >
> >     Can you use Subprocess::PIPE here so the process will terminate if the parent exits?

dropped as per discussion offline.


- Chi


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


On March 4, 2015, 1:47 a.m., Chi Zhang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31250/
> -----------------------------------------------------------
> 
> (Updated March 4, 2015, 1:47 a.m.)
> 
> 
> Review request for mesos and Ian Downes.
> 
> 
> Bugs: mesos-2103
>     https://issues.apache.org/jira/browse/mesos-2103
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Ian, as discussed, I put the logic in cpu isolator for now until linux launcher sees more and more need to report isolator-independent cgroup usage.
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto 9df972d750ce1e4a81d2e96cc508d6f83cad2fc8 
>   src/slave/containerizer/isolators/cgroups/cpushare.cpp d4967687241ddf50a85beb961d691a8974f24fea 
>   src/slave/flags.hpp 56b25caf3901b38bdecb50310e8bcae0b114efa8 
>   src/tests/isolator_tests.cpp f7323965b94543bfda78afc4f60fc677f4900ecd 
> 
> Diff: https://reviews.apache.org/r/31250/diff/
> 
> 
> Testing
> -------
> 
> added a new test. the test is end-to-end so that future replacement of the actual logic won't require a change.
> 
> 
> Thanks,
> 
> Chi Zhang
> 
>