You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Benjamin Mahler <bm...@apache.org> on 2018/07/16 02:03:43 UTC
Review Request 67923: Improved performance of cgroups::read by
verifying after failure.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67923/
-----------------------------------------------------------
Review request for mesos, Gilbert Song and Jie Yu.
Bugs: MESOS-8418
https://issues.apache.org/jira/browse/MESOS-8418
Repository: mesos
Description
-------
It turns out that cgroups::verify is expensive and leads to a severe
performance issue on the agent during container metrics collection
if there are a lot of containers on the agent. See MESOS-8418.
Since cgroups::verify serves to provide a helpful error message,
this patch preserves the error message, but only if the read fails.
Longer term, there probably needs to be some re-structuring of the
code to make verification caller-controlled, or perhaps the verify
code can occur consistently post-operation (as done in this patch),
or perhaps verify can be optimized substantially.
Diffs
-----
src/linux/cgroups.cpp b12e63c112a7aa7a6f7150359ff5409f8214067e
Diff: https://reviews.apache.org/r/67923/diff/1/
Testing
-------
Ran through internal CI.
Thanks,
Benjamin Mahler
Re: Review Request 67923: Improved performance of cgroups::read by
verifying after failure.
Posted by Benjamin Mahler <bm...@apache.org>.
> On July 16, 2018, 8:54 p.m., Jie Yu wrote:
> > LGTM! Thanks Ben. I think longer term, we will modify caller to call `verify`, making read/write/create pure helper for writing to cgroup filesystem.
Yeah, I'll copy the longer options from the commit description into a TODO so it's clear to any readers.
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67923/#review206128
-----------------------------------------------------------
On July 16, 2018, 2:03 a.m., Benjamin Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67923/
> -----------------------------------------------------------
>
> (Updated July 16, 2018, 2:03 a.m.)
>
>
> Review request for mesos, Gilbert Song and Jie Yu.
>
>
> Bugs: MESOS-8418
> https://issues.apache.org/jira/browse/MESOS-8418
>
>
> Repository: mesos
>
>
> Description
> -------
>
> It turns out that cgroups::verify is expensive and leads to a severe
> performance issue on the agent during container metrics collection
> if there are a lot of containers on the agent. See MESOS-8418.
>
> Since cgroups::verify serves to provide a helpful error message,
> this patch preserves the error message, but only if the read fails.
>
> Longer term, there probably needs to be some re-structuring of the
> code to make verification caller-controlled, or perhaps the verify
> code can occur consistently post-operation (as done in this patch),
> or perhaps verify can be optimized substantially.
>
>
> Diffs
> -----
>
> src/linux/cgroups.cpp b12e63c112a7aa7a6f7150359ff5409f8214067e
>
>
> Diff: https://reviews.apache.org/r/67923/diff/1/
>
>
> Testing
> -------
>
> Ran through internal CI.
>
>
> Thanks,
>
> Benjamin Mahler
>
>
Re: Review Request 67923: Improved performance of cgroups::read by
verifying after failure.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67923/#review206128
-----------------------------------------------------------
Ship it!
LGTM! Thanks Ben. I think longer term, we will modify caller to call `verify`, making read/write/create pure helper for writing to cgroup filesystem.
- Jie Yu
On July 16, 2018, 2:03 a.m., Benjamin Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67923/
> -----------------------------------------------------------
>
> (Updated July 16, 2018, 2:03 a.m.)
>
>
> Review request for mesos, Gilbert Song and Jie Yu.
>
>
> Bugs: MESOS-8418
> https://issues.apache.org/jira/browse/MESOS-8418
>
>
> Repository: mesos
>
>
> Description
> -------
>
> It turns out that cgroups::verify is expensive and leads to a severe
> performance issue on the agent during container metrics collection
> if there are a lot of containers on the agent. See MESOS-8418.
>
> Since cgroups::verify serves to provide a helpful error message,
> this patch preserves the error message, but only if the read fails.
>
> Longer term, there probably needs to be some re-structuring of the
> code to make verification caller-controlled, or perhaps the verify
> code can occur consistently post-operation (as done in this patch),
> or perhaps verify can be optimized substantially.
>
>
> Diffs
> -----
>
> src/linux/cgroups.cpp b12e63c112a7aa7a6f7150359ff5409f8214067e
>
>
> Diff: https://reviews.apache.org/r/67923/diff/1/
>
>
> Testing
> -------
>
> Ran through internal CI.
>
>
> Thanks,
>
> Benjamin Mahler
>
>
Re: Review Request 67923: Improved performance of cgroups::read by
verifying after failure.
Posted by Stéphane Cottin via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67923/#review206095
-----------------------------------------------------------
Ship it!
Ship It!
- Stéphane Cottin
On July 16, 2018, 2:03 a.m., Benjamin Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67923/
> -----------------------------------------------------------
>
> (Updated July 16, 2018, 2:03 a.m.)
>
>
> Review request for mesos, Gilbert Song and Jie Yu.
>
>
> Bugs: MESOS-8418
> https://issues.apache.org/jira/browse/MESOS-8418
>
>
> Repository: mesos
>
>
> Description
> -------
>
> It turns out that cgroups::verify is expensive and leads to a severe
> performance issue on the agent during container metrics collection
> if there are a lot of containers on the agent. See MESOS-8418.
>
> Since cgroups::verify serves to provide a helpful error message,
> this patch preserves the error message, but only if the read fails.
>
> Longer term, there probably needs to be some re-structuring of the
> code to make verification caller-controlled, or perhaps the verify
> code can occur consistently post-operation (as done in this patch),
> or perhaps verify can be optimized substantially.
>
>
> Diffs
> -----
>
> src/linux/cgroups.cpp b12e63c112a7aa7a6f7150359ff5409f8214067e
>
>
> Diff: https://reviews.apache.org/r/67923/diff/1/
>
>
> Testing
> -------
>
> Ran through internal CI.
>
>
> Thanks,
>
> Benjamin Mahler
>
>
Re: Review Request 67923: Improved performance of cgroups::read by
verifying after failure.
Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67923/#review206093
-----------------------------------------------------------
PASS: Mesos patch 67923 was successfully built and tested.
Reviews applied: `['67923']`
All the build artifacts available at: http://dcos-win.westus.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/1928/mesos-review-67923
- Mesos Reviewbot Windows
On July 16, 2018, 2:03 a.m., Benjamin Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67923/
> -----------------------------------------------------------
>
> (Updated July 16, 2018, 2:03 a.m.)
>
>
> Review request for mesos, Gilbert Song and Jie Yu.
>
>
> Bugs: MESOS-8418
> https://issues.apache.org/jira/browse/MESOS-8418
>
>
> Repository: mesos
>
>
> Description
> -------
>
> It turns out that cgroups::verify is expensive and leads to a severe
> performance issue on the agent during container metrics collection
> if there are a lot of containers on the agent. See MESOS-8418.
>
> Since cgroups::verify serves to provide a helpful error message,
> this patch preserves the error message, but only if the read fails.
>
> Longer term, there probably needs to be some re-structuring of the
> code to make verification caller-controlled, or perhaps the verify
> code can occur consistently post-operation (as done in this patch),
> or perhaps verify can be optimized substantially.
>
>
> Diffs
> -----
>
> src/linux/cgroups.cpp b12e63c112a7aa7a6f7150359ff5409f8214067e
>
>
> Diff: https://reviews.apache.org/r/67923/diff/1/
>
>
> Testing
> -------
>
> Ran through internal CI.
>
>
> Thanks,
>
> Benjamin Mahler
>
>
Re: Review Request 67923: Improved performance of cgroups::read by
verifying after failure.
Posted by Gilbert Song <so...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67923/#review206142
-----------------------------------------------------------
Ship it!
Ship It!
- Gilbert Song
On July 15, 2018, 7:03 p.m., Benjamin Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67923/
> -----------------------------------------------------------
>
> (Updated July 15, 2018, 7:03 p.m.)
>
>
> Review request for mesos, Gilbert Song and Jie Yu.
>
>
> Bugs: MESOS-8418
> https://issues.apache.org/jira/browse/MESOS-8418
>
>
> Repository: mesos
>
>
> Description
> -------
>
> It turns out that cgroups::verify is expensive and leads to a severe
> performance issue on the agent during container metrics collection
> if there are a lot of containers on the agent. See MESOS-8418.
>
> Since cgroups::verify serves to provide a helpful error message,
> this patch preserves the error message, but only if the read fails.
>
> Longer term, there probably needs to be some re-structuring of the
> code to make verification caller-controlled, or perhaps the verify
> code can occur consistently post-operation (as done in this patch),
> or perhaps verify can be optimized substantially.
>
>
> Diffs
> -----
>
> src/linux/cgroups.cpp b12e63c112a7aa7a6f7150359ff5409f8214067e
>
>
> Diff: https://reviews.apache.org/r/67923/diff/1/
>
>
> Testing
> -------
>
> Ran through internal CI.
>
>
> Thanks,
>
> Benjamin Mahler
>
>