Re: 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/#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 > >
Re: Review Request 67923: Improved performance of cgroups::read by verifying after failure.
> 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.
--- 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.
--- 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.
--- 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 > >
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
