mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jiang Yan Xu <...@jxu.me>
Subject Re: Review Request 51031: Added non-recursive version of `cgroups::get`.
Date Tue, 01 Nov 2016 01:38:49 GMT

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




src/tests/containerizer/cgroups_tests.cpp (line 184)
<https://reviews.apache.org/r/51031/#comment223881>

    This would require all testing cgroups to be rooted under TEST_CGROUPS_ROOT (which would
be clean) but some current tests just create cgroups with "mesos_test" as a prefix, like "mesos_test1".

    
    Your previous revision of this review with `startsWith` would work with such cases but
this way wouldn't. Maybe we can fix the tests to not use patterns like "mesos_test1"? Of course
any of these cleanups doesn't have to be in this review.



src/tests/containerizer/cgroups_tests.cpp (lines 395 - 398)
<https://reviews.apache.org/r/51031/#comment223880>

    Nit: So `cgroups1` and `cgroups2` are captured in variables but `path::join(TEST_CGROUPS_ROOT,
"3"))` is not. It looks better if we call `string cgroup3 = path::join(TEST_CGROUPS_ROOT,
"3");` and `string cgroup4 = path::join(cgroup3, "4");` (so the variable name matches the
string literal. Then we can use variables consistently.
    
    Not having any such variable is fine too (so it's consistent too) because your are only
repeating once and they are simple.


- Jiang Yan Xu


On Oct. 27, 2016, 10:13 p.m., haosdent huang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51031/
> -----------------------------------------------------------
> 
> (Updated Oct. 27, 2016, 10:13 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, Zhengju Sha, and Jiang Yan
Xu.
> 
> 
> Bugs: MESOS-6035
>     https://issues.apache.org/jira/browse/MESOS-6035
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> In some 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 to retrieve cgroups recursively but
> made recursive retrieve the default behaviour. This patch fixed some
> incorrect `TEST_CGROUPS_ROOT` checks as well.
> 
> 
> Diffs
> -----
> 
>   src/linux/cgroups.hpp cfce09cb57501f2c988a8d997d7c6150280ed53d 
>   src/linux/cgroups.cpp 630e2ec2d0eac2bb35488d0416883df1601224c8 
>   src/tests/containerizer/cgroups_tests.cpp 0afaec6ae948cabf1472bf01103210d8f9809cb1

> 
> Diff: https://reviews.apache.org/r/51031/diff/
> 
> 
> Testing
> -------
> 
> ```
> sudo GLOG_v=1 ./bin/mesos-tests.sh --gtest_filter="*" --verbose
> ```
> 
> 
> Thanks,
> 
> haosdent huang
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message