You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Benjamin Hindman <be...@berkeley.edu> on 2012/10/16 21:24:09 UTC
Review Request: Refactored the cgroups tests so that they can run in a
multitude of environments. See each bullet below.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7622/
-----------------------------------------------------------
Review request for mesos, Vinod Kone, Jie Yu, and Ben Mahler.
Description
-------
* Added new test fixtures:
(1) CgroupsNoHierarchyTest for running tests where we need to create
a hierarchy (and thus, most likely, no existing hierarchy can
exist because it will already have the cpu and or memory
subsystem attached).
(2) CgroupsAnyHierarchyTest and subclasses
CgroupsAnyHierarchyWithCpuMemoryTest and
CgroupsAnyHierarchyWithCpuMemoryFreezerTest for running tests
with any hierarchy provided it has necessary subsystems
attached.
* Renamed cgroups (from "prof", "stu", etc. to "mesos_test") and
removed nested cgroups by default. The rename was done because we
might run tests inside of an existing hierarchy and we want to avoid
name clashes. The nested cgroups were removed in favor of a test
that explicitly tries to create nested cgroups (since some older
kernels with particular subsystems attached have a hard time with
this, and we'd like to detect that case explicitly).
* Created an explicit test for nested cgroups (see above).
* Updated the "write control" test to use a forked process rather than
the test process (to be more conservative in the presence of errors)
* Updated the "listen event" (i.e., "oom") test to check for the
proper control first (memory.oom_control).
* Updated the failure mechanism of forked (children) processes to use
'abort' rather than 'ASSERT_*' and 'FAIL' in order to make test
output more readable upon failures.
* Updated the notify mechanism from forked (children) processes to
parent processes to correctly distinguish a closed pipe from a value
written (to catch more instances of when the test is actually
failing).
Diffs
-----
src/tests/cgroups_tests.cpp a5098447081a87a2b69aadc4f1bdbd0aed7ac724
Diff: https://reviews.apache.org/r/7622/diff/
Testing
-------
make check
Thanks,
Benjamin Hindman
Re: Review Request: Refactored the cgroups tests so that they can run in a
multitude of environments. See each bullet below.
Posted by Benjamin Hindman <be...@berkeley.edu>.
> On Oct. 19, 2012, 1:45 a.m., Ben Mahler wrote:
> > src/tests/cgroups_tests.cpp, line 706
> > <https://reviews.apache.org/r/7622/diff/1/?file=177252#file177252line706>
> >
> > Did you intend to change the semantics here and elsewhere?
> > ASSERT_NE(-1, _); // -1 not ok
> > ASSERT_LT(0, _); // -1 ok
Yes, definitely intentional (see summary above).
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7622/#review12595
-----------------------------------------------------------
On Oct. 24, 2012, 4:42 a.m., Benjamin Hindman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7622/
> -----------------------------------------------------------
>
> (Updated Oct. 24, 2012, 4:42 a.m.)
>
>
> Review request for mesos, Vinod Kone, Jie Yu, and Ben Mahler.
>
>
> Description
> -------
>
> * Added new test fixtures:
> (1) CgroupsNoHierarchyTest for running tests where we need to create
> a hierarchy (and thus, most likely, no existing hierarchy can
> exist because it will already have the cpu and or memory
> subsystem attached).
> (2) CgroupsAnyHierarchyTest and subclasses
> CgroupsAnyHierarchyWithCpuMemoryTest and
> CgroupsAnyHierarchyWithCpuMemoryFreezerTest for running tests
> with any hierarchy provided it has necessary subsystems
> attached.
>
> * Renamed cgroups (from "prof", "stu", etc. to "mesos_test") and
> removed nested cgroups by default. The rename was done because we
> might run tests inside of an existing hierarchy and we want to avoid
> name clashes. The nested cgroups were removed in favor of a test
> that explicitly tries to create nested cgroups (since some older
> kernels with particular subsystems attached have a hard time with
> this, and we'd like to detect that case explicitly).
>
> * Created an explicit test for nested cgroups (see above).
>
> * Updated the "write control" test to use a forked process rather than
> the test process (to be more conservative in the presence of errors)
>
> * Updated the "listen event" (i.e., "oom") test to check for the
> proper control first (memory.oom_control).
>
> * Updated the failure mechanism of forked (children) processes to use
> 'abort' rather than 'ASSERT_*' and 'FAIL' in order to make test
> output more readable upon failures.
>
> * Updated the notify mechanism from forked (children) processes to
> parent processes to correctly distinguish a closed pipe from a value
> written (to catch more instances of when the test is actually
> failing).
>
>
> Diffs
> -----
>
> src/tests/cgroups_tests.cpp a5098447081a87a2b69aadc4f1bdbd0aed7ac724
>
> Diff: https://reviews.apache.org/r/7622/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Benjamin Hindman
>
>
Re: Review Request: Refactored the cgroups tests so that they can run in a
multitude of environments. See each bullet below.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7622/#review12595
-----------------------------------------------------------
src/tests/cgroups_tests.cpp
<https://reviews.apache.org/r/7622/#comment26893>
Did you intend to change the semantics here and elsewhere?
ASSERT_NE(-1, _); // -1 not ok
ASSERT_LT(0, _); // -1 ok
- Ben Mahler
On Oct. 16, 2012, 7:24 p.m., Benjamin Hindman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7622/
> -----------------------------------------------------------
>
> (Updated Oct. 16, 2012, 7:24 p.m.)
>
>
> Review request for mesos, Vinod Kone, Jie Yu, and Ben Mahler.
>
>
> Description
> -------
>
> * Added new test fixtures:
> (1) CgroupsNoHierarchyTest for running tests where we need to create
> a hierarchy (and thus, most likely, no existing hierarchy can
> exist because it will already have the cpu and or memory
> subsystem attached).
> (2) CgroupsAnyHierarchyTest and subclasses
> CgroupsAnyHierarchyWithCpuMemoryTest and
> CgroupsAnyHierarchyWithCpuMemoryFreezerTest for running tests
> with any hierarchy provided it has necessary subsystems
> attached.
>
> * Renamed cgroups (from "prof", "stu", etc. to "mesos_test") and
> removed nested cgroups by default. The rename was done because we
> might run tests inside of an existing hierarchy and we want to avoid
> name clashes. The nested cgroups were removed in favor of a test
> that explicitly tries to create nested cgroups (since some older
> kernels with particular subsystems attached have a hard time with
> this, and we'd like to detect that case explicitly).
>
> * Created an explicit test for nested cgroups (see above).
>
> * Updated the "write control" test to use a forked process rather than
> the test process (to be more conservative in the presence of errors)
>
> * Updated the "listen event" (i.e., "oom") test to check for the
> proper control first (memory.oom_control).
>
> * Updated the failure mechanism of forked (children) processes to use
> 'abort' rather than 'ASSERT_*' and 'FAIL' in order to make test
> output more readable upon failures.
>
> * Updated the notify mechanism from forked (children) processes to
> parent processes to correctly distinguish a closed pipe from a value
> written (to catch more instances of when the test is actually
> failing).
>
>
> Diffs
> -----
>
> src/tests/cgroups_tests.cpp a5098447081a87a2b69aadc4f1bdbd0aed7ac724
>
> Diff: https://reviews.apache.org/r/7622/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Benjamin Hindman
>
>
Re: Review Request: Refactored the cgroups tests so that they can run in a
multitude of environments. See each bullet below.
Posted by Benjamin Hindman <be...@berkeley.edu>.
> On Oct. 17, 2012, 1:27 a.m., Jie Yu wrote:
> > src/tests/cgroups_tests.cpp, line 64
> > <https://reviews.apache.org/r/7622/diff/1/?file=177252#file177252line64>
> >
> > One thing I am thinking is: in cgroups test, it's highly likely that a second test will fail during setup if the first test fails due to whatever reasons.
> >
> > For example, there are some processes left in a cgroup due to the failure of the first test, causing the second test (and all following tests) to fail during setup.
> >
> > Maybe what we can do is to declare a static flag in class CgroupsTest and set this flag if a test fails so that later tests will fail gracefully (have a print saying that it's due to the failure of the previous test), instead of a random failure.
Sounds good. I updated the output when we try and remove our old cgroup so that a user knows something seriously wrong has happened.
> On Oct. 17, 2012, 1:27 a.m., Jie Yu wrote:
> > src/tests/cgroups_tests.cpp, line 151
> > <https://reviews.apache.org/r/7622/diff/1/?file=177252#file177252line151>
> >
> > Compilable? ASSERT_SOME does not return an ostream.
I didn't include you on the review for that (https://reviews.apache.org/r/7619), but I refactored ASSERT_SOME to use gtest constructs. Of course this code compiles and all the tests pass! ;)
> On Oct. 17, 2012, 1:27 a.m., Jie Yu wrote:
> > src/tests/cgroups_tests.cpp, line 152
> > <https://reviews.apache.org/r/7622/diff/1/?file=177252#file177252line152>
> >
> > It is possible that here "subsystem" is empty. The createHierarchy will fail when subsystem is empty.
Fixed.
> On Oct. 17, 2012, 1:27 a.m., Jie Yu wrote:
> > src/tests/cgroups_tests.cpp, line 373
> > <https://reviews.apache.org/r/7622/diff/1/?file=177252#file177252line373>
> >
> > we should add one more check here:
> >
> > ASSERT_EQ(2UL, cgroups.get().size());
Fixed.
> On Oct. 17, 2012, 1:27 a.m., Jie Yu wrote:
> > src/tests/cgroups_tests.cpp, line 377
> > <https://reviews.apache.org/r/7622/diff/1/?file=177252#file177252line377>
> >
> > "mesos_test" should not be returned here.
Thanks! This test obviously fails with my kernel version. ;)
> On Oct. 17, 2012, 1:27 a.m., Jie Yu wrote:
> > src/tests/cgroups_tests.cpp, line 382
> > <https://reviews.apache.org/r/7622/diff/1/?file=177252#file177252line382>
> >
> > Why expect cgroups to be empty here?
Just a copy-pasta bug when I split this test into it's own NestedCgroup test. Again, my kernel version is too old to run this test. :(
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7622/#review12489
-----------------------------------------------------------
On Oct. 24, 2012, 4:42 a.m., Benjamin Hindman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7622/
> -----------------------------------------------------------
>
> (Updated Oct. 24, 2012, 4:42 a.m.)
>
>
> Review request for mesos, Vinod Kone, Jie Yu, and Ben Mahler.
>
>
> Description
> -------
>
> * Added new test fixtures:
> (1) CgroupsNoHierarchyTest for running tests where we need to create
> a hierarchy (and thus, most likely, no existing hierarchy can
> exist because it will already have the cpu and or memory
> subsystem attached).
> (2) CgroupsAnyHierarchyTest and subclasses
> CgroupsAnyHierarchyWithCpuMemoryTest and
> CgroupsAnyHierarchyWithCpuMemoryFreezerTest for running tests
> with any hierarchy provided it has necessary subsystems
> attached.
>
> * Renamed cgroups (from "prof", "stu", etc. to "mesos_test") and
> removed nested cgroups by default. The rename was done because we
> might run tests inside of an existing hierarchy and we want to avoid
> name clashes. The nested cgroups were removed in favor of a test
> that explicitly tries to create nested cgroups (since some older
> kernels with particular subsystems attached have a hard time with
> this, and we'd like to detect that case explicitly).
>
> * Created an explicit test for nested cgroups (see above).
>
> * Updated the "write control" test to use a forked process rather than
> the test process (to be more conservative in the presence of errors)
>
> * Updated the "listen event" (i.e., "oom") test to check for the
> proper control first (memory.oom_control).
>
> * Updated the failure mechanism of forked (children) processes to use
> 'abort' rather than 'ASSERT_*' and 'FAIL' in order to make test
> output more readable upon failures.
>
> * Updated the notify mechanism from forked (children) processes to
> parent processes to correctly distinguish a closed pipe from a value
> written (to catch more instances of when the test is actually
> failing).
>
>
> Diffs
> -----
>
> src/tests/cgroups_tests.cpp a5098447081a87a2b69aadc4f1bdbd0aed7ac724
>
> Diff: https://reviews.apache.org/r/7622/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Benjamin Hindman
>
>
Re: Review Request: Refactored the cgroups tests so that they can run in a
multitude of environments. See each bullet below.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7622/#review12489
-----------------------------------------------------------
That's awesome!
src/tests/cgroups_tests.cpp
<https://reviews.apache.org/r/7622/#comment26582>
One thing I am thinking is: in cgroups test, it's highly likely that a second test will fail during setup if the first test fails due to whatever reasons.
For example, there are some processes left in a cgroup due to the failure of the first test, causing the second test (and all following tests) to fail during setup.
Maybe what we can do is to declare a static flag in class CgroupsTest and set this flag if a test fails so that later tests will fail gracefully (have a print saying that it's due to the failure of the previous test), instead of a random failure.
src/tests/cgroups_tests.cpp
<https://reviews.apache.org/r/7622/#comment26592>
Compilable? ASSERT_SOME does not return an ostream.
src/tests/cgroups_tests.cpp
<https://reviews.apache.org/r/7622/#comment26594>
It is possible that here "subsystem" is empty. The createHierarchy will fail when subsystem is empty.
src/tests/cgroups_tests.cpp
<https://reviews.apache.org/r/7622/#comment26598>
we should add one more check here:
ASSERT_EQ(2UL, cgroups.get().size());
src/tests/cgroups_tests.cpp
<https://reviews.apache.org/r/7622/#comment26597>
"mesos_test" should not be returned here.
src/tests/cgroups_tests.cpp
<https://reviews.apache.org/r/7622/#comment26599>
Why expect cgroups to be empty here?
src/tests/cgroups_tests.cpp
<https://reviews.apache.org/r/7622/#comment26602>
This should be a CgroupsAnyHierarchyWithCpuMemoryTest
- Jie Yu
On Oct. 16, 2012, 7:24 p.m., Benjamin Hindman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7622/
> -----------------------------------------------------------
>
> (Updated Oct. 16, 2012, 7:24 p.m.)
>
>
> Review request for mesos, Vinod Kone, Jie Yu, and Ben Mahler.
>
>
> Description
> -------
>
> * Added new test fixtures:
> (1) CgroupsNoHierarchyTest for running tests where we need to create
> a hierarchy (and thus, most likely, no existing hierarchy can
> exist because it will already have the cpu and or memory
> subsystem attached).
> (2) CgroupsAnyHierarchyTest and subclasses
> CgroupsAnyHierarchyWithCpuMemoryTest and
> CgroupsAnyHierarchyWithCpuMemoryFreezerTest for running tests
> with any hierarchy provided it has necessary subsystems
> attached.
>
> * Renamed cgroups (from "prof", "stu", etc. to "mesos_test") and
> removed nested cgroups by default. The rename was done because we
> might run tests inside of an existing hierarchy and we want to avoid
> name clashes. The nested cgroups were removed in favor of a test
> that explicitly tries to create nested cgroups (since some older
> kernels with particular subsystems attached have a hard time with
> this, and we'd like to detect that case explicitly).
>
> * Created an explicit test for nested cgroups (see above).
>
> * Updated the "write control" test to use a forked process rather than
> the test process (to be more conservative in the presence of errors)
>
> * Updated the "listen event" (i.e., "oom") test to check for the
> proper control first (memory.oom_control).
>
> * Updated the failure mechanism of forked (children) processes to use
> 'abort' rather than 'ASSERT_*' and 'FAIL' in order to make test
> output more readable upon failures.
>
> * Updated the notify mechanism from forked (children) processes to
> parent processes to correctly distinguish a closed pipe from a value
> written (to catch more instances of when the test is actually
> failing).
>
>
> Diffs
> -----
>
> src/tests/cgroups_tests.cpp a5098447081a87a2b69aadc4f1bdbd0aed7ac724
>
> Diff: https://reviews.apache.org/r/7622/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Benjamin Hindman
>
>
Re: Review Request: Refactored the cgroups tests so that they can run in a
multitude of environments. See each bullet below.
Posted by Vinod Kone <vi...@gmail.com>.
> On Oct. 18, 2012, 9:47 p.m., Vinod Kone wrote:
> >
missed these comments?
- Vinod
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7622/#review12581
-----------------------------------------------------------
On Oct. 24, 2012, 4:42 a.m., Benjamin Hindman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7622/
> -----------------------------------------------------------
>
> (Updated Oct. 24, 2012, 4:42 a.m.)
>
>
> Review request for mesos, Vinod Kone, Jie Yu, and Ben Mahler.
>
>
> Description
> -------
>
> * Added new test fixtures:
> (1) CgroupsNoHierarchyTest for running tests where we need to create
> a hierarchy (and thus, most likely, no existing hierarchy can
> exist because it will already have the cpu and or memory
> subsystem attached).
> (2) CgroupsAnyHierarchyTest and subclasses
> CgroupsAnyHierarchyWithCpuMemoryTest and
> CgroupsAnyHierarchyWithCpuMemoryFreezerTest for running tests
> with any hierarchy provided it has necessary subsystems
> attached.
>
> * Renamed cgroups (from "prof", "stu", etc. to "mesos_test") and
> removed nested cgroups by default. The rename was done because we
> might run tests inside of an existing hierarchy and we want to avoid
> name clashes. The nested cgroups were removed in favor of a test
> that explicitly tries to create nested cgroups (since some older
> kernels with particular subsystems attached have a hard time with
> this, and we'd like to detect that case explicitly).
>
> * Created an explicit test for nested cgroups (see above).
>
> * Updated the "write control" test to use a forked process rather than
> the test process (to be more conservative in the presence of errors)
>
> * Updated the "listen event" (i.e., "oom") test to check for the
> proper control first (memory.oom_control).
>
> * Updated the failure mechanism of forked (children) processes to use
> 'abort' rather than 'ASSERT_*' and 'FAIL' in order to make test
> output more readable upon failures.
>
> * Updated the notify mechanism from forked (children) processes to
> parent processes to correctly distinguish a closed pipe from a value
> written (to catch more instances of when the test is actually
> failing).
>
>
> Diffs
> -----
>
> src/tests/cgroups_tests.cpp a5098447081a87a2b69aadc4f1bdbd0aed7ac724
>
> Diff: https://reviews.apache.org/r/7622/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Benjamin Hindman
>
>
Re: Review Request: Refactored the cgroups tests so that they can run in a
multitude of environments. See each bullet below.
Posted by Benjamin Hindman <be...@berkeley.edu>.
> On Oct. 18, 2012, 9:47 p.m., Vinod Kone wrote:
> >
>
> Vinod Kone wrote:
> missed these comments?
Yes, getting to them now.
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7622/#review12581
-----------------------------------------------------------
On Oct. 24, 2012, 4:42 a.m., Benjamin Hindman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7622/
> -----------------------------------------------------------
>
> (Updated Oct. 24, 2012, 4:42 a.m.)
>
>
> Review request for mesos, Vinod Kone, Jie Yu, and Ben Mahler.
>
>
> Description
> -------
>
> * Added new test fixtures:
> (1) CgroupsNoHierarchyTest for running tests where we need to create
> a hierarchy (and thus, most likely, no existing hierarchy can
> exist because it will already have the cpu and or memory
> subsystem attached).
> (2) CgroupsAnyHierarchyTest and subclasses
> CgroupsAnyHierarchyWithCpuMemoryTest and
> CgroupsAnyHierarchyWithCpuMemoryFreezerTest for running tests
> with any hierarchy provided it has necessary subsystems
> attached.
>
> * Renamed cgroups (from "prof", "stu", etc. to "mesos_test") and
> removed nested cgroups by default. The rename was done because we
> might run tests inside of an existing hierarchy and we want to avoid
> name clashes. The nested cgroups were removed in favor of a test
> that explicitly tries to create nested cgroups (since some older
> kernels with particular subsystems attached have a hard time with
> this, and we'd like to detect that case explicitly).
>
> * Created an explicit test for nested cgroups (see above).
>
> * Updated the "write control" test to use a forked process rather than
> the test process (to be more conservative in the presence of errors)
>
> * Updated the "listen event" (i.e., "oom") test to check for the
> proper control first (memory.oom_control).
>
> * Updated the failure mechanism of forked (children) processes to use
> 'abort' rather than 'ASSERT_*' and 'FAIL' in order to make test
> output more readable upon failures.
>
> * Updated the notify mechanism from forked (children) processes to
> parent processes to correctly distinguish a closed pipe from a value
> written (to catch more instances of when the test is actually
> failing).
>
>
> Diffs
> -----
>
> src/tests/cgroups_tests.cpp a5098447081a87a2b69aadc4f1bdbd0aed7ac724
>
> Diff: https://reviews.apache.org/r/7622/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Benjamin Hindman
>
>
Re: Review Request: Refactored the cgroups tests so that they can run in a
multitude of environments. See each bullet below.
Posted by Benjamin Hindman <be...@berkeley.edu>.
> On Oct. 18, 2012, 9:47 p.m., Vinod Kone wrote:
> > src/tests/cgroups_tests.cpp, line 76
> > <https://reviews.apache.org/r/7622/diff/1/?file=177252#file177252line76>
> >
> > is it possible for HIERARCHY to not exist, given we already did a checkHierachy?
It's possible that the ASSERT_SOME failed above, so this is a last ditch effort to remove the directory if possible.
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7622/#review12581
-----------------------------------------------------------
On Oct. 26, 2012, 4:50 p.m., Benjamin Hindman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7622/
> -----------------------------------------------------------
>
> (Updated Oct. 26, 2012, 4:50 p.m.)
>
>
> Review request for mesos, Vinod Kone, Jie Yu, and Ben Mahler.
>
>
> Description
> -------
>
> * Added new test fixtures:
> (1) CgroupsNoHierarchyTest for running tests where we need to create
> a hierarchy (and thus, most likely, no existing hierarchy can
> exist because it will already have the cpu and or memory
> subsystem attached).
> (2) CgroupsAnyHierarchyTest and subclasses
> CgroupsAnyHierarchyWithCpuMemoryTest and
> CgroupsAnyHierarchyWithCpuMemoryFreezerTest for running tests
> with any hierarchy provided it has necessary subsystems
> attached.
>
> * Renamed cgroups (from "prof", "stu", etc. to "mesos_test") and
> removed nested cgroups by default. The rename was done because we
> might run tests inside of an existing hierarchy and we want to avoid
> name clashes. The nested cgroups were removed in favor of a test
> that explicitly tries to create nested cgroups (since some older
> kernels with particular subsystems attached have a hard time with
> this, and we'd like to detect that case explicitly).
>
> * Created an explicit test for nested cgroups (see above).
>
> * Updated the "write control" test to use a forked process rather than
> the test process (to be more conservative in the presence of errors)
>
> * Updated the "listen event" (i.e., "oom") test to check for the
> proper control first (memory.oom_control).
>
> * Updated the failure mechanism of forked (children) processes to use
> 'abort' rather than 'ASSERT_*' and 'FAIL' in order to make test
> output more readable upon failures.
>
> * Updated the notify mechanism from forked (children) processes to
> parent processes to correctly distinguish a closed pipe from a value
> written (to catch more instances of when the test is actually
> failing).
>
>
> Diffs
> -----
>
> src/tests/cgroups_tests.cpp a5098447081a87a2b69aadc4f1bdbd0aed7ac724
>
> Diff: https://reviews.apache.org/r/7622/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Benjamin Hindman
>
>
Re: Review Request: Refactored the cgroups tests so that they can run in a
multitude of environments. See each bullet below.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7622/#review12581
-----------------------------------------------------------
src/tests/cgroups_tests.cpp
<https://reviews.apache.org/r/7622/#comment26851>
is it possible for HIERARCHY to not exist, given we already did a checkHierachy?
src/tests/cgroups_tests.cpp
<https://reviews.apache.org/r/7622/#comment26853>
s/-\n/--\n/
src/tests/cgroups_tests.cpp
<https://reviews.apache.org/r/7622/#comment26852>
Would be great to give the user the exact gtest_filter to disable this.
src/tests/cgroups_tests.cpp
<https://reviews.apache.org/r/7622/#comment26857>
ditto, a filter expression here would be great.
- Vinod Kone
On Oct. 16, 2012, 7:24 p.m., Benjamin Hindman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7622/
> -----------------------------------------------------------
>
> (Updated Oct. 16, 2012, 7:24 p.m.)
>
>
> Review request for mesos, Vinod Kone, Jie Yu, and Ben Mahler.
>
>
> Description
> -------
>
> * Added new test fixtures:
> (1) CgroupsNoHierarchyTest for running tests where we need to create
> a hierarchy (and thus, most likely, no existing hierarchy can
> exist because it will already have the cpu and or memory
> subsystem attached).
> (2) CgroupsAnyHierarchyTest and subclasses
> CgroupsAnyHierarchyWithCpuMemoryTest and
> CgroupsAnyHierarchyWithCpuMemoryFreezerTest for running tests
> with any hierarchy provided it has necessary subsystems
> attached.
>
> * Renamed cgroups (from "prof", "stu", etc. to "mesos_test") and
> removed nested cgroups by default. The rename was done because we
> might run tests inside of an existing hierarchy and we want to avoid
> name clashes. The nested cgroups were removed in favor of a test
> that explicitly tries to create nested cgroups (since some older
> kernels with particular subsystems attached have a hard time with
> this, and we'd like to detect that case explicitly).
>
> * Created an explicit test for nested cgroups (see above).
>
> * Updated the "write control" test to use a forked process rather than
> the test process (to be more conservative in the presence of errors)
>
> * Updated the "listen event" (i.e., "oom") test to check for the
> proper control first (memory.oom_control).
>
> * Updated the failure mechanism of forked (children) processes to use
> 'abort' rather than 'ASSERT_*' and 'FAIL' in order to make test
> output more readable upon failures.
>
> * Updated the notify mechanism from forked (children) processes to
> parent processes to correctly distinguish a closed pipe from a value
> written (to catch more instances of when the test is actually
> failing).
>
>
> Diffs
> -----
>
> src/tests/cgroups_tests.cpp a5098447081a87a2b69aadc4f1bdbd0aed7ac724
>
> Diff: https://reviews.apache.org/r/7622/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Benjamin Hindman
>
>
Re: Review Request: Refactored the cgroups tests so that they can run in a
multitude of environments. See each bullet below.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7622/#review12840
-----------------------------------------------------------
Ship it!
Ship It!
- Jie Yu
On Oct. 26, 2012, 4:50 p.m., Benjamin Hindman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7622/
> -----------------------------------------------------------
>
> (Updated Oct. 26, 2012, 4:50 p.m.)
>
>
> Review request for mesos, Vinod Kone, Jie Yu, and Ben Mahler.
>
>
> Description
> -------
>
> * Added new test fixtures:
> (1) CgroupsNoHierarchyTest for running tests where we need to create
> a hierarchy (and thus, most likely, no existing hierarchy can
> exist because it will already have the cpu and or memory
> subsystem attached).
> (2) CgroupsAnyHierarchyTest and subclasses
> CgroupsAnyHierarchyWithCpuMemoryTest and
> CgroupsAnyHierarchyWithCpuMemoryFreezerTest for running tests
> with any hierarchy provided it has necessary subsystems
> attached.
>
> * Renamed cgroups (from "prof", "stu", etc. to "mesos_test") and
> removed nested cgroups by default. The rename was done because we
> might run tests inside of an existing hierarchy and we want to avoid
> name clashes. The nested cgroups were removed in favor of a test
> that explicitly tries to create nested cgroups (since some older
> kernels with particular subsystems attached have a hard time with
> this, and we'd like to detect that case explicitly).
>
> * Created an explicit test for nested cgroups (see above).
>
> * Updated the "write control" test to use a forked process rather than
> the test process (to be more conservative in the presence of errors)
>
> * Updated the "listen event" (i.e., "oom") test to check for the
> proper control first (memory.oom_control).
>
> * Updated the failure mechanism of forked (children) processes to use
> 'abort' rather than 'ASSERT_*' and 'FAIL' in order to make test
> output more readable upon failures.
>
> * Updated the notify mechanism from forked (children) processes to
> parent processes to correctly distinguish a closed pipe from a value
> written (to catch more instances of when the test is actually
> failing).
>
>
> Diffs
> -----
>
> src/tests/cgroups_tests.cpp a5098447081a87a2b69aadc4f1bdbd0aed7ac724
>
> Diff: https://reviews.apache.org/r/7622/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Benjamin Hindman
>
>
Re: Review Request: Refactored the cgroups tests so that they can run in a
multitude of environments. See each bullet below.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7622/#review12823
-----------------------------------------------------------
Ship it!
src/tests/cgroups_tests.cpp
<https://reviews.apache.org/r/7622/#comment27472>
sweet!
- Vinod Kone
On Oct. 26, 2012, 4:50 p.m., Benjamin Hindman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7622/
> -----------------------------------------------------------
>
> (Updated Oct. 26, 2012, 4:50 p.m.)
>
>
> Review request for mesos, Vinod Kone, Jie Yu, and Ben Mahler.
>
>
> Description
> -------
>
> * Added new test fixtures:
> (1) CgroupsNoHierarchyTest for running tests where we need to create
> a hierarchy (and thus, most likely, no existing hierarchy can
> exist because it will already have the cpu and or memory
> subsystem attached).
> (2) CgroupsAnyHierarchyTest and subclasses
> CgroupsAnyHierarchyWithCpuMemoryTest and
> CgroupsAnyHierarchyWithCpuMemoryFreezerTest for running tests
> with any hierarchy provided it has necessary subsystems
> attached.
>
> * Renamed cgroups (from "prof", "stu", etc. to "mesos_test") and
> removed nested cgroups by default. The rename was done because we
> might run tests inside of an existing hierarchy and we want to avoid
> name clashes. The nested cgroups were removed in favor of a test
> that explicitly tries to create nested cgroups (since some older
> kernels with particular subsystems attached have a hard time with
> this, and we'd like to detect that case explicitly).
>
> * Created an explicit test for nested cgroups (see above).
>
> * Updated the "write control" test to use a forked process rather than
> the test process (to be more conservative in the presence of errors)
>
> * Updated the "listen event" (i.e., "oom") test to check for the
> proper control first (memory.oom_control).
>
> * Updated the failure mechanism of forked (children) processes to use
> 'abort' rather than 'ASSERT_*' and 'FAIL' in order to make test
> output more readable upon failures.
>
> * Updated the notify mechanism from forked (children) processes to
> parent processes to correctly distinguish a closed pipe from a value
> written (to catch more instances of when the test is actually
> failing).
>
>
> Diffs
> -----
>
> src/tests/cgroups_tests.cpp a5098447081a87a2b69aadc4f1bdbd0aed7ac724
>
> Diff: https://reviews.apache.org/r/7622/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Benjamin Hindman
>
>
Re: Review Request: Refactored the cgroups tests so that they can run in a
multitude of environments. See each bullet below.
Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7622/
-----------------------------------------------------------
(Updated Oct. 26, 2012, 4:50 p.m.)
Review request for mesos, Vinod Kone, Jie Yu, and Ben Mahler.
Changes
-------
Updates from review comments.
Description
-------
* Added new test fixtures:
(1) CgroupsNoHierarchyTest for running tests where we need to create
a hierarchy (and thus, most likely, no existing hierarchy can
exist because it will already have the cpu and or memory
subsystem attached).
(2) CgroupsAnyHierarchyTest and subclasses
CgroupsAnyHierarchyWithCpuMemoryTest and
CgroupsAnyHierarchyWithCpuMemoryFreezerTest for running tests
with any hierarchy provided it has necessary subsystems
attached.
* Renamed cgroups (from "prof", "stu", etc. to "mesos_test") and
removed nested cgroups by default. The rename was done because we
might run tests inside of an existing hierarchy and we want to avoid
name clashes. The nested cgroups were removed in favor of a test
that explicitly tries to create nested cgroups (since some older
kernels with particular subsystems attached have a hard time with
this, and we'd like to detect that case explicitly).
* Created an explicit test for nested cgroups (see above).
* Updated the "write control" test to use a forked process rather than
the test process (to be more conservative in the presence of errors)
* Updated the "listen event" (i.e., "oom") test to check for the
proper control first (memory.oom_control).
* Updated the failure mechanism of forked (children) processes to use
'abort' rather than 'ASSERT_*' and 'FAIL' in order to make test
output more readable upon failures.
* Updated the notify mechanism from forked (children) processes to
parent processes to correctly distinguish a closed pipe from a value
written (to catch more instances of when the test is actually
failing).
Diffs (updated)
-----
src/tests/cgroups_tests.cpp a5098447081a87a2b69aadc4f1bdbd0aed7ac724
Diff: https://reviews.apache.org/r/7622/diff/
Testing
-------
make check
Thanks,
Benjamin Hindman
Re: Review Request: Refactored the cgroups tests so that they can run in a
multitude of environments. See each bullet below.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7622/#review12766
-----------------------------------------------------------
Ship it!
Ship It!
- Ben Mahler
On Oct. 24, 2012, 4:42 a.m., Benjamin Hindman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7622/
> -----------------------------------------------------------
>
> (Updated Oct. 24, 2012, 4:42 a.m.)
>
>
> Review request for mesos, Vinod Kone, Jie Yu, and Ben Mahler.
>
>
> Description
> -------
>
> * Added new test fixtures:
> (1) CgroupsNoHierarchyTest for running tests where we need to create
> a hierarchy (and thus, most likely, no existing hierarchy can
> exist because it will already have the cpu and or memory
> subsystem attached).
> (2) CgroupsAnyHierarchyTest and subclasses
> CgroupsAnyHierarchyWithCpuMemoryTest and
> CgroupsAnyHierarchyWithCpuMemoryFreezerTest for running tests
> with any hierarchy provided it has necessary subsystems
> attached.
>
> * Renamed cgroups (from "prof", "stu", etc. to "mesos_test") and
> removed nested cgroups by default. The rename was done because we
> might run tests inside of an existing hierarchy and we want to avoid
> name clashes. The nested cgroups were removed in favor of a test
> that explicitly tries to create nested cgroups (since some older
> kernels with particular subsystems attached have a hard time with
> this, and we'd like to detect that case explicitly).
>
> * Created an explicit test for nested cgroups (see above).
>
> * Updated the "write control" test to use a forked process rather than
> the test process (to be more conservative in the presence of errors)
>
> * Updated the "listen event" (i.e., "oom") test to check for the
> proper control first (memory.oom_control).
>
> * Updated the failure mechanism of forked (children) processes to use
> 'abort' rather than 'ASSERT_*' and 'FAIL' in order to make test
> output more readable upon failures.
>
> * Updated the notify mechanism from forked (children) processes to
> parent processes to correctly distinguish a closed pipe from a value
> written (to catch more instances of when the test is actually
> failing).
>
>
> Diffs
> -----
>
> src/tests/cgroups_tests.cpp a5098447081a87a2b69aadc4f1bdbd0aed7ac724
>
> Diff: https://reviews.apache.org/r/7622/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Benjamin Hindman
>
>
Re: Review Request: Refactored the cgroups tests so that they can run in a
multitude of environments. See each bullet below.
Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7622/
-----------------------------------------------------------
(Updated Oct. 24, 2012, 4:42 a.m.)
Review request for mesos, Vinod Kone, Jie Yu, and Ben Mahler.
Changes
-------
Updates based on review comments.
Description
-------
* Added new test fixtures:
(1) CgroupsNoHierarchyTest for running tests where we need to create
a hierarchy (and thus, most likely, no existing hierarchy can
exist because it will already have the cpu and or memory
subsystem attached).
(2) CgroupsAnyHierarchyTest and subclasses
CgroupsAnyHierarchyWithCpuMemoryTest and
CgroupsAnyHierarchyWithCpuMemoryFreezerTest for running tests
with any hierarchy provided it has necessary subsystems
attached.
* Renamed cgroups (from "prof", "stu", etc. to "mesos_test") and
removed nested cgroups by default. The rename was done because we
might run tests inside of an existing hierarchy and we want to avoid
name clashes. The nested cgroups were removed in favor of a test
that explicitly tries to create nested cgroups (since some older
kernels with particular subsystems attached have a hard time with
this, and we'd like to detect that case explicitly).
* Created an explicit test for nested cgroups (see above).
* Updated the "write control" test to use a forked process rather than
the test process (to be more conservative in the presence of errors)
* Updated the "listen event" (i.e., "oom") test to check for the
proper control first (memory.oom_control).
* Updated the failure mechanism of forked (children) processes to use
'abort' rather than 'ASSERT_*' and 'FAIL' in order to make test
output more readable upon failures.
* Updated the notify mechanism from forked (children) processes to
parent processes to correctly distinguish a closed pipe from a value
written (to catch more instances of when the test is actually
failing).
Diffs (updated)
-----
src/tests/cgroups_tests.cpp a5098447081a87a2b69aadc4f1bdbd0aed7ac724
Diff: https://reviews.apache.org/r/7622/diff/
Testing
-------
make check
Thanks,
Benjamin Hindman