You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by ji...@apache.org on 2018/08/20 18:17:01 UTC
[mesos] branch master updated (fa4de10 -> 4bed583)
This is an automated email from the ASF dual-hosted git repository.
jieyu pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/mesos.git.
from fa4de10 Added MESOS-9116 to the 1.5.2 CHANGELOG.
new 374f3eb Refactored some cgroups helpers to do verify from callers.
new 4bed583 Added MESOS-9081 to 1.7.0 CHANGELOG.
The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
CHANGELOG | 1 +
src/linux/cgroups.cpp | 305 ++++-----------------
src/linux/cgroups.hpp | 269 +++++++++++-------
src/linux/systemd.cpp | 6 +-
.../mesos/isolators/cgroups/cgroups.cpp | 21 +-
.../mesos/isolators/cgroups/subsystems/cpu.cpp | 11 +-
.../containerizer/mesos/isolators/gpu/isolator.cpp | 17 +-
src/slave/containerizer/mesos/linux_launcher.cpp | 27 +-
src/slave/main.cpp | 11 +-
.../nested_mesos_containerizer_tests.cpp | 24 +-
10 files changed, 241 insertions(+), 451 deletions(-)
[mesos] 01/02: Refactored some cgroups helpers to do verify from
callers.
Posted by ji...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
jieyu pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mesos.git
commit 374f3eb5e146a3896eec63bcf051f5e09e07821b
Author: Jie Yu <yu...@gmail.com>
AuthorDate: Mon Aug 6 09:49:51 2018 -0700
Refactored some cgroups helpers to do verify from callers.
Prior to this patch, some of the cgroups helpers call `cgroups::verify`
to make sure the hierarchy and cgroup are valid. This causes some
performance issues for the agent because `cgroups::verify` will read the
entire mount table, and mount table read is expensive if the mount table
is large. See more details in MESOS-8418.
In most of the cases, the verification has already been done when those
cgroups helpers are called. Therefore, it is better to let the caller do
the verification and make those cgroups helpers pure helpers.
Also, the verification is subject to race anyway. The cgroups global
state might change (e.g., operators modify the cgroups) after the
verification is done, making the verification itself not that super
useful.
This patch refactored the cgroups helpers and moved the verifications to
the callers.
Review: https://reviews.apache.org/r/68426
---
src/linux/cgroups.cpp | 305 ++++-----------------
src/linux/cgroups.hpp | 269 +++++++++++-------
src/linux/systemd.cpp | 6 +-
.../mesos/isolators/cgroups/cgroups.cpp | 21 +-
.../mesos/isolators/cgroups/subsystems/cpu.cpp | 11 +-
.../containerizer/mesos/isolators/gpu/isolator.cpp | 17 +-
src/slave/containerizer/mesos/linux_launcher.cpp | 27 +-
src/slave/main.cpp | 11 +-
.../nested_mesos_containerizer_tests.cpp | 24 +-
9 files changed, 240 insertions(+), 451 deletions(-)
diff --git a/src/linux/cgroups.cpp b/src/linux/cgroups.cpp
index 86b0306..95efc1d 100644
--- a/src/linux/cgroups.cpp
+++ b/src/linux/cgroups.cpp
@@ -226,19 +226,6 @@ static Try<Nothing> mount(const string& hierarchy, const string& subsystems)
}
-// Unmount the cgroups virtual file system from the given hierarchy root. Make
-// sure to remove all cgroups in the hierarchy before unmount. This function
-// assumes the given hierarchy is currently mounted with a cgroups virtual file
-// system.
-// @param hierarchy Path to the hierarchy root.
-// @return Some if the operation succeeds.
-// Error if the operation fails.
-static Try<Nothing> unmount(const string& hierarchy)
-{
- return fs::unmount(hierarchy);
-}
-
-
// Copies the value of 'cpuset.cpus' and 'cpuset.mems' from a parent
// cgroup to a child cgroup so the child cgroup can actually run tasks
// (otherwise it gets the error 'Device or resource busy').
@@ -276,109 +263,6 @@ static Try<Nothing> cloneCpusetCpusMems(
return Nothing();
}
-
-// Create a cgroup in a given hierarchy. To create a cgroup, one just
-// need to create a directory in the cgroups virtual file system. The
-// given cgroup is a relative path to the given hierarchy. This
-// function assumes the given hierarchy is valid and is currently
-// mounted with a cgroup virtual file system. The function also
-// assumes the given cgroup is valid.
-// @param hierarchy Path to the hierarchy root.
-// @param cgroup Path to the cgroup relative to the hierarchy root.
-// @param recursive Create nest cgroup structure
-// @return Some if the operation succeeds.
-// Error if the operation fails.
-static Try<Nothing> create(
- const string& hierarchy,
- const string& cgroup,
- bool recursive)
-{
- string path = path::join(hierarchy, cgroup);
- Try<Nothing> mkdir = os::mkdir(path, recursive);
- if (mkdir.isError()) {
- return Error(
- "Failed to create directory '" + path + "': " + mkdir.error());
- }
-
- // Now clone 'cpuset.cpus' and 'cpuset.mems' if the 'cpuset'
- // subsystem is attached to the hierarchy.
- Try<set<string>> attached = cgroups::subsystems(hierarchy);
- if (attached.isError()) {
- return Error(
- "Failed to determine if hierarchy '" + hierarchy +
- "' has the 'cpuset' subsystem attached: " + attached.error());
- } else if (attached->count("cpuset") > 0) {
- string parent = Path(path::join("/", cgroup)).dirname();
- return cloneCpusetCpusMems(hierarchy, parent, cgroup);
- }
-
- return Nothing();
-}
-
-
-// Remove a cgroup in a given hierarchy. To remove a cgroup, one needs
-// to remove the corresponding directory in the cgroups virtual file
-// system. A cgroup cannot be removed if it has processes or
-// sub-cgroups inside. This function does nothing but tries to remove
-// the corresponding directory of the given cgroup. It will return
-// error if the remove operation fails because it has either processes
-// or sub-cgroups inside.
-// @param hierarchy Path to the hierarchy root.
-// @param cgroup Path to the cgroup relative to the hierarchy root.
-// @return Some if the operation succeeds.
-// Error if the operation fails.
-static Try<Nothing> remove(const string& hierarchy, const string& cgroup)
-{
- string path = path::join(hierarchy, cgroup);
-
- // Do NOT recursively remove cgroups.
- Try<Nothing> rmdir = os::rmdir(path, false);
-
- if (rmdir.isError()) {
- return Error(
- "Failed to remove cgroup '" + path + "': " + rmdir.error());
- }
-
- return rmdir;
-}
-
-
-// Read a control file. Control files are the gateway to monitor and
-// control cgroups. This function assumes the cgroups virtual file
-// systems are properly mounted on the given hierarchy, and the given
-// cgroup has been already created properly. The given control file
-// name should also be valid.
-// @param hierarchy Path to the hierarchy root.
-// @param cgroup Path to the cgroup relative to the hierarchy root.
-// @param control Name of the control file.
-// @return The value read from the control file.
-static Try<string> read(
- const string& hierarchy,
- const string& cgroup,
- const string& control)
-{
- string path = path::join(hierarchy, cgroup, control);
- return os::read(path);
-}
-
-
-// Write a control file.
-// @param hierarchy Path to the hierarchy root.
-// @param cgroup Path to the cgroup relative to the hierarchy root.
-// @param control Name of the control file.
-// @param value Value to be written.
-// @return Some if the operation succeeds.
-// Error if the operation fails.
-static Try<Nothing> write(
- const string& hierarchy,
- const string& cgroup,
- const string& control,
- const string& value)
-{
- string path = path::join(hierarchy, cgroup, control);
- return os::write(path, value);
-}
-
} // namespace internal {
@@ -437,15 +321,7 @@ Try<string> prepare(
CHECK_SOME(hierarchy);
// Create the cgroup if it doesn't exist.
- Try<bool> exists = cgroups::exists(hierarchy.get(), cgroup);
- if (exists.isError()) {
- return Error(
- "Failed to check existence of root cgroup " +
- path::join(hierarchy.get(), cgroup) +
- ": " + exists.error());
- }
-
- if (!exists.get()) {
+ if (!cgroups::exists(hierarchy.get(), cgroup)) {
// No cgroup exists, create it.
Try<Nothing> create = cgroups::create(hierarchy.get(), cgroup, true);
if (create.isError()) {
@@ -460,12 +336,10 @@ Try<string> prepare(
}
-// Returns some error string if either (a) hierarchy is not mounted,
-// (b) cgroup does not exist, or (c) control file does not exist.
-static Option<Error> verify(
+Try<Nothing> verify(
const string& hierarchy,
- const string& cgroup = "",
- const string& control = "")
+ const string& cgroup,
+ const string& control)
{
Try<bool> mounted = cgroups::mounted(hierarchy);
if (mounted.isError()) {
@@ -489,7 +363,7 @@ static Option<Error> verify(
}
}
- return None();
+ return Nothing();
}
@@ -708,12 +582,7 @@ Try<Nothing> mount(const string& hierarchy, const string& subsystems, int retry)
Try<Nothing> unmount(const string& hierarchy)
{
- Option<Error> error = verify(hierarchy);
- if (error.isSome()) {
- return error.get();
- }
-
- Try<Nothing> unmount = internal::unmount(hierarchy);
+ Try<Nothing> unmount = fs::unmount(hierarchy);
if (unmount.isError()) {
return unmount;
}
@@ -777,53 +646,52 @@ Try<Nothing> create(
const string& cgroup,
bool recursive)
{
- Option<Error> error = verify(hierarchy);
- if (error.isSome()) {
- return error.get();
+ string path = path::join(hierarchy, cgroup);
+ Try<Nothing> mkdir = os::mkdir(path, recursive);
+ if (mkdir.isError()) {
+ return Error(
+ "Failed to create directory '" + path + "': " + mkdir.error());
+ }
+
+ // Now clone 'cpuset.cpus' and 'cpuset.mems' if the 'cpuset'
+ // subsystem is attached to the hierarchy.
+ Try<set<string>> attached = cgroups::subsystems(hierarchy);
+ if (attached.isError()) {
+ return Error(
+ "Failed to determine if hierarchy '" + hierarchy +
+ "' has the 'cpuset' subsystem attached: " + attached.error());
+ } else if (attached->count("cpuset") > 0) {
+ string parent = Path(path::join("/", cgroup)).dirname();
+ return internal::cloneCpusetCpusMems(hierarchy, parent, cgroup);
}
- return internal::create(hierarchy, cgroup, recursive);
+ return Nothing();
}
Try<Nothing> remove(const string& hierarchy, const string& cgroup)
{
- Option<Error> error = verify(hierarchy, cgroup);
- if (error.isSome()) {
- return error.get();
- }
-
- Try<vector<string>> cgroups = cgroups::get(hierarchy, cgroup);
- if (cgroups.isError()) {
- return Error("Failed to get nested cgroups: " + cgroups.error());
- }
+ string path = path::join(hierarchy, cgroup);
- if (!cgroups->empty()) {
- return Error("Nested cgroups exist");
+ // Do NOT recursively remove cgroups.
+ Try<Nothing> rmdir = os::rmdir(path, false);
+ if (rmdir.isError()) {
+ return Error(
+ "Failed to remove cgroup '" + path + "': " + rmdir.error());
}
- return internal::remove(hierarchy, cgroup);
+ return rmdir;
}
-Try<bool> exists(const string& hierarchy, const string& cgroup)
+bool exists(const string& hierarchy, const string& cgroup)
{
- Option<Error> error = verify(hierarchy);
- if (error.isSome()) {
- return error.get();
- }
-
return os::exists(path::join(hierarchy, cgroup));
}
Try<vector<string>> get(const string& hierarchy, const string& cgroup)
{
- Option<Error> error = verify(hierarchy, cgroup);
- if (error.isSome()) {
- return error.get();
- }
-
Result<string> hierarchyAbsPath = os::realpath(hierarchy);
if (!hierarchyAbsPath.isSome()) {
return Error(
@@ -885,11 +753,6 @@ Try<Nothing> kill(
const string& cgroup,
int signal)
{
- Option<Error> error = verify(hierarchy, cgroup);
- if (error.isSome()) {
- return error.get();
- }
-
Try<set<pid_t>> pids = processes(hierarchy, cgroup);
if (pids.isError()) {
return Error("Failed to get processes of cgroup: " + pids.error());
@@ -917,25 +780,8 @@ Try<string> read(
const string& cgroup,
const string& control)
{
- 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 read;
+ string path = path::join(hierarchy, cgroup, control);
+ return os::read(path);
}
@@ -945,25 +791,16 @@ Try<Nothing> write(
const string& control,
const string& value)
{
- Option<Error> error = verify(hierarchy, cgroup, control);
- if (error.isSome()) {
- return error.get();
- }
-
- return internal::write(hierarchy, cgroup, control, value);
+ string path = path::join(hierarchy, cgroup, control);
+ return os::write(path, value);
}
-Try<bool> exists(
+bool exists(
const string& hierarchy,
const string& cgroup,
const string& control)
{
- Option<Error> error = verify(hierarchy, cgroup);
- if (error.isSome()) {
- return error.get();
- }
-
return os::exists(path::join(hierarchy, cgroup, control));
}
@@ -1038,12 +875,7 @@ Try<Nothing> isolate(
pid_t pid)
{
// Create cgroup if necessary.
- Try<bool> exists = cgroups::exists(hierarchy, cgroup);
- if (exists.isError()) {
- return Error("Failed to check existence of cgroup: " + exists.error());
- }
-
- if (!exists.get()) {
+ if (!cgroups::exists(hierarchy, cgroup)) {
Try<Nothing> create = cgroups::create(hierarchy, cgroup, true);
if (create.isError()) {
return Error("Failed to create cgroup: " + create.error());
@@ -1143,8 +975,13 @@ static Try<int> registerNotifier(
if (args.isSome()) {
out << " " << args.get();
}
- Try<Nothing> write = internal::write(
- hierarchy, cgroup, "cgroup.event_control", out.str());
+
+ Try<Nothing> write = cgroups::write(
+ hierarchy,
+ cgroup,
+ "cgroup.event_control",
+ out.str());
+
if (write.isError()) {
os::close(efd);
os::close(cfd.get());
@@ -1297,11 +1134,6 @@ Future<uint64_t> listen(
const string& control,
const Option<string>& args)
{
- Option<Error> error = verify(hierarchy, cgroup, control);
- if (error.isSome()) {
- return Failure(error.get());
- }
-
Listener* listener = new Listener(hierarchy, cgroup, control, args);
spawn(listener, true);
@@ -1440,13 +1272,6 @@ public:
protected:
void initialize() override
{
- Option<Error> error = verify(hierarchy, cgroup, "freezer.state");
- if (error.isSome()) {
- promise.fail("Invalid freezer cgroup: " + error->message);
- terminate(self());
- return;
- }
-
// Stop attempting to freeze/thaw if nobody cares.
promise.future().onDiscard(lambda::bind(
static_cast<void(*)(const UPID&, bool)>(terminate), self(), true));
@@ -1676,7 +1501,7 @@ private:
void remove()
{
foreach (const string& cgroup, cgroups) {
- Try<Nothing> remove = internal::remove(hierarchy, cgroup);
+ Try<Nothing> remove = cgroups::remove(hierarchy, cgroup);
if (remove.isError()) {
// If the `cgroup` still exists in the hierarchy, treat this as
// an error; otherwise, treat this as a success since the `cgroup`
@@ -1724,8 +1549,7 @@ Future<Nothing> destroy(const string& hierarchy, const string& cgroup)
}
// If the freezer subsystem is available, destroy the cgroups.
- Option<Error> error = verify(hierarchy, cgroup, "freezer.state");
- if (error.isNone()) {
+ if (cgroups::exists(hierarchy, cgroup, "freezer.state")) {
internal::Destroyer* destroyer =
new internal::Destroyer(hierarchy, candidates);
Future<Nothing> future = destroyer->future();
@@ -2447,16 +2271,7 @@ Result<Bytes> memsw_limit_in_bytes(
const string& hierarchy,
const string& cgroup)
{
- Try<bool> exists = cgroups::exists(
- hierarchy, cgroup, "memory.memsw.limit_in_bytes");
-
- if (exists.isError()) {
- return Error(
- "Could not check for existence of 'memory.memsw.limit_in_bytes': " +
- exists.error());
- }
-
- if (!exists.get()) {
+ if (!cgroups::exists(hierarchy, cgroup, "memory.memsw.limit_in_bytes")) {
return None();
}
@@ -2482,16 +2297,7 @@ Try<bool> memsw_limit_in_bytes(
const string& cgroup,
const Bytes& limit)
{
- Try<bool> exists = cgroups::exists(
- hierarchy, cgroup, "memory.memsw.limit_in_bytes");
-
- if (exists.isError()) {
- return Error(
- "Could not check for existence of 'memory.memsw.limit_in_bytes': " +
- exists.error());
- }
-
- if (!exists.get()) {
+ if (!cgroups::exists(hierarchy, cgroup, "memory.memsw.limit_in_bytes")) {
return false;
}
@@ -2587,15 +2393,11 @@ namespace killer {
Try<bool> enabled(const string& hierarchy, const string& cgroup)
{
- Try<bool> exists = cgroups::exists(hierarchy, cgroup, "memory.oom_control");
-
- if (exists.isError() || !exists.get()) {
- return Error("Could not find 'memory.oom_control' control file: " +
- (exists.isError() ? exists.error() : "does not exist"));
+ if (!cgroups::exists(hierarchy, cgroup, "memory.oom_control")) {
+ return Error("Could not find 'memory.oom_control' control file");
}
Try<string> read = cgroups::read(hierarchy, cgroup, "memory.oom_control");
-
if (read.isError()) {
return Error("Could not read 'memory.oom_control' control file: " +
read.error());
@@ -2750,11 +2552,6 @@ Try<Owned<Counter>> Counter::create(
const string& cgroup,
Level level)
{
- Option<Error> error = verify(hierarchy, cgroup);
- if (error.isSome()) {
- return Error(error.get());
- }
-
return Owned<Counter>(new Counter(hierarchy, cgroup, level));
}
diff --git a/src/linux/cgroups.hpp b/src/linux/cgroups.hpp
index 282b8e7..f4cc2d8 100644
--- a/src/linux/cgroups.hpp
+++ b/src/linux/cgroups.hpp
@@ -75,6 +75,16 @@ Try<std::string> prepare(
const std::string& cgroup);
+// Returns error if any of the following is true:
+// (a) hierarchy is not mounted,
+// (b) cgroup does not exist
+// (c) control file does not exist.
+Try<Nothing> verify(
+ const std::string& hierarchy,
+ const std::string& cgroup = "",
+ const std::string& control = "");
+
+
// Check whether cgroups module is enabled on the current machine.
// @return True if cgroups module is enabled.
// False if cgroups module is not available.
@@ -150,10 +160,11 @@ Try<Nothing> mount(
int retry = 0);
-// Unmount a hierarchy and remove the directory associated with
-// it. This function will return error if the given hierarchy is not
-// valid. Also, it will return error if the given hierarchy has
-// any cgroups.
+// Unmount the cgroups virtual file system from the given hierarchy
+// root. The caller must make sure to remove all cgroups in the
+// hierarchy before unmount. This function assumes the given hierarchy
+// is currently mounted with a cgroups virtual file system. It will
+// return error if the given hierarchy has any cgroups.
// @param hierarchy Path to the hierarchy root.
// @return Some if the operation succeeds.
// Error if the operation fails.
@@ -173,8 +184,12 @@ Try<bool> mounted(
const std::string& subsystems = "");
-// Create a cgroup under a given hierarchy. This function will return error if
-// the given hierarchy is not valid.
+// Create a cgroup in a given hierarchy. To create a cgroup, one just
+// needs to create a directory in the cgroups virtual file system. The
+// given cgroup is a relative path to the given hierarchy. This
+// function assumes the given hierarchy is valid and is currently
+// mounted with a cgroup virtual file system. The function also
+// assumes the given cgroup is valid.
// @param hierarchy Path to the hierarchy root.
// @param cgroup Path to the cgroup relative to the hierarchy root.
// @param recursive Will create nested cgroups
@@ -186,11 +201,15 @@ Try<Nothing> create(
bool recursive = false);
-// Remove a cgroup under a given hierarchy. This function will return error if
-// the given hierarchy or the given cgroup is not valid. The cgroup will NOT be
-// removed recursively. In other words, if the cgroup has sub-cgroups inside,
-// the function will return error. Also, if any process is attached to the
-// given cgroup, the removal operation will also fail.
+// Remove a cgroup in a given hierarchy. To remove a cgroup, one needs
+// to remove the corresponding directory in the cgroups virtual file
+// system. A cgroup cannot be removed if it has processes or
+// sub-cgroups inside. This function does nothing but tries to remove
+// the corresponding directory of the given cgroup. It will return
+// error if the remove operation fails because it has either processes
+// or sub-cgroups inside. The cgroup will NOT be removed recursively.
+// This function assumes that the given hierarchy and cgroup are
+// valid.
// @param hierarchy Path to the hierarchy root.
// @param cgroup Path to the cgroup relative to the hierarchy root.
Try<Nothing> remove(const std::string& hierarchy, const std::string& cgroup);
@@ -201,14 +220,14 @@ Try<Nothing> remove(const std::string& hierarchy, const std::string& cgroup);
// @param cgroup Path to the cgroup relative to the hierarchy root.
// @return True if the cgroup exists.
// False if the cgroup does not exist.
-// Error if the operation fails (i.e., hierarchy is not mounted).
-Try<bool> exists(const std::string& hierarchy, const std::string& cgroup);
+bool exists(const std::string& hierarchy, const std::string& cgroup);
-// Return all the cgroups under the given cgroup of a given hierarchy. By
-// default, it returns all the cgroups under the given hierarchy. This function
-// will return error if the given hierarchy is not mounted or the cgroup does
-// not exist. We use a post-order walk here to ease the removal of cgroups.
+// Return all the cgroups under the given cgroup of a given hierarchy.
+// By default, it returns all the cgroups under the given hierarchy.
+// This function assumes that the given hierarchy and cgroup are
+// valid. We use a post-order walk here to ease the removal of
+// cgroups.
// @param hierarchy Path to the hierarchy root.
// @return A vector of cgroup names.
Try<std::vector<std::string>> get(
@@ -216,7 +235,8 @@ Try<std::vector<std::string>> get(
const std::string& cgroup = "/");
-// Send the specified signal to all process in a cgroup.
+// Send the specified signal to all process in a cgroup. This function
+// assumes that the given hierarchy and cgroup are valid.
// @param hierarchy Path to the hierarchy root.
// @param cgroup Path to the cgroup relative to the hierarchy root.
// @param signal The signal to send to all tasks within the cgroup.
@@ -228,11 +248,11 @@ Try<Nothing> kill(
int signal);
-// Read a control file. Control files are used to monitor and control cgroups.
-// This function will verify all the parameters. If the given hierarchy is not
-// properly mounted with appropriate subsystems, or the given cgroup is not
-// valid, or the given control file is not valid, the function will return
-// error.
+// Read a control file. Control files are the gateway to monitor and
+// control cgroups. This function assumes the cgroups virtual file
+// systems are properly mounted on the given hierarchy, and the given
+// cgroup has been already created properly. The given control file
+// name should also be valid.
// @param hierarchy Path to the hierarchy root.
// @param cgroup Path to the cgroup relative to the hierarchy root.
// @param control Name of the control file.
@@ -243,7 +263,10 @@ Try<std::string> read(
const std::string& control);
-// Write a control file. Parameter checking is similar to read.
+// Write a control file. This function assumes the cgroups virtual
+// file systems are properly mounted on the given hierarchy, and the
+// given cgroup has been already created properly. The given control
+// file name should also be valid.
// @param hierarchy Path to the hierarchy root.
// @param cgroup Path to the cgroup relative to the hierarchy root.
// @param control Name of the control file.
@@ -257,23 +280,24 @@ Try<Nothing> write(
const std::string& value);
-// Check whether a control file is valid under a given cgroup and a given
-// hierarchy. This function will return error if the given hierarchy is not
-// properly mounted with appropriate subsystems, or the given cgroup does not
-// exist, or the control file does not exist.
+// Check whether a control file is valid under a given cgroup and a
+// given hierarchy. This function will return error if the given
+// hierarchy is not properly mounted with appropriate subsystems, or
+// the given cgroup does not exist, or the control file does not
+// exist.
// @param hierarchy Path to the hierarchy root.
// @param cgroup Path to the cgroup relative to the hierarchy root.
// @param control Name of the control file.
-// @return Some if the check succeeds.
-// Error if the check fails.
-Try<bool> exists(
+// @return True if the check succeeds.
+// False if the check fails.
+bool exists(
const std::string& hierarchy,
const std::string& cgroup,
const std::string& control);
-// Return the set of process IDs in a given cgroup under a given hierarchy. It
-// will return error if the given hierarchy or the given cgroup is not valid.
+// Return the set of process IDs in a given cgroup under a given
+// hierarchy. It assumes the given hierarchy and cgroup are valid.
// @param hierarchy Path to the hierarchy root.
// @param cgroup Path to the cgroup relative to the hierarchy root.
// @return The set of process ids.
@@ -282,8 +306,8 @@ Try<std::set<pid_t>> processes(
const std::string& cgroup);
-// Return the set of thread IDs in a given cgroup under a given hierarchy. It
-// will return error if the given hierarchy or the given cgroup is not valid.
+// Return the set of thread IDs in a given cgroup under a given
+// hierarchy. It assumes the given hierarchy and cgroup are valid.
// @param hierarchy Path to the hierarchy root.
// @param cgroup Path to the cgroup relative to the hierarchy root.
// @return The set of thread ids.
@@ -292,10 +316,10 @@ Try<std::set<pid_t>> threads(
const std::string& cgroup);
-// Assign a given process specified by its pid to a given cgroup. All threads
-// in the pid's threadgroup will also be moved to the cgroup. This function
-// will return error if the given hierarchy or the given cgroup is not valid.
-// Also, it will return error if the pid has no process associated with it.
+// Assign a given process specified by its pid to a given cgroup. All
+// threads in the pid's threadgroup will also be moved to the cgroup.
+// This function assumes the given hierarchy and cgroup are valid. It
+// will return error if the pid has no process associated with it.
// @param hierarchy Path to the hierarchy root.
// @param cgroup Path to the cgroup relative to the hierarchy root.
// @param pid The pid of the given process.
@@ -307,10 +331,10 @@ Try<Nothing> assign(
pid_t pid);
-// Isolate a given process specified by its 'pid' to a given cgroup
-// by both creating the cgroup (recursively) if it doesn't exist and
-// then assigning the process to that cgroup.
-//
+// Isolate a given process specified by its 'pid' to a given cgroup by
+// both creating the cgroup (recursively) if it doesn't exist and then
+// assigning the process to that cgroup. It assumes the hierarchy is
+// valid.
// @param hierarchy Path to the hierarchy root.
// @param cgroup Path to the cgroup relative to the hierarchy root.
// @param pid The pid of the given process.
@@ -324,10 +348,9 @@ Try<Nothing> isolate(
namespace event {
-// Listen on an event notifier and return a future which will become ready when
-// the certain event happens. This function will return a future failure if some
-// expected happens (e.g. the given hierarchy does not have the proper
-// subsystems attached).
+// Listen on an event notifier and return a future which will become
+// ready when the certain event happens. This function assumes the
+// given hierarchy, cgroup and control file are valid.
// @param hierarchy Path to the hierarchy root.
// @param cgroup Path to the cgroup relative to the hierarchy root.
// @param control Name of the control file.
@@ -347,9 +370,8 @@ process::Future<uint64_t> listen(
// destroy any sub-cgroups. If the freezer subsystem is attached to
// the hierarchy, we attempt to kill all tasks in a given cgroup,
// before removing it. Otherwise, we just attempt to remove the
-// cgroup. This function will return an error if the given hierarchy
-// or the given cgroup does not exist or if we failed to destroy any
-// of the cgroups.
+// cgroup. This function assumes the given hierarchy and cgroup are
+// valid. It returns error if we failed to destroy any of the cgroups.
// NOTE: If cgroup is "/" (default), all cgroups under the
// hierarchy are destroyed.
// TODO(vinod): Add support for killing tasks when freezer subsystem
@@ -381,7 +403,8 @@ process::Future<Nothing> destroy(
process::Future<bool> cleanup(const std::string& hierarchy);
-// Returns the stat information from the given file.
+// Returns the stat information from the given file. This function
+// assumes the given hierarchy and cgroup are valid.
// @param hierarchy Path to the hierarchy root.
// @param cgroup Path to the cgroup relative to the hierarchy root.
// @param file The stat file to read from. (Ex: "memory.stat").
@@ -482,7 +505,8 @@ Try<std::vector<Value>> sectors_recursive(
// Returns the total number of bios/requests merged into requests
-// belonging to the given cgroup from blkio.io_merged.
+// belonging to the given cgroup from blkio.io_merged. This function
+// assumes the given hierarchy and cgroup are valid.
Try<std::vector<Value>> io_merged(
const std::string& hierarchy,
const std::string& cgroup);
@@ -490,14 +514,16 @@ Try<std::vector<Value>> io_merged(
// Returns the total number of bios/requests merged into requests
// belonging to the given cgroup and all its descendants from
-// blkio.io_merged_recursive.
+// blkio.io_merged_recursive. This function assumes the given
+// hierarchy and cgroup are valid.
Try<std::vector<Value>> io_merged_recursive(
const std::string& hierarchy,
const std::string& cgroup);
// Returns the total number of requests queued up in the given
-// cgroup from blkio.io_queued.
+// cgroup from blkio.io_queued. This function assumes the given
+// hierarchy and cgroup are valid.
Try<std::vector<Value>> io_queued(
const std::string& hierarchy,
const std::string& cgroup);
@@ -505,21 +531,24 @@ Try<std::vector<Value>> io_queued(
// Returns the total number of requests queued up in the given
// cgroup and all its descendants from blkio.io_queued_recursive.
+// This function assumes the given hierarchy and cgroup are valid.
Try<std::vector<Value>> io_queued_recursive(
const std::string& hierarchy,
const std::string& cgroup);
-// Returns the number of bytes transferred to/from the disk by
-// the given cgroup from blkio.io_service_bytes.
+// Returns the number of bytes transferred to/from the disk by the
+// given cgroup from blkio.io_service_bytes. This function assumes the
+// given hierarchy and cgroup are valid.
Try<std::vector<Value>> io_service_bytes(
const std::string& hierarchy,
const std::string& cgroup);
-// Returns the number of bytes transferred to/from the disk by
-// the given cgroup and all its descendants from
-// blkio.io_service_bytes_recursive.
+// Returns the number of bytes transferred to/from the disk by the
+// given cgroup and all its descendants from
+// blkio.io_service_bytes_recursive. This function assumes the given
+// hierarchy and cgroup are valid.
Try<std::vector<Value>> io_service_bytes_recursive(
const std::string& hierarchy,
const std::string& cgroup);
@@ -527,7 +556,8 @@ Try<std::vector<Value>> io_service_bytes_recursive(
// Returns the total amount of time between request dispatch and
// completion by the IOs done by the given cgroup from
-// blkio.io_service_time.
+// blkio.io_service_time. This function assumes the given hierarchy
+// and cgroup are valid.
Try<std::vector<Value>> io_service_time(
const std::string& hierarchy,
const std::string& cgroup);
@@ -535,14 +565,16 @@ Try<std::vector<Value>> io_service_time(
// Returns the total amount of time between request dispatch and
// completion by the IOs done by the given cgroup and all its
-// descendants from blkio.io_service_time_recursive.
+// descendants from blkio.io_service_time_recursive. This function
+// assumes the given hierarchy and cgroup are valid.
Try<std::vector<Value>> io_service_time_recursive(
const std::string& hierarchy,
const std::string& cgroup);
// Returns the number of IOs (bio) issued to the disk by the given
-// cgroup from blkio.io_serviced.
+// cgroup from blkio.io_serviced. This function assumes the given
+// hierarchy and cgroup are valid.
Try<std::vector<Value>> io_serviced(
const std::string& hierarchy,
const std::string& cgroup);
@@ -550,22 +582,24 @@ Try<std::vector<Value>> io_serviced(
// Returns the number of IOs (bio) issued to the disk by the given
// cgroup and all its descendants from blkio.io_serviced_recursive.
+// This function assumes the given hierarchy and cgroup are valid.
Try<std::vector<Value>> io_serviced_recursive(
const std::string& hierarchy,
const std::string& cgroup);
-// Returns the total amount of time the IOs for the given cgroup
-// spent waiting in the schedule queues for service from
-// blkio.io_wait_time.
+// Returns the total amount of time the IOs for the given cgroup spent
+// waiting in the schedule queues for service from blkio.io_wait_time.
+// This function assumes the given hierarchy and cgroup are valid.
Try<std::vector<Value>> io_wait_time(
const std::string& hierarchy,
const std::string& cgroup);
-// Returns the total amount of time the IOs for the given cgroup
-// and all its descendants spent waiting in the scheduler queues
-// for service from blkio.io_wait_time_recursive.
+// Returns the total amount of time the IOs for the given cgroup and
+// all its descendants spent waiting in the scheduler queues for
+// service from blkio.io_wait_time_recursive. This function assumes
+// the given hierarchy and cgroup are valid.
Try<std::vector<Value>> io_wait_time_recursive(
const std::string& hierarchy,
const std::string& cgroup);
@@ -575,15 +609,17 @@ Try<std::vector<Value>> io_wait_time_recursive(
namespace throttle {
-// Returns the numbers of bytes transferred to/from the disk for
-// the given cgroup from blkio.throttle.io_service_bytes.
+// Returns the numbers of bytes transferred to/from the disk for the
+// given cgroup from blkio.throttle.io_service_bytes. This function
+// assumes the given hierarchy and cgroup are valid.
Try<std::vector<Value>> io_service_bytes(
const std::string& hierarchy,
const std::string& cgroup);
-// Returns the numbers of IOs (bio) issued to the disk for the
-// given cgroup from blkio.throttle.io_serviced.
+// Returns the numbers of IOs (bio) issued to the disk for the given
+// cgroup from blkio.throttle.io_serviced. This function assumes the
+// given hierarchy and cgroup are valid.
Try<std::vector<Value>> io_serviced(
const std::string& hierarchy,
const std::string& cgroup);
@@ -636,38 +672,43 @@ inline std::ostream& operator<<(std::ostream& stream, const Value& value)
namespace cpu {
// Returns the cgroup that the specified pid is a member of within the
-// hierarchy that the 'cpu' subsytem is mounted or None if the
+// hierarchy that the 'cpu' subsystem is mounted or None if the
// subsystem is not mounted or the pid is not a member of a cgroup.
Result<std::string> cgroup(pid_t pid);
-// Sets the cpu shares using cpu.shares.
+// Sets the cpu shares using cpu.shares. This function assumes the
+// given hierarchy and cgroup are valid.
Try<Nothing> shares(
const std::string& hierarchy,
const std::string& cgroup,
uint64_t shares);
-// Returns the cpu shares from cpu.shares.
+// Returns the cpu shares from cpu.shares. This function assumes the
+// given hierarchy and cgroup are valid.
Try<uint64_t> shares(
const std::string& hierarchy,
const std::string& cgroup);
-// Sets the cfs period using cpu.cfs_period_us.
+// Sets the cfs period using cpu.cfs_period_us. This function assumes
+// the given hierarchy and cgroup are valid.
Try<Nothing> cfs_period_us(
const std::string& hierarchy,
const std::string& cgroup,
const Duration& duration);
-// Returns the cfs quota from cpu.cfs_quota_us.
+// Returns the cfs quota from cpu.cfs_quota_us. This function assumes
+// the given hierarchy and cgroup are valid.
Try<Duration> cfs_quota_us(
const std::string& hierarchy,
const std::string& cgroup);
-// Sets the cfs quota using cpu.cfs_quota_us.
+// Sets the cfs quota using cpu.cfs_quota_us. This function assumes
+// the given hierarchy and cgroup are valid.
Try<Nothing> cfs_quota_us(
const std::string& hierarchy,
const std::string& cgroup,
@@ -698,7 +739,8 @@ struct Stats
};
-// Returns 'Stats' for a given hierarchy and cgroup.
+// Returns 'Stats' for a given hierarchy and cgroup. This function
+// assumes the given hierarchy and cgroup are valid.
//
// @param hierarchy hierarchy for the 'cpuacct' subsystem.
// @param cgroup cgroup for a given process.
@@ -720,13 +762,15 @@ namespace memory {
Result<std::string> cgroup(pid_t pid);
-// Returns the memory limit from memory.limit_in_bytes.
+// Returns the memory limit from memory.limit_in_bytes. This function
+// assumes the given hierarchy and cgroup are valid.
Try<Bytes> limit_in_bytes(
const std::string& hierarchy,
const std::string& cgroup);
-// Sets the memory limit using memory.limit_in_bytes.
+// Sets the memory limit using memory.limit_in_bytes. This function
+// assumes the given hierarchy and cgroup are valid.
Try<Nothing> limit_in_bytes(
const std::string& hierarchy,
const std::string& cgroup,
@@ -735,7 +779,8 @@ Try<Nothing> limit_in_bytes(
// Returns the memory limit from memory.memsw.limit_in_bytes. Returns
// none if memory.memsw.limit_in_bytes is not supported (e.g., when
-// swap is turned off).
+// swap is turned off). This function assumes the given hierarchy and
+// cgroup are valid.
Result<Bytes> memsw_limit_in_bytes(
const std::string& hierarchy,
const std::string& cgroup);
@@ -743,39 +788,45 @@ Result<Bytes> memsw_limit_in_bytes(
// Sets the memory limit using memory.memsw.limit_in_bytes. Returns
// false if memory.memsw.limit_in_bytes is not supported (e.g., when
-// swap is turned off).
+// swap is turned off). This function assumes the given hierarchy and
+// cgroup are valid.
Try<bool> memsw_limit_in_bytes(
const std::string& hierarchy,
const std::string& cgroup,
const Bytes& limit);
-// Returns the soft memory limit from memory.soft_limit_in_bytes.
+// Returns the soft memory limit from memory.soft_limit_in_bytes. This
+// function assumes the given hierarchy and cgroup are valid.
Try<Bytes> soft_limit_in_bytes(
const std::string& hierarchy,
const std::string& cgroup);
-// Sets the soft memory limit using memory.soft_limit_in_bytes.
+// Sets the soft memory limit using memory.soft_limit_in_bytes. This
+// function assumes the given hierarchy and cgroup are valid.
Try<Nothing> soft_limit_in_bytes(
const std::string& hierarchy,
const std::string& cgroup,
const Bytes& limit);
-// Returns the memory usage from memory.usage_in_bytes.
+// Returns the memory usage from memory.usage_in_bytes. This function
+// assumes the given hierarchy and cgroup are valid.
Try<Bytes> usage_in_bytes(
const std::string& hierarchy,
const std::string& cgroup);
// Returns the memory + swap usage from memory.memsw.usage_in_bytes.
+// This function assumes the given hierarchy and cgroup are valid.
Try<Bytes> memsw_usage_in_bytes(
const std::string& hierarchy,
const std::string& cgroup);
-// Returns the max memory usage from memory.max_usage_in_bytes.
+// Returns the max memory usage from memory.max_usage_in_bytes. This
+// function assumes the given hierarchy and cgroup are valid.
Try<Bytes> max_usage_in_bytes(
const std::string& hierarchy,
const std::string& cgroup);
@@ -784,7 +835,8 @@ Try<Bytes> max_usage_in_bytes(
// Out-of-memory (OOM) controls.
namespace oom {
-// Listen for an OOM event for the cgroup.
+// Listen for an OOM event for the cgroup. This function assumes the
+// given hierarchy and cgroup are valid.
process::Future<Nothing> listen(
const std::string& hierarchy,
const std::string& cgroup);
@@ -793,18 +845,21 @@ process::Future<Nothing> listen(
namespace killer {
// Return whether the kernel OOM killer is enabled for the cgroup.
+// This function assumes the given hierarchy and cgroup are valid.
Try<bool> enabled(
const std::string& hierarchy,
const std::string& cgroup);
// Enable the kernel OOM killer for the cgroup. The control file will
-// only be written to if necessary.
+// only be written to if necessary. This function assumes the given
+// hierarchy and cgroup are valid.
Try<Nothing> enable(
const std::string& hierarchy,
const std::string& cgroup);
// Disable the kernel OOM killer. The control file will only be
-// written to if necessary.
+// written to if necessary. This function assumes the given hierarchy
+// and cgroup are valid.
Try<Nothing> disable(
const std::string& hierarchy,
const std::string& cgroup);
@@ -840,7 +895,8 @@ class Counter
{
public:
// Create a memory pressure counter for the given cgroup on the
- // specified level.
+ // specified level. This function assumes the given hierarchy and
+ // cgroup are valid.
static Try<process::Owned<Counter>> create(
const std::string& hierarchy,
const std::string& cgroup,
@@ -930,18 +986,21 @@ bool operator==(
const Entry& right);
-// Returns the entries within devices.list.
+// Returns the entries within devices.list. This function assumes the
+// given hierarchy and cgroup are valid.
Try<std::vector<Entry>> list(
const std::string& hierarchy,
const std::string& cgroup);
-// Writes the provided `entry` into devices.allow.
+// Writes the provided `entry` into devices.allow. This function
+// assumes the given hierarchy and cgroup are valid.
Try<Nothing> allow(
const std::string& hierarchy,
const std::string& cgroup,
const Entry& entry);
-// Writes the provided `entry` into devices.deny.
+// Writes the provided `entry` into devices.deny. This function
+// assumes the given hierarchy and cgroup are valid.
Try<Nothing> deny(
const std::string& hierarchy,
const std::string& cgroup,
@@ -957,18 +1016,20 @@ Try<Nothing> deny(
// 3. FROZEN : All processes are frozen.
namespace freezer {
-// Freeze all processes in the given cgroup. The cgroup must be in a freezer
-// hierarchy. This function will return a future which will become ready when
-// all processes have been frozen (cgroup is in the FROZEN state).
+// Freeze all processes in the given cgroup. This function will return
+// a future which will become ready when all processes have been
+// frozen (cgroup is in the FROZEN state). This function assumes the
+// given hierarchy and cgroup are valid.
process::Future<Nothing> freeze(
const std::string& hierarchy,
const std::string& cgroup);
-// Thaw all processes in the given cgroup. The cgroup must be in a freezer
-// hierarchy. This is a revert operation of freezer::freeze. This function will
-// return a future which will become ready when all processes have been thawed
-// (cgroup is in the THAWED state).
+// Thaw all processes in the given cgroup. This is a revert operation
+// of freezer::freeze. This function will return a future which will
+// become ready when all processes have been thawed (cgroup is in the
+// THAWED state). This function assumes the given hierarchy and cgroup
+// are valid.
process::Future<Nothing> thaw(
const std::string& hierarchy,
const std::string& cgroup);
@@ -979,13 +1040,15 @@ process::Future<Nothing> thaw(
// Net_cls subsystem.
namespace net_cls {
-// Read the uint32_t handle set in `net_cls.classid`.
+// Read the uint32_t handle set in `net_cls.classid`. This function
+// assumes the given hierarchy and cgroup are valid.
Try<uint32_t> classid(
const std::string& hierarchy,
const std::string& cgroup);
-// Write the uint32_t handle to the `net_cls.classid`.
+// Write the uint32_t handle to the `net_cls.classid`. This function
+// assumes the given hierarchy and cgroup are valid.
Try<Nothing> classid(
const std::string& hierarchy,
const std::string& cgroup,
diff --git a/src/linux/systemd.cpp b/src/linux/systemd.cpp
index 2444ff4..8126b31 100644
--- a/src/linux/systemd.cpp
+++ b/src/linux/systemd.cpp
@@ -178,13 +178,13 @@ Try<Nothing> initialize(const Flags& flags)
// Now the `MESOS_EXECUTORS_SLICE` is ready for us to assign any pids. We can
// verify that our cgroups assignments will work by testing the hierarchy.
- Try<bool> exists = cgroups::exists(
+ Try<Nothing> cgroupsVerify = cgroups::verify(
systemd::hierarchy(),
mesos::MESOS_EXECUTORS_SLICE);
- if (exists.isError() || !exists.get()) {
+ if (cgroupsVerify.isError()) {
return Error("Failed to locate systemd cgroups hierarchy: " +
- (exists.isError() ? exists.error() : "does not exist"));
+ cgroupsVerify.error());
}
initialized->done();
diff --git a/src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp b/src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp
index 1444c05..11dfbab 100644
--- a/src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp
+++ b/src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp
@@ -339,16 +339,7 @@ Future<Nothing> CgroupsIsolatorProcess::___recover(
// TODO(haosdent): Use foreachkey once MESOS-5037 is resolved.
foreach (const string& hierarchy, subsystems.keys()) {
- Try<bool> exists = cgroups::exists(hierarchy, cgroup);
- if (exists.isError()) {
- return Failure(
- "Failed to check the existence of the cgroup "
- "'" + cgroup + "' in hierarchy '" + hierarchy + "' "
- "for container " + stringify(containerId) +
- ": " + exists.error());
- }
-
- if (!exists.get()) {
+ if (!cgroups::exists(hierarchy, cgroup)) {
// This may occur if the executor has exited and the isolator
// has destroyed the cgroup but the agent dies before noticing
// this. This will be detected when the containerizer tries to
@@ -438,15 +429,7 @@ Future<Option<ContainerLaunchInfo>> CgroupsIsolatorProcess::prepare(
VLOG(1) << "Creating cgroup at '" << path << "' "
<< "for container " << containerId;
- Try<bool> exists = cgroups::exists(
- hierarchy,
- infos[containerId]->cgroup);
-
- if (exists.isError()) {
- return Failure(
- "Failed to check the existence of cgroup at "
- "'" + path + "': " + exists.error());
- } else if (exists.get()) {
+ if (cgroups::exists(hierarchy, infos[containerId]->cgroup)) {
return Failure("The cgroup at '" + path + "' already exists");
}
diff --git a/src/slave/containerizer/mesos/isolators/cgroups/subsystems/cpu.cpp b/src/slave/containerizer/mesos/isolators/cgroups/subsystems/cpu.cpp
index 321671f..960bd14 100644
--- a/src/slave/containerizer/mesos/isolators/cgroups/subsystems/cpu.cpp
+++ b/src/slave/containerizer/mesos/isolators/cgroups/subsystems/cpu.cpp
@@ -38,16 +38,7 @@ Try<Owned<SubsystemProcess>> CpuSubsystemProcess::create(
const string& hierarchy)
{
if (flags.cgroups_enable_cfs) {
- Try<bool> exists = cgroups::exists(
- hierarchy,
- flags.cgroups_root,
- "cpu.cfs_quota_us");
-
- if (exists.isError()) {
- return Error(
- "Failed to check the existence of 'cpu.cfs_quota_us': " +
- exists.error());
- } else if (!exists.get()) {
+ if (!cgroups::exists(hierarchy, flags.cgroups_root, "cpu.cfs_quota_us")) {
return Error(
"Failed to find 'cpu.cfs_quota_us'. Your kernel might be "
"too old to use the CFS quota feature");
diff --git a/src/slave/containerizer/mesos/isolators/gpu/isolator.cpp b/src/slave/containerizer/mesos/isolators/gpu/isolator.cpp
index d84f78c..dbbf92f 100644
--- a/src/slave/containerizer/mesos/isolators/gpu/isolator.cpp
+++ b/src/slave/containerizer/mesos/isolators/gpu/isolator.cpp
@@ -261,22 +261,7 @@ Future<Nothing> NvidiaGpuIsolatorProcess::recover(
const string cgroup = path::join(flags.cgroups_root, containerId.value());
- Try<bool> exists = cgroups::exists(hierarchy, cgroup);
- if (exists.isError()) {
- foreachvalue (Info* info, infos) {
- delete info;
- }
-
- infos.clear();
-
- return Failure(
- "Failed to check the existence of the cgroup "
- "'" + cgroup + "' in hierarchy '" + hierarchy + "' "
- "for container " + stringify(containerId) +
- ": " + exists.error());
- }
-
- if (!exists.get()) {
+ if (!cgroups::exists(hierarchy, cgroup)) {
// This may occur if the executor has exited and the isolator
// has destroyed the cgroup but the slave dies before noticing
// this. This will be detected when the containerizer tries to
diff --git a/src/slave/containerizer/mesos/linux_launcher.cpp b/src/slave/containerizer/mesos/linux_launcher.cpp
index e51352a..e9ab36a 100644
--- a/src/slave/containerizer/mesos/linux_launcher.cpp
+++ b/src/slave/containerizer/mesos/linux_launcher.cpp
@@ -161,18 +161,7 @@ Try<Launcher*> LinuxLauncher::create(const Flags& flags)
systemdHierarchy = systemd::hierarchy();
// Create the root cgroup if does not exist.
- Try<bool> exists = cgroups::exists(
- systemdHierarchy.get(),
- flags.cgroups_root);
-
- if (exists.isError()) {
- return Error(
- "Failed to check the existence of cgroup root '" +
- flags.cgroups_root + "' under systemd hierarchy '" +
- systemdHierarchy.get() + "': " + exists.error());
- }
-
- if (!exists.get()) {
+ if (!cgroups::exists(systemdHierarchy.get(), flags.cgroups_root)) {
Try<Nothing> create = cgroups::create(
systemdHierarchy.get(),
flags.cgroups_root);
@@ -623,12 +612,7 @@ Future<Nothing> LinuxLauncherProcess::destroy(const ContainerID& containerId)
// is considered partially destroyed if we have recovered it from
// ContainerState but we don't have a freezer cgroup for it. If this
// is a partially destroyed container than there is nothing to do.
- Try<bool> exists = cgroups::exists(freezerHierarchy, cgroup);
- if (exists.isError()) {
- return Failure("Failed to determine if cgroup exists: " + exists.error());
- }
-
- if (!exists.get()) {
+ if (!cgroups::exists(freezerHierarchy, cgroup)) {
LOG(WARNING) << "Couldn't find freezer cgroup for container "
<< container->id << " so assuming partially destroyed";
@@ -663,12 +647,7 @@ Future<Nothing> LinuxLauncherProcess::_destroy(const ContainerID& containerId)
const string cgroup =
LinuxLauncher::cgroup(flags.cgroups_root, containerId);
- Try<bool> exists = cgroups::exists(systemdHierarchy.get(), cgroup);
- if (exists.isError()) {
- return Failure("Failed to determine if cgroup exists: " + exists.error());
- }
-
- if (!exists.get()) {
+ if (!cgroups::exists(systemdHierarchy.get(), cgroup)) {
return Nothing();
}
diff --git a/src/slave/main.cpp b/src/slave/main.cpp
index 489e875..55615e7 100644
--- a/src/slave/main.cpp
+++ b/src/slave/main.cpp
@@ -151,16 +151,7 @@ static Try<Nothing> assignCgroups(const slave::Flags& flags)
// Create a cgroup for the slave.
string cgroup = path::join(flags.cgroups_root, "slave");
- Try<bool> exists = cgroups::exists(hierarchy.get(), cgroup);
- if (exists.isError()) {
- return Error(
- "Failed to find cgroup " + cgroup +
- " for subsystem " + subsystem +
- " under hierarchy " + hierarchy.get() +
- " for agent: " + exists.error());
- }
-
- if (!exists.get()) {
+ if (!cgroups::exists(hierarchy.get(), cgroup)) {
Try<Nothing> create = cgroups::create(hierarchy.get(), cgroup);
if (create.isError()) {
return Error(
diff --git a/src/tests/containerizer/nested_mesos_containerizer_tests.cpp b/src/tests/containerizer/nested_mesos_containerizer_tests.cpp
index 295dbc8..ac9a6b8 100644
--- a/src/tests/containerizer/nested_mesos_containerizer_tests.cpp
+++ b/src/tests/containerizer/nested_mesos_containerizer_tests.cpp
@@ -2025,7 +2025,7 @@ TEST_F(NestedMesosContainerizerTest, ROOT_CGROUPS_RecoverLauncherOrphans)
buildPath(containerId, "mesos", JOIN));
ASSERT_SOME(cgroups::create(freezerHierarchy.get(), cgroup, true));
- ASSERT_SOME_TRUE(cgroups::exists(freezerHierarchy.get(), cgroup));
+ ASSERT_TRUE(cgroups::exists(freezerHierarchy.get(), cgroup));
SlaveState state;
state.id = SlaveID();
@@ -2038,7 +2038,7 @@ TEST_F(NestedMesosContainerizerTest, ROOT_CGROUPS_RecoverLauncherOrphans)
// NOTE: `wait()` can return `Some` or `None` due to a race condition between
// `recover()` and `______destroy()` for an orphan container.
AWAIT_READY(containerizer->wait(containerId));
- ASSERT_SOME_FALSE(cgroups::exists(freezerHierarchy.get(), cgroup));
+ ASSERT_FALSE(cgroups::exists(freezerHierarchy.get(), cgroup));
Future<hashset<ContainerID>> containers = containerizer->containers();
AWAIT_READY(containers);
@@ -2203,7 +2203,7 @@ TEST_F(NestedMesosContainerizerTest,
buildPath(containerId, "mesos", JOIN));
ASSERT_SOME(cgroups::create(freezerHierarchy.get(), cgroupParent, true));
- ASSERT_SOME_TRUE(cgroups::exists(freezerHierarchy.get(), cgroupParent));
+ ASSERT_TRUE(cgroups::exists(freezerHierarchy.get(), cgroupParent));
ContainerID nestedContainerId;
nestedContainerId.mutable_parent()->CopyFrom(containerId);
@@ -2214,7 +2214,7 @@ TEST_F(NestedMesosContainerizerTest,
buildPath(nestedContainerId, "mesos", JOIN));
ASSERT_SOME(cgroups::create(freezerHierarchy.get(), cgroupNested, true));
- ASSERT_SOME_TRUE(cgroups::exists(freezerHierarchy.get(), cgroupNested));
+ ASSERT_TRUE(cgroups::exists(freezerHierarchy.get(), cgroupNested));
SlaveState state;
state.id = SlaveID();
@@ -2227,14 +2227,14 @@ TEST_F(NestedMesosContainerizerTest,
// NOTE: `wait()` can return `Some` or `None` due to a race condition between
// `recover()` and `______destroy()` for an orphan container.
AWAIT_READY(containerizer->wait(nestedContainerId));
- ASSERT_SOME_FALSE(cgroups::exists(freezerHierarchy.get(), cgroupNested));
+ ASSERT_FALSE(cgroups::exists(freezerHierarchy.get(), cgroupNested));
Future<hashset<ContainerID>> containers = containerizer->containers();
AWAIT_READY(containers);
ASSERT_FALSE(containers->contains(nestedContainerId));
AWAIT_READY(containerizer->wait(containerId));
- ASSERT_SOME_FALSE(cgroups::exists(freezerHierarchy.get(), cgroupParent));
+ ASSERT_FALSE(cgroups::exists(freezerHierarchy.get(), cgroupParent));
containers = containerizer->containers();
AWAIT_READY(containers);
@@ -2574,7 +2574,7 @@ TEST_F(NestedMesosContainerizerTest,
buildPath(containerId, "mesos", JOIN));
ASSERT_SOME(cgroups::create(freezerHierarchy.get(), cgroupParent, true));
- ASSERT_SOME_TRUE(cgroups::exists(freezerHierarchy.get(), cgroupParent));
+ ASSERT_TRUE(cgroups::exists(freezerHierarchy.get(), cgroupParent));
ContainerID nestedContainerId1;
nestedContainerId1.mutable_parent()->CopyFrom(containerId);
@@ -2585,7 +2585,7 @@ TEST_F(NestedMesosContainerizerTest,
buildPath(nestedContainerId1, "mesos", JOIN));
ASSERT_SOME(cgroups::create(freezerHierarchy.get(), cgroupNested1, true));
- ASSERT_SOME_TRUE(cgroups::exists(freezerHierarchy.get(), cgroupNested1));
+ ASSERT_TRUE(cgroups::exists(freezerHierarchy.get(), cgroupNested1));
ContainerID nestedContainerId2;
nestedContainerId2.mutable_parent()->CopyFrom(containerId);
@@ -2596,7 +2596,7 @@ TEST_F(NestedMesosContainerizerTest,
buildPath(nestedContainerId2, "mesos", JOIN));
ASSERT_SOME(cgroups::create(freezerHierarchy.get(), cgroupNested2, true));
- ASSERT_SOME_TRUE(cgroups::exists(freezerHierarchy.get(), cgroupNested2));
+ ASSERT_TRUE(cgroups::exists(freezerHierarchy.get(), cgroupNested2));
SlaveState state;
state.id = SlaveID();
@@ -2609,13 +2609,13 @@ TEST_F(NestedMesosContainerizerTest,
// NOTE: `wait()` can return `Some` or `None` due to a race condition between
// `recover()` and `______destroy()` for an orphan container.
AWAIT_READY(containerizer->wait(nestedContainerId1));
- ASSERT_SOME_FALSE(cgroups::exists(freezerHierarchy.get(), cgroupNested1));
+ ASSERT_FALSE(cgroups::exists(freezerHierarchy.get(), cgroupNested1));
AWAIT_READY(containerizer->wait(nestedContainerId2));
- ASSERT_SOME_FALSE(cgroups::exists(freezerHierarchy.get(), cgroupNested2));
+ ASSERT_FALSE(cgroups::exists(freezerHierarchy.get(), cgroupNested2));
AWAIT_READY(containerizer->wait(containerId));
- ASSERT_SOME_FALSE(cgroups::exists(freezerHierarchy.get(), cgroupParent));
+ ASSERT_FALSE(cgroups::exists(freezerHierarchy.get(), cgroupParent));
Future<hashset<ContainerID>> containers = containerizer->containers();
AWAIT_READY(containers);
[mesos] 02/02: Added MESOS-9081 to 1.7.0 CHANGELOG.
Posted by ji...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
jieyu pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mesos.git
commit 4bed583a04600cd267a95045362f571308578c67
Author: Jie Yu <yu...@gmail.com>
AuthorDate: Mon Aug 20 11:16:08 2018 -0700
Added MESOS-9081 to 1.7.0 CHANGELOG.
---
CHANGELOG | 1 +
1 file changed, 1 insertion(+)
diff --git a/CHANGELOG b/CHANGELOG
index a374bae..52ac6d4 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -162,6 +162,7 @@ All Resolved Issues:
* [MESOS-9015] - Allow resources to be removed when updating the sorter.
* [MESOS-9055] - Make gRPC call deadline configurable.
* [MESOS-9067] - Improve performance of json parsing by avoiding conversion cost.
+ * [MESOS-9081] - cgroups::verify is expensive and is done implicitly during cgroups operations.
* [MESOS-9086] - Optimize range subtraction operation.
* [MESOS-9092] - Adopt rapidjson for improved json serialization performance.
* [MESOS-9104] - Refactor capability related logic in the allocator.