Repository: mesos Updated Branches: refs/heads/1.6.x ae82dd5cc -> 2f6f3812c
Improved performance of cgroups::read by verifying after failure. 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. Review: https://reviews.apache.org/r/67923/ Project: http://git-wip-us.apache.org/repos/asf/mesos/repo Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/9129ff1b Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/9129ff1b Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/9129ff1b Branch: refs/heads/1.6.x Commit: 9129ff1b89c0cf22891d2d650e234340e9a2014a Parents: ae82dd5 Author: Benjamin Mahler <bmah...@apache.org> Authored: Mon Jul 16 17:22:45 2018 -0700 Committer: Benjamin Mahler <bmah...@apache.org> Committed: Mon Jul 16 17:36:26 2018 -0700 ---------------------------------------------------------------------- src/linux/cgroups.cpp | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mesos/blob/9129ff1b/src/linux/cgroups.cpp ---------------------------------------------------------------------- diff --git a/src/linux/cgroups.cpp b/src/linux/cgroups.cpp index 847f116..04cac90 100644 --- a/src/linux/cgroups.cpp +++ b/src/linux/cgroups.cpp @@ -919,12 +919,25 @@ Try<string> read( const string& cgroup, const string& control) { - Option<Error> error = verify(hierarchy, cgroup, control); - if (error.isSome()) { - return error.get(); + Try<string> read = internal::read(hierarchy, cgroup, control); + + // It turns out that `verify()` is expensive, so rather than + // verifying *prior* to the read, as is currently done for other + // cgroup helpers, we only verify if the read fails. This ensures + // we still provide a good error message. See: MESOS-8418. + // + // TODO(bmahler): Longer term, verification should be done + // explicitly by the caller, or its performance should be + // improved, or verification could be done consistently + // post-failure, as done here. + if (read.isError()) { + Option<Error> error = verify(hierarchy, cgroup, control); + if (error.isSome()) { + return error.get(); + } } - return internal::read(hierarchy, cgroup, control); + return read; }
