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/10/02 22:02:34 UTC
[3/9] git commit: Added memory threshold notifications for inducing
OOMs.
Added memory threshold notifications for inducing OOMs.
Review: https://reviews.apache.org/r/14043
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/8d742395
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/8d742395
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/8d742395
Branch: refs/heads/master
Commit: 8d742395b77cb9fb95cf5fdc123545aeb69f5c48
Parents: 9629fe0
Author: Benjamin Mahler <bm...@twitter.com>
Authored: Wed Aug 28 13:43:46 2013 -0700
Committer: Benjamin Mahler <bm...@twitter.com>
Committed: Wed Oct 2 12:17:52 2013 -0700
----------------------------------------------------------------------
src/slave/cgroups_isolator.cpp | 108 ++++++++++++++++++++++++++++--------
src/slave/cgroups_isolator.hpp | 9 ++-
src/slave/flags.hpp | 12 ++++
3 files changed, 106 insertions(+), 23 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/8d742395/src/slave/cgroups_isolator.cpp
----------------------------------------------------------------------
diff --git a/src/slave/cgroups_isolator.cpp b/src/slave/cgroups_isolator.cpp
index 4be3448..fc36f1f 100644
--- a/src/slave/cgroups_isolator.cpp
+++ b/src/slave/cgroups_isolator.cpp
@@ -672,13 +672,12 @@ void CgroupsIsolator::killExecutor(
LOG(INFO) << "Killing executor " << executorId
<< " of framework " << frameworkId;
- // Stop the OOM listener if needed.
- if (info->oomNotifier.isPending()) {
- info->oomNotifier.discard();
- }
-
info->killed = true;
+ // Stop the OOM and threshold listeners.
+ info->oomNotifier.discard();
+ info->memoryThresholdNotifier.discard();
+
// Destroy the cgroup that is associated with the executor. Here, we
// don't wait for it to succeed as we don't want to block the
// isolator. Instead, we register a callback which will be invoked
@@ -1073,6 +1072,8 @@ Try<Nothing> CgroupsIsolator::memChanged(
return Error("Expecting resource 'mem' to be a scalar");
}
+ // The soft limit is used as a threshold for inducing our own "OOM"s
+ // so that we can capture memory information for debugging purposes.
Bytes mem = Bytes((uint64_t) resource.scalar().value() * 1024LL * 1024LL);
Bytes limit = std::max(mem, MIN_MEMORY);
@@ -1089,7 +1090,7 @@ Try<Nothing> CgroupsIsolator::memChanged(
<< " for executor " << info->executorId
<< " of framework " << info->frameworkId;
- // Read the existing limit.
+ // Determine the current hard limit.
Try<Bytes> currentLimit =
cgroups::memory::limit_in_bytes(hierarchy, info->name());
@@ -1098,6 +1099,10 @@ Try<Nothing> CgroupsIsolator::memChanged(
"Failed to read 'memory.limit_in_bytes': " + currentLimit.error());
}
+ // The hard limit has an additional memory buffer to allow us
+ // to induce our own OOM when the soft limit threshold is reached.
+ Bytes hardLimit = limit + flags.cgroups_oom_buffer;
+
// Determine whether to set the hard limit. If this is the first
// time (info->pid.isNone()), or we're raising the existing limit,
// then we can update the hard limit safely. Otherwise, if we need
@@ -1108,16 +1113,43 @@ Try<Nothing> CgroupsIsolator::memChanged(
// TODO(benh): Introduce a MemoryWatcherProcess which monitors the
// discrepancy between usage and soft limit and introduces a
// "manual oom" if necessary.
- if (info->pid.isNone() || limit > currentLimit.get()) {
- write = cgroups::memory::limit_in_bytes(hierarchy, info->name(), limit);
+ if (info->pid.isNone() || hardLimit > currentLimit.get()) {
+ // Set the memory hard limit.
+ write = cgroups::memory::limit_in_bytes(hierarchy, info->name(), hardLimit);
if (write.isError()) {
return Error("Failed to set 'memory.limit_in_bytes': " + write.error());
}
- LOG(INFO) << "Updated 'memory.limit_in_bytes' to " << limit
+ LOG(INFO) << "Updated 'memory.limit_in_bytes' to " << hardLimit
<< " for executor " << info->executorId
<< " of framework " << info->frameworkId;
+
+ // Update the memory threshold notification.
+ if (info->memoryThresholdNotifier.isPending()) {
+ // The threshold can be triggered while we go to discard it
+ // here, but that's ok. It's still an OOM!
+ info->memoryThresholdNotifier.discard();
+ }
+
+ // The new threshold notification should be created only if we
+ // were able to successfully discard it above.
+ if (info->memoryThresholdNotifier.isDiscarded()) {
+ info->memoryThresholdNotifier = cgroups::listen(
+ hierarchy,
+ info->name(),
+ "memory.usage_in_bytes",
+ stringify(limit.bytes()));
+
+ info->memoryThresholdNotifier.onAny(
+ defer(PID<CgroupsIsolator>(this),
+ &CgroupsIsolator::oomWaited,
+ info->frameworkId,
+ info->executorId,
+ info->uuid.get(),
+ "memory threshold (" + stringify(limit) + ")",
+ lambda::_1));
+ }
}
return Nothing();
@@ -1152,6 +1184,7 @@ void CgroupsIsolator::oomListen(
frameworkId,
executorId,
info->uuid.get(),
+ "OOM",
lambda::_1));
}
@@ -1160,22 +1193,46 @@ void CgroupsIsolator::oomWaited(
const FrameworkID& frameworkId,
const ExecutorID& executorId,
const UUID& uuid,
+ const string& event,
const Future<uint64_t>& future)
{
- LOG(INFO) << "OOM notifier is triggered for executor "
- << executorId << " of framework " << frameworkId
- << " with uuid " << uuid;
-
if (future.isDiscarded()) {
- LOG(INFO) << "Discarded OOM notifier for executor "
+ LOG(INFO) << "Discarded " << event << " notifier for executor "
<< executorId << " of framework " << frameworkId
<< " with uuid " << uuid;
} else if (future.isFailed()) {
- LOG(ERROR) << "Listening on OOM events failed for executor "
+ LOG(ERROR) << "Failed to listen to " << event << " events for executor "
<< executorId << " of framework " << frameworkId
<< " with uuid " << uuid << ": " << future.failure();
} else {
- // Out-of-memory event happened, call the handler.
+ CgroupInfo* info = findCgroupInfo(frameworkId, executorId);
+
+ if (info == NULL) {
+ LOG(WARNING) << event << " handler invoked for the already terminated "
+ << "executor " << executorId
+ << " of framework " << frameworkId
+ << " with uuid " << uuid;
+ return;
+ }
+
+ if (info->killed) {
+ LOG(WARNING) << "Ignoring " << event << " notification for the killed "
+ << "executor " << executorId
+ << " of framework " << frameworkId
+ << " with uuid " << uuid;
+ return;
+ }
+
+ LOG(INFO) << event << " event detected for executor " << executorId
+ << " of framework " << frameworkId << " with uuid " << uuid
+ << " ; invoking OOM handler";
+
+ // Discard both notifiers. If either an OOM occurs or the memory
+ // threshold is reached, we're treating it as an OOM and no longer
+ // need to be notified.
+ info->memoryThresholdNotifier.discard();
+ info->oomNotifier.discard();
+
oom(frameworkId, executorId, uuid);
}
}
@@ -1191,7 +1248,9 @@ void CgroupsIsolator::oom(
// It is likely that processExited is executed before this function (e.g.
// The kill and OOM events happen at the same time, and the process exit
// event arrives first.) Therefore, we should not report a fatal error here.
- LOG(INFO) << "OOM detected for an already terminated executor";
+ LOG(WARNING) << "OOM handler invoked for the already terminated executor "
+ << executorId << " of framework " << frameworkId
+ << " with uuid " << uuid;
return;
}
@@ -1199,15 +1258,18 @@ void CgroupsIsolator::oom(
// previous instance of an executor.
CHECK_SOME(info->uuid);
if (uuid != info->uuid.get()) {
- LOG(INFO) << "OOM detected for a previous executor instance";
+ LOG(WARNING) << "OOM handler invoked for the previous executor instance "
+ << executorId << " of framework " << frameworkId
+ << " with uuid " << uuid
+ << " vs current uuid " << info->uuid.get();
return;
}
// If killed is set, the OOM notifier will be discarded in oomWaited.
// Therefore, we should not be able to reach this point.
- CHECK(!info->killed) << "OOM detected for an already killed executor";
+ CHECK(!info->killed) << "OOM handler invoked for an already killed executor";
- LOG(INFO) << "OOM detected for executor " << executorId
+ LOG(INFO) << "OOM handler invoked for executor " << executorId
<< " of framework " << frameworkId
<< " with uuid " << uuid;
@@ -1217,10 +1279,12 @@ void CgroupsIsolator::oom(
message << "Memory limit exceeded: ";
// Output the requested memory limit.
- Try<Bytes> limit = cgroups::memory::limit_in_bytes(hierarchy, info->name());
+ Try<Bytes> limit = cgroups::memory::soft_limit_in_bytes(
+ hierarchy, info->name());
if (limit.isError()) {
- LOG(ERROR) << "Failed to read 'memory.limit_in_bytes': " << limit.error();
+ LOG(ERROR) << "Failed to read 'memory.soft_limit_in_bytes': "
+ << limit.error();
} else {
message << "Requested: " << limit.get() << " ";
}
http://git-wip-us.apache.org/repos/asf/mesos/blob/8d742395/src/slave/cgroups_isolator.hpp
----------------------------------------------------------------------
diff --git a/src/slave/cgroups_isolator.hpp b/src/slave/cgroups_isolator.hpp
index e86062e..ca3e1ae 100644
--- a/src/slave/cgroups_isolator.hpp
+++ b/src/slave/cgroups_isolator.hpp
@@ -172,6 +172,9 @@ private:
// Used to cancel the OOM listening.
process::Future<uint64_t> oomNotifier;
+ // Used to cancel the memory threshold listening.
+ process::Future<uint64_t> memoryThresholdNotifier;
+
// CPUs allocated if using 'cpuset' subsystem.
Cpuset* cpuset;
};
@@ -222,13 +225,17 @@ private:
// @param frameworkId The id of the given framework.
// @param executorId The id of the given executor.
// @param uuid The uuid of the given executor.
+ // @param event The OOM event type name:
+ // "OOM" or "memory threshold ({Bytes})".
void oomWaited(
const FrameworkID& frameworkId,
const ExecutorID& executorId,
const UUID& uuid,
+ const std::string& event,
const process::Future<uint64_t>& future);
- // This function is invoked when the OOM event happens.
+ // This function is invoked to process an executor that has
+ // OOMed or exceeded its memory limit.
// @param frameworkId The id of the given framework.
// @param executorId The id of the given executor.
// @param uuid The uuid of the given executor.
http://git-wip-us.apache.org/repos/asf/mesos/blob/8d742395/src/slave/flags.hpp
----------------------------------------------------------------------
diff --git a/src/slave/flags.hpp b/src/slave/flags.hpp
index ea1e4f7..db777e3 100644
--- a/src/slave/flags.hpp
+++ b/src/slave/flags.hpp
@@ -21,6 +21,7 @@
#include <string>
+#include <stout/bytes.hpp>
#include <stout/duration.hpp>
#include <stout/flags.hpp>
#include <stout/option.hpp>
@@ -179,6 +180,16 @@ public:
"Cgroups feature flag to enable hard limits on CPU resources\n"
"via the CFS bandwidth limiting subfeature.\n",
false);
+
+ add(&Flags::cgroups_oom_buffer,
+ "cgroups_oom_buffer",
+ "Additional memory alloted to executors in order to allow mesos\n"
+ "to soft-kill the executor prior to the kernel placing it under OOM.\n"
+ "(e.g. 128MB)\n"
+ "NOTE: This was added to compensate for a bug in the Linux kernel\n"
+ " that causes the system to lock up when the oom_killer is\n"
+ " disabled.",
+ Megabytes(128));
#endif
}
@@ -204,6 +215,7 @@ public:
std::string cgroups_root;
std::string cgroups_subsystems;
bool cgroups_enable_cfs;
+ Bytes cgroups_oom_buffer;
#endif
};