You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by bm...@apache.org on 2013/12/04 01:25:23 UTC
[4/4] git commit: Fixed cgroups code to use processes rather than
threads.
Fixed cgroups code to use processes rather than threads.
The cgroup 'tasks' file lists the threads in a cgroup.
We should use the 'cgroup.procs' file which lists processes.
See: https://issues.apache.org/jira/browse/MESOS-859
From: Ian Downes <ia...@gmail.com>
Review: https://reviews.apache.org/r/15954
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/d0cb03f9
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/d0cb03f9
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/d0cb03f9
Branch: refs/heads/master
Commit: d0cb03f9535ae1b6997f60f4de50ca96f6b19e70
Parents: fb9edfd
Author: Benjamin Mahler <bm...@twitter.com>
Authored: Tue Dec 3 14:24:15 2013 -0800
Committer: Benjamin Mahler <bm...@twitter.com>
Committed: Tue Dec 3 14:24:15 2013 -0800
----------------------------------------------------------------------
src/linux/cgroups.cpp | 25 +++++++++++++++----------
src/linux/cgroups.hpp | 2 +-
src/tests/cgroups_tests.cpp | 4 ++--
3 files changed, 18 insertions(+), 13 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/d0cb03f9/src/linux/cgroups.cpp
----------------------------------------------------------------------
diff --git a/src/linux/cgroups.cpp b/src/linux/cgroups.cpp
index 5a95e75..19ab1f3 100644
--- a/src/linux/cgroups.cpp
+++ b/src/linux/cgroups.cpp
@@ -829,9 +829,9 @@ Try<Nothing> kill(
return error.get();
}
- Try<set<pid_t> > pids = tasks(hierarchy, cgroup);
+ Try<set<pid_t> > pids = processes(hierarchy, cgroup);
if (pids.isError()) {
- return Error("Failed to get tasks of cgroup: " + pids.error());
+ return Error("Failed to get processes of cgroup: " + pids.error());
}
foreach (pid_t pid, pids.get()) {
@@ -894,14 +894,19 @@ Try<bool> exists(
}
-Try<set<pid_t> > tasks(const string& hierarchy, const string& cgroup)
+Try<set<pid_t> > processes(const string& hierarchy, const string& cgroup)
{
- Try<string> value = cgroups::read(hierarchy, cgroup, "tasks");
+ // Note: (from cgroups/cgroups.txt documentation)
+ // cgroup.procs: list of thread group IDs in the cgroup. This list is not
+ // guaranteed to be sorted or free of duplicate TGIDs, and userspace should
+ // sort/uniquify the list if this property is required.
+ Try<string> value = cgroups::read(hierarchy, cgroup, "cgroup.procs");
if (value.isError()) {
- return Error("Failed to read cgroups control 'tasks': " + value.error());
+ return Error("Failed to read cgroups control 'cgroup.procs': " + value.error());
}
- // Parse the value read from the control file.
+ // Parse the values read from the control file and insert into a set. This
+ // ensures they are unique (and also sorted).
set<pid_t> pids;
std::istringstream ss(value.get());
ss >> std::dec;
@@ -1255,9 +1260,9 @@ private:
// make sure that the freezer can finish.
// TODO(jieyu): This code can be removed in the future as the newer
// version of the kernel solves this problem (e.g. Linux-3.2.0).
- Try<set<pid_t> > pids = tasks(hierarchy, cgroup);
+ Try<set<pid_t> > pids = processes(hierarchy, cgroup);
if (pids.isError()) {
- promise.fail("Failed to get tasks of cgroup: " + pids.error());
+ promise.fail("Failed to get processes of cgroup: " + pids.error());
terminate(self());
return;
}
@@ -1453,9 +1458,9 @@ protected:
private:
void check(unsigned int attempt = 0)
{
- Try<set<pid_t> > pids = tasks(hierarchy, cgroup);
+ Try<set<pid_t> > pids = processes(hierarchy, cgroup);
if (pids.isError()) {
- promise.fail("Failed to get tasks of cgroup: " + pids.error());
+ promise.fail("Failed to get processes of cgroup: " + pids.error());
terminate(self());
return;
}
http://git-wip-us.apache.org/repos/asf/mesos/blob/d0cb03f9/src/linux/cgroups.hpp
----------------------------------------------------------------------
diff --git a/src/linux/cgroups.hpp b/src/linux/cgroups.hpp
index 6f60c43..cefa476 100644
--- a/src/linux/cgroups.hpp
+++ b/src/linux/cgroups.hpp
@@ -256,7 +256,7 @@ Try<bool> exists(
// @param hierarchy Path to the hierarchy root.
// @param cgroup Path to the cgroup relative to the hierarchy root.
// @return The set of process ids.
-Try<std::set<pid_t> > tasks(
+Try<std::set<pid_t> > processes(
const std::string& hierarchy,
const std::string& cgroup);
http://git-wip-us.apache.org/repos/asf/mesos/blob/d0cb03f9/src/tests/cgroups_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/cgroups_tests.cpp b/src/tests/cgroups_tests.cpp
index bc5f961..0e9316d 100644
--- a/src/tests/cgroups_tests.cpp
+++ b/src/tests/cgroups_tests.cpp
@@ -338,7 +338,7 @@ TEST_F(CgroupsAnyHierarchyTest, ROOT_CGROUPS_NestedCgroups)
TEST_F(CgroupsAnyHierarchyTest, ROOT_CGROUPS_Tasks)
{
- Try<std::set<pid_t> > pids = cgroups::tasks(hierarchy, "/");
+ Try<std::set<pid_t> > pids = cgroups::processes(hierarchy, "/");
ASSERT_SOME(pids);
EXPECT_NE(0u, pids.get().count(1));
EXPECT_NE(0u, pids.get().count(::getpid()));
@@ -370,7 +370,7 @@ TEST_F(CgroupsAnyHierarchyTest, ROOT_CGROUPS_Write)
ASSERT_SOME(
cgroups::write(hierarchy, TEST_CGROUPS_ROOT, "tasks", stringify(pid)));
- Try<std::set<pid_t> > pids = cgroups::tasks(hierarchy, TEST_CGROUPS_ROOT);
+ Try<std::set<pid_t> > pids = cgroups::processes(hierarchy, TEST_CGROUPS_ROOT);
ASSERT_SOME(pids);
EXPECT_NE(0u, pids.get().count(pid));