You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Vinod Kone <vi...@gmail.com> on 2013/03/08 03:46:45 UTC
Review Request: Refactored cgroups tests and made root cgroup used by
cgroups isolation module configurable.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9814/
-----------------------------------------------------------
Review request for mesos, Benjamin Hindman and Ben Mahler.
Description
-------
Pulled out some cgroups test fixtures into test utils, and made them generic, so that they can be re-used by other tests.
Also the root cgroup can be configured via flag, helping cgroups isolation module related tests to use a different cgroup (e.g. "mesos_tests").
NOTE: This is based of my earlier cgroups review: https://reviews.apache.org/r/9806/
Diffs
-----
src/slave/cgroups_isolation_module.cpp c7ca32c8122ce97f77a5ba68f6ad3dc1f17704f7
src/slave/flags.hpp 9cd436cae5772b906b3566eb1f344904d5894214
src/tests/cgroups_tests.cpp ed419eb174a095786fb6de9e181d42587e5b40e2
src/tests/utils.hpp 0d4edad4787f92f86b1486b6b510f46a62d7d541
Diff: https://reviews.apache.org/r/9814/diff/
Testing
-------
sudo GLOG_v=1 ./bin/mesos-tests.sh --gtest_filter="*Cgroups*" --verbose
Thanks,
Vinod Kone
Re: Review Request: Refactored cgroups tests and made root cgroup used by
cgroups isolation module configurable.
Posted by Vinod Kone <vi...@gmail.com>.
> On March 8, 2013, 7 p.m., Benjamin Hindman wrote:
> > src/slave/flags.hpp, line 117
> > <https://reviews.apache.org/r/9814/diff/1/?file=268321#file268321line117>
> >
> > For the same reason we should s/cgroups_hierarchy_root/cgroups_hierarchy/, I'd like to s/cgroups_root_cgroup/cgroups_root/. Also, I'd prefer s/TEST_HIERARCHY/TEST_CGROUPS_HIERARCHY/ and s/TEST_ROOT_CGROUP/TEST_CGROUPS_ROOT/ so that they match the flag names. Arguably we should get rid of the constants anyway since that is what the flags are for!
As discussed yesterday, If I don't have those constants, I can't use them in SetupTestCase/TearDownTestCase static functions! Is there a way?
- Vinod
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9814/#review17620
-----------------------------------------------------------
On March 8, 2013, 2:46 a.m., Vinod Kone wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9814/
> -----------------------------------------------------------
>
> (Updated March 8, 2013, 2:46 a.m.)
>
>
> Review request for mesos, Benjamin Hindman and Ben Mahler.
>
>
> Description
> -------
>
> Pulled out some cgroups test fixtures into test utils, and made them generic, so that they can be re-used by other tests.
>
> Also the root cgroup can be configured via flag, helping cgroups isolation module related tests to use a different cgroup (e.g. "mesos_tests").
>
> NOTE: This is based of my earlier cgroups review: https://reviews.apache.org/r/9806/
>
>
> Diffs
> -----
>
> src/slave/cgroups_isolation_module.cpp c7ca32c8122ce97f77a5ba68f6ad3dc1f17704f7
> src/slave/flags.hpp 9cd436cae5772b906b3566eb1f344904d5894214
> src/tests/cgroups_tests.cpp ed419eb174a095786fb6de9e181d42587e5b40e2
> src/tests/utils.hpp 0d4edad4787f92f86b1486b6b510f46a62d7d541
>
> Diff: https://reviews.apache.org/r/9814/diff/
>
>
> Testing
> -------
>
> sudo GLOG_v=1 ./bin/mesos-tests.sh --gtest_filter="*Cgroups*" --verbose
>
>
> Thanks,
>
> Vinod Kone
>
>
Re: Review Request: Refactored cgroups tests and made root cgroup used by
cgroups isolation module configurable.
Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9814/#review17620
-----------------------------------------------------------
Ship it!
src/slave/flags.hpp
<https://reviews.apache.org/r/9814/#comment37365>
For the same reason we should s/cgroups_hierarchy_root/cgroups_hierarchy/, I'd like to s/cgroups_root_cgroup/cgroups_root/. Also, I'd prefer s/TEST_HIERARCHY/TEST_CGROUPS_HIERARCHY/ and s/TEST_ROOT_CGROUP/TEST_CGROUPS_ROOT/ so that they match the flag names. Arguably we should get rid of the constants anyway since that is what the flags are for!
- Benjamin Hindman
On March 8, 2013, 2:46 a.m., Vinod Kone wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9814/
> -----------------------------------------------------------
>
> (Updated March 8, 2013, 2:46 a.m.)
>
>
> Review request for mesos, Benjamin Hindman and Ben Mahler.
>
>
> Description
> -------
>
> Pulled out some cgroups test fixtures into test utils, and made them generic, so that they can be re-used by other tests.
>
> Also the root cgroup can be configured via flag, helping cgroups isolation module related tests to use a different cgroup (e.g. "mesos_tests").
>
> NOTE: This is based of my earlier cgroups review: https://reviews.apache.org/r/9806/
>
>
> Diffs
> -----
>
> src/slave/cgroups_isolation_module.cpp c7ca32c8122ce97f77a5ba68f6ad3dc1f17704f7
> src/slave/flags.hpp 9cd436cae5772b906b3566eb1f344904d5894214
> src/tests/cgroups_tests.cpp ed419eb174a095786fb6de9e181d42587e5b40e2
> src/tests/utils.hpp 0d4edad4787f92f86b1486b6b510f46a62d7d541
>
> Diff: https://reviews.apache.org/r/9814/diff/
>
>
> Testing
> -------
>
> sudo GLOG_v=1 ./bin/mesos-tests.sh --gtest_filter="*Cgroups*" --verbose
>
>
> Thanks,
>
> Vinod Kone
>
>
Re: Review Request: Refactored cgroups tests and made root cgroup used by
cgroups isolation module configurable.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9814/
-----------------------------------------------------------
(Updated March 8, 2013, 11:12 p.m.)
Review request for mesos, Benjamin Hindman and Ben Mahler.
Changes
-------
rebased off trunk for posterity. no need for review.
Description
-------
Pulled out some cgroups test fixtures into test utils, and made them generic, so that they can be re-used by other tests.
Also the root cgroup can be configured via flag, helping cgroups isolation module related tests to use a different cgroup (e.g. "mesos_tests").
NOTE: This is based of my earlier cgroups review: https://reviews.apache.org/r/9806/
Diffs (updated)
-----
src/slave/cgroups_isolation_module.cpp 438fa3e761dfd21d989158453c5213b08f1f7380
src/slave/flags.hpp c060c17798c96a563524cced1010f38debdeac4e
src/tests/cgroups_tests.cpp 6db000e9db0768cb3065211bff019089ddec331c
src/tests/utils.hpp 0863b789776408598e9720c68d5494393010ac8b
Diff: https://reviews.apache.org/r/9814/diff/
Testing
-------
sudo GLOG_v=1 ./bin/mesos-tests.sh --gtest_filter="*Cgroups*" --verbose
Thanks,
Vinod Kone
Re: Review Request: Refactored cgroups tests and made root cgroup used by
cgroups isolation module configurable.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9814/
-----------------------------------------------------------
(Updated March 8, 2013, 10:43 p.m.)
Review request for mesos, Benjamin Hindman and Ben Mahler.
Changes
-------
benm's.
Description
-------
Pulled out some cgroups test fixtures into test utils, and made them generic, so that they can be re-used by other tests.
Also the root cgroup can be configured via flag, helping cgroups isolation module related tests to use a different cgroup (e.g. "mesos_tests").
NOTE: This is based of my earlier cgroups review: https://reviews.apache.org/r/9806/
Diffs (updated)
-----
src/slave/cgroups_isolation_module.cpp c7ca32c8122ce97f77a5ba68f6ad3dc1f17704f7
src/slave/flags.hpp 9cd436cae5772b906b3566eb1f344904d5894214
src/tests/cgroups_tests.cpp ed419eb174a095786fb6de9e181d42587e5b40e2
src/tests/utils.hpp 0d4edad4787f92f86b1486b6b510f46a62d7d541
Diff: https://reviews.apache.org/r/9814/diff/
Testing
-------
sudo GLOG_v=1 ./bin/mesos-tests.sh --gtest_filter="*Cgroups*" --verbose
Thanks,
Vinod Kone
Re: Review Request: Refactored cgroups tests and made root cgroup used by
cgroups isolation module configurable.
Posted by Vinod Kone <vi...@gmail.com>.
> On March 8, 2013, 9:58 p.m., Ben Mahler wrote:
> > Wonder how I could make use of these in my templated Isolator tests, only when instantiated with CgroupsIsolator. Maybe I'll have to make push most of the testing logic into a test fixture that I can re-use, as you mentioned.
I'll have to do this for slave recovery tests too. I'll let you know what I come up with.
> On March 8, 2013, 9:58 p.m., Ben Mahler wrote:
> > src/tests/utils.hpp, line 159
> > <https://reviews.apache.org/r/9814/diff/3/?file=268413#file268413line159>
> >
> > So the setup runs the teardown? Maybe a comment as to why?
done.
- Vinod
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9814/#review17632
-----------------------------------------------------------
On March 8, 2013, 7:45 p.m., Vinod Kone wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9814/
> -----------------------------------------------------------
>
> (Updated March 8, 2013, 7:45 p.m.)
>
>
> Review request for mesos, Benjamin Hindman and Ben Mahler.
>
>
> Description
> -------
>
> Pulled out some cgroups test fixtures into test utils, and made them generic, so that they can be re-used by other tests.
>
> Also the root cgroup can be configured via flag, helping cgroups isolation module related tests to use a different cgroup (e.g. "mesos_tests").
>
> NOTE: This is based of my earlier cgroups review: https://reviews.apache.org/r/9806/
>
>
> Diffs
> -----
>
> src/slave/cgroups_isolation_module.cpp c7ca32c8122ce97f77a5ba68f6ad3dc1f17704f7
> src/slave/flags.hpp 9cd436cae5772b906b3566eb1f344904d5894214
> src/tests/cgroups_tests.cpp ed419eb174a095786fb6de9e181d42587e5b40e2
> src/tests/utils.hpp 0d4edad4787f92f86b1486b6b510f46a62d7d541
>
> Diff: https://reviews.apache.org/r/9814/diff/
>
>
> Testing
> -------
>
> sudo GLOG_v=1 ./bin/mesos-tests.sh --gtest_filter="*Cgroups*" --verbose
>
>
> Thanks,
>
> Vinod Kone
>
>
Re: Review Request: Refactored cgroups tests and made root cgroup used by
cgroups isolation module configurable.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9814/#review17632
-----------------------------------------------------------
Ship it!
Wonder how I could make use of these in my templated Isolator tests, only when instantiated with CgroupsIsolator. Maybe I'll have to make push most of the testing logic into a test fixture that I can re-use, as you mentioned.
src/tests/utils.hpp
<https://reviews.apache.org/r/9814/#comment37391>
So the setup runs the teardown? Maybe a comment as to why?
- Ben Mahler
On March 8, 2013, 7:45 p.m., Vinod Kone wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9814/
> -----------------------------------------------------------
>
> (Updated March 8, 2013, 7:45 p.m.)
>
>
> Review request for mesos, Benjamin Hindman and Ben Mahler.
>
>
> Description
> -------
>
> Pulled out some cgroups test fixtures into test utils, and made them generic, so that they can be re-used by other tests.
>
> Also the root cgroup can be configured via flag, helping cgroups isolation module related tests to use a different cgroup (e.g. "mesos_tests").
>
> NOTE: This is based of my earlier cgroups review: https://reviews.apache.org/r/9806/
>
>
> Diffs
> -----
>
> src/slave/cgroups_isolation_module.cpp c7ca32c8122ce97f77a5ba68f6ad3dc1f17704f7
> src/slave/flags.hpp 9cd436cae5772b906b3566eb1f344904d5894214
> src/tests/cgroups_tests.cpp ed419eb174a095786fb6de9e181d42587e5b40e2
> src/tests/utils.hpp 0d4edad4787f92f86b1486b6b510f46a62d7d541
>
> Diff: https://reviews.apache.org/r/9814/diff/
>
>
> Testing
> -------
>
> sudo GLOG_v=1 ./bin/mesos-tests.sh --gtest_filter="*Cgroups*" --verbose
>
>
> Thanks,
>
> Vinod Kone
>
>
Re: Review Request: Refactored cgroups tests and made root cgroup used by
cgroups isolation module configurable.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9814/
-----------------------------------------------------------
(Updated March 8, 2013, 7:45 p.m.)
Review request for mesos, Benjamin Hindman and Ben Mahler.
Changes
-------
Changed some CHECK_SOMEs to EXITs and incorporated BenM's changes from https://reviews.apache.org/r/9790/.
Description
-------
Pulled out some cgroups test fixtures into test utils, and made them generic, so that they can be re-used by other tests.
Also the root cgroup can be configured via flag, helping cgroups isolation module related tests to use a different cgroup (e.g. "mesos_tests").
NOTE: This is based of my earlier cgroups review: https://reviews.apache.org/r/9806/
Diffs (updated)
-----
src/slave/cgroups_isolation_module.cpp c7ca32c8122ce97f77a5ba68f6ad3dc1f17704f7
src/slave/flags.hpp 9cd436cae5772b906b3566eb1f344904d5894214
src/tests/cgroups_tests.cpp ed419eb174a095786fb6de9e181d42587e5b40e2
src/tests/utils.hpp 0d4edad4787f92f86b1486b6b510f46a62d7d541
Diff: https://reviews.apache.org/r/9814/diff/
Testing
-------
sudo GLOG_v=1 ./bin/mesos-tests.sh --gtest_filter="*Cgroups*" --verbose
Thanks,
Vinod Kone
Re: Review Request: Refactored cgroups tests and made root cgroup used by
cgroups isolation module configurable.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9814/
-----------------------------------------------------------
(Updated March 8, 2013, 7:22 p.m.)
Review request for mesos, Benjamin Hindman and Ben Mahler.
Changes
-------
Renamed variable names per BenH's review.
Description
-------
Pulled out some cgroups test fixtures into test utils, and made them generic, so that they can be re-used by other tests.
Also the root cgroup can be configured via flag, helping cgroups isolation module related tests to use a different cgroup (e.g. "mesos_tests").
NOTE: This is based of my earlier cgroups review: https://reviews.apache.org/r/9806/
Diffs (updated)
-----
src/slave/cgroups_isolation_module.cpp c7ca32c8122ce97f77a5ba68f6ad3dc1f17704f7
src/slave/flags.hpp 9cd436cae5772b906b3566eb1f344904d5894214
src/tests/cgroups_tests.cpp ed419eb174a095786fb6de9e181d42587e5b40e2
src/tests/utils.hpp 0d4edad4787f92f86b1486b6b510f46a62d7d541
Diff: https://reviews.apache.org/r/9814/diff/
Testing
-------
sudo GLOG_v=1 ./bin/mesos-tests.sh --gtest_filter="*Cgroups*" --verbose
Thanks,
Vinod Kone