You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by ya...@apache.org on 2016/10/20 18:26:47 UTC
mesos git commit: Treat failure in destroy of non-existing nested
cgroups as a success.
Repository: mesos
Updated Branches:
refs/heads/master 4c25f17d8 -> fcdc547d7
Treat failure in destroy of non-existing nested cgroups as a success.
This may happen if certain daemon on the host creates and deletes
nested cgroups inside Mesos containers but the nested cgroups are not
managed by Mesos; we shouldn't fail the destroy. Ultimately the
rationale for this change is that if the goal is to destroy a cgroup
and the end result is that it's gone, then we should consider it a
success even though some nested cgroups are cleaned up out-of-band
by other entities.
Review: https://reviews.apache.org/r/53031/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/fcdc547d
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/fcdc547d
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/fcdc547d
Branch: refs/heads/master
Commit: fcdc547d720cbb962ec2e498a70fbadd3cf75397
Parents: 4c25f17
Author: Anindya Sinha <an...@apple.com>
Authored: Thu Oct 20 11:05:37 2016 -0700
Committer: Jiang Yan Xu <xu...@apple.com>
Committed: Thu Oct 20 11:26:29 2016 -0700
----------------------------------------------------------------------
src/linux/cgroups.cpp | 35 ++++++++++++++++++++++++++++-------
1 file changed, 28 insertions(+), 7 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/fcdc547d/src/linux/cgroups.cpp
----------------------------------------------------------------------
diff --git a/src/linux/cgroups.cpp b/src/linux/cgroups.cpp
index 95e374d..7d62b02 100644
--- a/src/linux/cgroups.cpp
+++ b/src/linux/cgroups.cpp
@@ -1630,14 +1630,25 @@ private:
terminate(self());
return;
} else if (future.isFailed()) {
- promise.fail(future.failure());
+ // If the `cgroup` still exists in the hierarchy, treat this as
+ // an error; otherwise, treat this as a success since the `cgroup`
+ // has actually been cleaned up.
+ if (os::exists(path::join(hierarchy, cgroup))) {
+ promise.fail(future.failure());
+ } else {
+ promise.set(Nothing());
+ }
+
terminate(self());
return;
}
// Verify the cgroup is now empty.
Try<set<pid_t>> processes = cgroups::processes(hierarchy, cgroup);
- if (processes.isError() || !processes.get().empty()) {
+
+ // If the `cgroup` is already removed, treat this as a success.
+ if ((processes.isError() || !processes.get().empty()) &&
+ os::exists(path::join(hierarchy, cgroup))) {
promise.fail("Failed to kill all processes in cgroup: " +
(processes.isError() ? processes.error()
: "processes remain"));
@@ -1718,10 +1729,15 @@ private:
foreach (const string& cgroup, cgroups) {
Try<Nothing> remove = internal::remove(hierarchy, cgroup);
if (remove.isError()) {
- promise.fail(
- "Failed to remove cgroup '" + cgroup + "': " + remove.error());
- terminate(self());
- return;
+ // If the `cgroup` still exists in the hierarchy, treat this as
+ // an error; otherwise, treat this as a success since the `cgroup`
+ // has actually been cleaned up.
+ if (os::exists(path::join(hierarchy, cgroup))) {
+ promise.fail(
+ "Failed to remove cgroup '" + cgroup + "': " + remove.error());
+ terminate(self());
+ return;
+ }
}
}
@@ -1771,7 +1787,12 @@ Future<Nothing> destroy(const string& hierarchy, const string& cgroup)
foreach (const string& cgroup, candidates) {
Try<Nothing> remove = cgroups::remove(hierarchy, cgroup);
if (remove.isError()) {
- return Failure(remove.error());
+ // If the `cgroup` still exists in the hierarchy, treat this as
+ // an error; otherwise, treat this as a success since the `cgroup`
+ // has actually been cleaned up.
+ if (os::exists(path::join(hierarchy, cgroup))) {
+ return Failure(remove.error());
+ }
}
}
}