[1/2] mesos git commit: Improved performance of cgroups::read by verifying after failure.
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
Authored: Mon Jul 16 17:22:45 2018 -0700
Committer: Benjamin Mahler
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 read(
const string& cgroup,
const string& control)
{
- Option error = verify(hierarchy, cgroup, control);
- if (error.isSome()) {
-return error.get();
+ Try 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 = verify(hierarchy, cgroup, control);
+if (error.isSome()) {
+ return error.get();
+}
}
- return internal::read(hierarchy, cgroup, control);
+ return read;
}
[1/2] mesos git commit: Improved performance of cgroups::read by verifying after failure.
Repository: mesos
Updated Branches:
refs/heads/1.5.x eee8f2124 -> 6e2dfd06c
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/f2be36d4
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/f2be36d4
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/f2be36d4
Branch: refs/heads/1.5.x
Commit: f2be36d4a57206b2c53a0f42b2e392ccd65c8c98
Parents: eee8f21
Author: Benjamin Mahler
Authored: Mon Jul 16 17:22:45 2018 -0700
Committer: Benjamin Mahler
Committed: Mon Jul 16 17:36:31 2018 -0700
--
src/linux/cgroups.cpp | 21 +
1 file changed, 17 insertions(+), 4 deletions(-)
--
http://git-wip-us.apache.org/repos/asf/mesos/blob/f2be36d4/src/linux/cgroups.cpp
--
diff --git a/src/linux/cgroups.cpp b/src/linux/cgroups.cpp
index cdd0f40..96ea289 100644
--- a/src/linux/cgroups.cpp
+++ b/src/linux/cgroups.cpp
@@ -948,12 +948,25 @@ Try read(
const string& cgroup,
const string& control)
{
- Option error = verify(hierarchy, cgroup, control);
- if (error.isSome()) {
-return error.get();
+ Try 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 = verify(hierarchy, cgroup, control);
+if (error.isSome()) {
+ return error.get();
+}
}
- return internal::read(hierarchy, cgroup, control);
+ return read;
}
[1/2] mesos git commit: Improved performance of cgroups::read by verifying after failure.
Repository: mesos
Updated Branches:
refs/heads/1.4.x 5f6d4cdc7 -> 163dd77ef
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/4fc17599
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/4fc17599
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/4fc17599
Branch: refs/heads/1.4.x
Commit: 4fc1759965b8a4ae00f70a05406a3b93e52150dc
Parents: 5f6d4cd
Author: Benjamin Mahler
Authored: Mon Jul 16 17:22:45 2018 -0700
Committer: Benjamin Mahler
Committed: Mon Jul 16 17:36:08 2018 -0700
--
src/linux/cgroups.cpp | 21 +
1 file changed, 17 insertions(+), 4 deletions(-)
--
http://git-wip-us.apache.org/repos/asf/mesos/blob/4fc17599/src/linux/cgroups.cpp
--
diff --git a/src/linux/cgroups.cpp b/src/linux/cgroups.cpp
index 0a7bb89..ab594e0 100644
--- a/src/linux/cgroups.cpp
+++ b/src/linux/cgroups.cpp
@@ -946,12 +946,25 @@ Try read(
const string& cgroup,
const string& control)
{
- Option error = verify(hierarchy, cgroup, control);
- if (error.isSome()) {
-return error.get();
+ Try 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 = verify(hierarchy, cgroup, control);
+if (error.isSome()) {
+ return error.get();
+}
}
- return internal::read(hierarchy, cgroup, control);
+ return read;
}
