You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Joris Van Remoortere <jo...@gmail.com> on 2015/07/26 21:02:34 UTC
Review Request 36823: Used std::thread instead of pthread for cgroups
tests.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36823/
-----------------------------------------------------------
Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Michael Park.
Bugs: MESOS-3120
https://issues.apache.org/jira/browse/MESOS-3120
Repository: mesos
Description
-------
See summary.
Diffs
-----
src/tests/containerizer/cgroups_tests.cpp caecd5dfa3fef33dba35cfc1b5934a11e2cc961a
Diff: https://reviews.apache.org/r/36823/diff/
Testing
-------
sudo ./bin/mesos-tests.sh --gtest_filter="CgroupsAnyHierarchyWithFreezerTest.*"
Thanks,
Joris Van Remoortere
Re: Review Request 36823: Used std::thread instead of pthread for
cgroups tests.
Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36823/#review93160
-----------------------------------------------------------
src/tests/containerizer/cgroups_tests.cpp (line 817)
<https://reviews.apache.org/r/36823/#comment147428>
Mind moving this up to the previous line?
src/tests/containerizer/cgroups_tests.cpp (lines 820 - 824)
<https://reviews.apache.org/r/36823/#comment147429>
I'm not convinced that we will _always_ start all threads before doing 'cond.notify_all()' in the main thread, which could mean that when we 'join' we'll wait forever. Technically the test would fail in this circumstance (and I haven't heard about this test being flaky), but at least the test would always clean up in those cases where as now we might block forever. What about just using a 'Latch' here that we open trigger the latch then any stragglers will just fall through?
- Benjamin Hindman
On July 27, 2015, 6:57 a.m., Joris Van Remoortere wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36823/
> -----------------------------------------------------------
>
> (Updated July 27, 2015, 6:57 a.m.)
>
>
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Michael Park.
>
>
> Bugs: MESOS-3120
> https://issues.apache.org/jira/browse/MESOS-3120
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/tests/containerizer/cgroups_tests.cpp caecd5dfa3fef33dba35cfc1b5934a11e2cc961a
>
> Diff: https://reviews.apache.org/r/36823/diff/
>
>
> Testing
> -------
>
> sudo ./bin/mesos-tests.sh --gtest_filter="CgroupsAnyHierarchyWithFreezerTest.*"
>
>
> Thanks,
>
> Joris Van Remoortere
>
>
Re: Review Request 36823: Used std::thread instead of pthread for
cgroups tests.
Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36823/#review93527
-----------------------------------------------------------
Ship it!
src/tests/containerizer/cgroups_tests.cpp (line 814)
<https://reviews.apache.org/r/36823/#comment147912>
We don't use '&' as a default capture, didn't you write that style guide rule? ;-)
So, I'll just swap for a 'Latch*' instead of an Owned<Latch> for you before committing, thanks!
- Benjamin Hindman
On July 27, 2015, 8:35 p.m., Joris Van Remoortere wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36823/
> -----------------------------------------------------------
>
> (Updated July 27, 2015, 8:35 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Michael Park.
>
>
> Bugs: MESOS-3120
> https://issues.apache.org/jira/browse/MESOS-3120
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/tests/containerizer/cgroups_tests.cpp caecd5dfa3fef33dba35cfc1b5934a11e2cc961a
>
> Diff: https://reviews.apache.org/r/36823/diff/
>
>
> Testing
> -------
>
> sudo ./bin/mesos-tests.sh --gtest_filter="CgroupsAnyHierarchyWithFreezerTest.*"
>
>
> Thanks,
>
> Joris Van Remoortere
>
>
Re: Review Request 36823: Used std::thread instead of pthread for
cgroups tests.
Posted by Joris Van Remoortere <jo...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36823/
-----------------------------------------------------------
(Updated July 27, 2015, 8:35 p.m.)
Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Michael Park.
Changes
-------
remove unused include
Bugs: MESOS-3120
https://issues.apache.org/jira/browse/MESOS-3120
Repository: mesos
Description
-------
See summary.
Diffs (updated)
-----
src/tests/containerizer/cgroups_tests.cpp caecd5dfa3fef33dba35cfc1b5934a11e2cc961a
Diff: https://reviews.apache.org/r/36823/diff/
Testing
-------
sudo ./bin/mesos-tests.sh --gtest_filter="CgroupsAnyHierarchyWithFreezerTest.*"
Thanks,
Joris Van Remoortere
Re: Review Request 36823: Used std::thread instead of pthread for
cgroups tests.
Posted by Joris Van Remoortere <jo...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36823/
-----------------------------------------------------------
(Updated July 27, 2015, 8:11 p.m.)
Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Michael Park.
Changes
-------
Switched to use Latch instead of condition variable.
Bugs: MESOS-3120
https://issues.apache.org/jira/browse/MESOS-3120
Repository: mesos
Description
-------
See summary.
Diffs (updated)
-----
src/tests/containerizer/cgroups_tests.cpp caecd5dfa3fef33dba35cfc1b5934a11e2cc961a
Diff: https://reviews.apache.org/r/36823/diff/
Testing
-------
sudo ./bin/mesos-tests.sh --gtest_filter="CgroupsAnyHierarchyWithFreezerTest.*"
Thanks,
Joris Van Remoortere
Re: Review Request 36823: Used std::thread instead of pthread for
cgroups tests.
Posted by Joris Van Remoortere <jo...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36823/
-----------------------------------------------------------
(Updated July 27, 2015, 6:57 a.m.)
Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Michael Park.
Changes
-------
Addressed issues.
Bugs: MESOS-3120
https://issues.apache.org/jira/browse/MESOS-3120
Repository: mesos
Description
-------
See summary.
Diffs (updated)
-----
src/tests/containerizer/cgroups_tests.cpp caecd5dfa3fef33dba35cfc1b5934a11e2cc961a
Diff: https://reviews.apache.org/r/36823/diff/
Testing
-------
sudo ./bin/mesos-tests.sh --gtest_filter="CgroupsAnyHierarchyWithFreezerTest.*"
Thanks,
Joris Van Remoortere
Re: Review Request 36823: Used std::thread instead of pthread for
cgroups tests.
Posted by Artem Harutyunyan <ar...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36823/#review93084
-----------------------------------------------------------
Ship it!
src/tests/containerizer/cgroups_tests.cpp (line 807)
<https://reviews.apache.org/r/36823/#comment147343>
const?
- Artem Harutyunyan
On July 26, 2015, 12:02 p.m., Joris Van Remoortere wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36823/
> -----------------------------------------------------------
>
> (Updated July 26, 2015, 12:02 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Michael Park.
>
>
> Bugs: MESOS-3120
> https://issues.apache.org/jira/browse/MESOS-3120
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/tests/containerizer/cgroups_tests.cpp caecd5dfa3fef33dba35cfc1b5934a11e2cc961a
>
> Diff: https://reviews.apache.org/r/36823/diff/
>
>
> Testing
> -------
>
> sudo ./bin/mesos-tests.sh --gtest_filter="CgroupsAnyHierarchyWithFreezerTest.*"
>
>
> Thanks,
>
> Joris Van Remoortere
>
>