You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by an...@apache.org on 2017/12/04 20:56:43 UTC
mesos git commit: Improved Windows isolators with `struct Info`
abstraction.
Repository: mesos
Updated Branches:
refs/heads/master 6f3c74a4a -> 6a98530a5
Improved Windows isolators with `struct Info` abstraction.
Replaced the two maps of `ContainerId -> pid` and `ContainerId -> limit`
with a mapping of `ContainerId -> struct Info { pid, limit }`. This
abstraction correctly ties the the `pid` and cpu/mem `limit` together.
Notably this fixes a subtle bug in `prepare` so that it can no longer be
called multiple times if a resource limit hadn't been set.
Furthermore, this patch imports all the types used in the source files
to enchance code conciseness, and then reformatted with `clang-format`.
Review: https://reviews.apache.org/r/64272
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/6a98530a
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/6a98530a
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/6a98530a
Branch: refs/heads/master
Commit: 6a98530a5d5d498a77a2004b9953f939fe907779
Parents: 6f3c74a
Author: Andrew Schwartzmeyer <an...@schwartzmeyer.com>
Authored: Fri Dec 1 15:07:47 2017 -0800
Committer: Andrew Schwartzmeyer <an...@schwartzmeyer.com>
Committed: Mon Dec 4 12:40:48 2017 -0800
----------------------------------------------------------------------
.../mesos/isolators/windows/cpu.cpp | 87 +++++++++++++-------
.../mesos/isolators/windows/cpu.hpp | 9 +-
.../mesos/isolators/windows/mem.cpp | 87 +++++++++++++-------
.../mesos/isolators/windows/mem.hpp | 9 +-
4 files changed, 124 insertions(+), 68 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/6a98530a/src/slave/containerizer/mesos/isolators/windows/cpu.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/isolators/windows/cpu.cpp b/src/slave/containerizer/mesos/isolators/windows/cpu.cpp
index e9f4dac..782c7ad 100644
--- a/src/slave/containerizer/mesos/isolators/windows/cpu.cpp
+++ b/src/slave/containerizer/mesos/isolators/windows/cpu.cpp
@@ -33,8 +33,18 @@
#include "slave/containerizer/mesos/isolators/windows/cpu.hpp"
+using mesos::slave::ContainerConfig;
+using mesos::slave::ContainerLaunchInfo;
+using mesos::slave::ContainerState;
+using mesos::slave::Isolator;
+
+using process::Clock;
using process::Failure;
using process::Future;
+using process::Owned;
+
+using std::list;
+using std::max;
namespace mesos {
namespace internal {
@@ -50,37 +60,36 @@ bool WindowsCpuIsolatorProcess::supportsNesting() { return true; }
// When recovering, this ensures that our ContainerID -> PID mapping is
// recreated.
Future<Nothing> WindowsCpuIsolatorProcess::recover(
- const std::list<mesos::slave::ContainerState>& state,
- const hashset<ContainerID>& orphans)
+ const list<ContainerState>& state, const hashset<ContainerID>& orphans)
{
- foreach (const mesos::slave::ContainerState& run, state) {
+ foreach (const ContainerState& run, state) {
// This should (almost) never occur: see comment in
// SubprocessLauncher::recover().
- if (pids.contains(run.container_id())) {
+ if (infos.contains(run.container_id())) {
return Failure("Container already recovered");
}
- pids.put(run.container_id(), run.pid());
+ infos[run.container_id()] = {run.pid(), None()};
}
return Nothing();
}
-process::Future<Option<mesos::slave::ContainerLaunchInfo>>
-WindowsCpuIsolatorProcess::prepare(
- const ContainerID& containerId,
- const mesos::slave::ContainerConfig& containerConfig)
+Future<Option<ContainerLaunchInfo>> WindowsCpuIsolatorProcess::prepare(
+ const ContainerID& containerId, const ContainerConfig& containerConfig)
{
- if (cpuLimits.contains(containerId)) {
+ if (infos.contains(containerId)) {
return Failure("Container already prepared: " + stringify(containerId));
}
- Resources resources{containerConfig.resources()};
+ infos[containerId] = {};
+
+ const Resources resources = containerConfig.resources();
if (resources.cpus().isSome()) {
// Save the limit information so that `isolate` can set the limit
// immediately.
- cpuLimits[containerId] = std::max(resources.cpus().get(), MIN_CPU);
+ infos[containerId].limit = max(resources.cpus().get(), MIN_CPU);
}
return None();
@@ -92,15 +101,20 @@ WindowsCpuIsolatorProcess::prepare(
Future<Nothing> WindowsCpuIsolatorProcess::isolate(
const ContainerID& containerId, pid_t pid)
{
- if (pids.contains(containerId)) {
+ if (!infos.contains(containerId)) {
+ return Failure(
+ "Container not prepared before isolation: " + stringify(containerId));
+ }
+
+ if (infos[containerId].pid.isSome()) {
return Failure("Container already isolated: " + stringify(containerId));
}
- pids.put(containerId, pid);
+ infos[containerId].pid = pid;
- if (cpuLimits.contains(containerId)) {
- Try<Nothing> set =
- os::set_job_cpu_limit(pids[containerId], cpuLimits[containerId]);
+ if (infos[containerId].limit.isSome()) {
+ const Try<Nothing> set = os::set_job_cpu_limit(
+ infos[containerId].pid.get(), infos[containerId].limit.get());
if (set.isError()) {
return Failure(
"Failed to update container '" + stringify(containerId) +
@@ -115,23 +129,21 @@ Future<Nothing> WindowsCpuIsolatorProcess::isolate(
Future<Nothing> WindowsCpuIsolatorProcess::cleanup(
const ContainerID& containerId)
{
- if (!pids.contains(containerId)) {
+ if (!infos.contains(containerId)) {
VLOG(1) << "Ignoring cleanup request for unknown container " << containerId;
return Nothing();
}
- pids.erase(containerId);
- cpuLimits.erase(containerId);
+ infos.erase(containerId);
return Nothing();
}
-Try<mesos::slave::Isolator*> WindowsCpuIsolatorProcess::create(
- const Flags& flags)
+Try<Isolator*> WindowsCpuIsolatorProcess::create(const Flags& flags)
{
- process::Owned<MesosIsolatorProcess> process(new WindowsCpuIsolatorProcess());
+ Owned<MesosIsolatorProcess> process(new WindowsCpuIsolatorProcess());
return new MesosIsolator(process);
}
@@ -144,19 +156,24 @@ Future<Nothing> WindowsCpuIsolatorProcess::update(
return Failure("Not supported for nested containers");
}
- if (!pids.contains(containerId)) {
+ if (!infos.contains(containerId)) {
return Failure("Unknown container: " + stringify(containerId));
}
+ if (!infos[containerId].pid.isSome()) {
+ return Failure(
+ "Container not isolated before update: " + stringify(containerId));
+ }
+
if (resources.cpus().isNone()) {
return Failure(
"Failed to update container '" + stringify(containerId) +
"': No cpus resource given");
}
- cpuLimits[containerId] = std::max(resources.cpus().get(), MIN_CPU);
- Try<Nothing> set =
- os::set_job_cpu_limit(pids[containerId], cpuLimits[containerId]);
+ infos[containerId].limit = max(resources.cpus().get(), MIN_CPU);
+ const Try<Nothing> set = os::set_job_cpu_limit(
+ infos[containerId].pid.get(), infos[containerId].limit.get());
if (set.isError()) {
return Failure(
"Failed to update container '" + stringify(containerId) +
@@ -170,19 +187,25 @@ Future<Nothing> WindowsCpuIsolatorProcess::update(
Future<ResourceStatistics> WindowsCpuIsolatorProcess::usage(
const ContainerID& containerId)
{
- if (!pids.contains(containerId)) {
+ ResourceStatistics result;
+ result.set_timestamp(Clock::now().secs());
+
+ if (!infos.contains(containerId)) {
LOG(WARNING) << "No resource usage for unknown container '" << containerId
<< "'";
- return ResourceStatistics();
+ return result;
}
- ResourceStatistics result;
+ if (!infos[containerId].pid.isSome()) {
+ LOG(WARNING) << "No resource usage for container with unknown PID '"
+ << containerId << "'";
- result.set_timestamp(process::Clock::now().secs());
+ return result;
+ }
const Try<JOBOBJECT_BASIC_ACCOUNTING_INFORMATION> info =
- os::get_job_info(pids[containerId]);
+ os::get_job_info(infos[containerId].pid.get());
if (info.isError()) {
return result;
}
http://git-wip-us.apache.org/repos/asf/mesos/blob/6a98530a/src/slave/containerizer/mesos/isolators/windows/cpu.hpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/isolators/windows/cpu.hpp b/src/slave/containerizer/mesos/isolators/windows/cpu.hpp
index 8672b59..b996b07 100644
--- a/src/slave/containerizer/mesos/isolators/windows/cpu.hpp
+++ b/src/slave/containerizer/mesos/isolators/windows/cpu.hpp
@@ -63,8 +63,13 @@ public:
process::Future<Nothing> cleanup(const ContainerID& containerId) override;
private:
- hashmap<ContainerID, pid_t> pids;
- hashmap<ContainerID, double> cpuLimits;
+ struct Info
+ {
+ Option<pid_t> pid;
+ Option<double> limit;
+ };
+
+ hashmap<ContainerID, Info> infos;
WindowsCpuIsolatorProcess()
: ProcessBase(process::ID::generate("windows-cpu-isolator"))
http://git-wip-us.apache.org/repos/asf/mesos/blob/6a98530a/src/slave/containerizer/mesos/isolators/windows/mem.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/isolators/windows/mem.cpp b/src/slave/containerizer/mesos/isolators/windows/mem.cpp
index 5abcb4e..c6a2ded 100644
--- a/src/slave/containerizer/mesos/isolators/windows/mem.cpp
+++ b/src/slave/containerizer/mesos/isolators/windows/mem.cpp
@@ -34,8 +34,18 @@
#include "slave/containerizer/mesos/isolators/windows/mem.hpp"
+using mesos::slave::ContainerConfig;
+using mesos::slave::ContainerLaunchInfo;
+using mesos::slave::ContainerState;
+using mesos::slave::Isolator;
+
+using process::Clock;
using process::Failure;
using process::Future;
+using process::Owned;
+
+using std::list;
+using std::max;
namespace mesos {
namespace internal {
@@ -51,38 +61,36 @@ bool WindowsMemIsolatorProcess::supportsNesting() { return true; }
// When recovering, this ensures that our ContainerID -> PID mapping is
// recreated.
Future<Nothing> WindowsMemIsolatorProcess::recover(
- const std::list<mesos::slave::ContainerState>& state,
- const hashset<ContainerID>& orphans)
+ const list<ContainerState>& state, const hashset<ContainerID>& orphans)
{
- foreach (const mesos::slave::ContainerState& run, state) {
+ foreach (const ContainerState& run, state) {
// This should (almost) never occur: see comment in
// SubprocessLauncher::recover().
- if (pids.contains(run.container_id())) {
+ if (infos.contains(run.container_id())) {
return Failure("Container already recovered");
}
- pids.put(run.container_id(), run.pid());
+ infos[run.container_id()] = {run.pid(), None()};
}
return Nothing();
}
-process::Future<Option<mesos::slave::ContainerLaunchInfo>>
-WindowsMemIsolatorProcess::prepare(
- const ContainerID& containerId,
- const mesos::slave::ContainerConfig& containerConfig)
+Future<Option<ContainerLaunchInfo>> WindowsMemIsolatorProcess::prepare(
+ const ContainerID& containerId, const ContainerConfig& containerConfig)
{
- if (memLimits.contains(containerId)) {
+ if (infos.contains(containerId)) {
return Failure("Container already prepared: " + stringify(containerId));
}
- Resources resources{containerConfig.resources()};
+ infos[containerId] = {};
+ const Resources resources = containerConfig.resources();
if (resources.mem().isSome()) {
// Save the limit information so that `isolate` can set the limit
// immediately.
- memLimits[containerId] = std::max(resources.mem().get(), MIN_MEM);
+ infos[containerId].limit = max(resources.mem().get(), MIN_MEM);
}
return None();
@@ -94,15 +102,20 @@ WindowsMemIsolatorProcess::prepare(
Future<Nothing> WindowsMemIsolatorProcess::isolate(
const ContainerID& containerId, pid_t pid)
{
- if (pids.contains(containerId)) {
+ if (!infos.contains(containerId)) {
+ return Failure(
+ "Container not prepared before isolation: " + stringify(containerId));
+ }
+
+ if (infos[containerId].pid.isSome()) {
return Failure("Container already isolated: " + stringify(containerId));
}
- pids.put(containerId, pid);
+ infos[containerId].pid = pid;
- if (memLimits.contains(containerId)) {
- Try<Nothing> set =
- os::set_job_mem_limit(pids[containerId], memLimits[containerId]);
+ if (infos[containerId].limit.isSome()) {
+ const Try<Nothing> set = os::set_job_mem_limit(
+ infos[containerId].pid.get(), infos[containerId].limit.get());
if (set.isError()) {
return Failure(
"Failed to isolate container '" + stringify(containerId) +
@@ -117,23 +130,21 @@ Future<Nothing> WindowsMemIsolatorProcess::isolate(
Future<Nothing> WindowsMemIsolatorProcess::cleanup(
const ContainerID& containerId)
{
- if (!pids.contains(containerId)) {
+ if (!infos.contains(containerId)) {
VLOG(1) << "Ignoring cleanup request for unknown container " << containerId;
return Nothing();
}
- pids.erase(containerId);
- memLimits.erase(containerId);
+ infos.erase(containerId);
return Nothing();
}
-Try<mesos::slave::Isolator*> WindowsMemIsolatorProcess::create(
- const Flags& flags)
+Try<Isolator*> WindowsMemIsolatorProcess::create(const Flags& flags)
{
- process::Owned<MesosIsolatorProcess> process(new WindowsMemIsolatorProcess());
+ Owned<MesosIsolatorProcess> process(new WindowsMemIsolatorProcess());
return new MesosIsolator(process);
}
@@ -146,19 +157,24 @@ Future<Nothing> WindowsMemIsolatorProcess::update(
return Failure("Not supported for nested containers");
}
- if (!pids.contains(containerId)) {
+ if (!infos.contains(containerId)) {
return Failure("Unknown container: " + stringify(containerId));
}
+ if (!infos[containerId].pid.isSome()) {
+ return Failure(
+ "Container not isolated before update: " + stringify(containerId));
+ }
+
if (resources.mem().isNone()) {
return Failure(
"Failed to update container '" + stringify(containerId) +
"': No mem resource given");
}
- memLimits[containerId] = std::max(resources.mem().get(), MIN_MEM);
- Try<Nothing> set =
- os::set_job_mem_limit(pids[containerId], memLimits[containerId]);
+ infos[containerId].limit = max(resources.mem().get(), MIN_MEM);
+ const Try<Nothing> set = os::set_job_mem_limit(
+ infos[containerId].pid.get(), infos[containerId].limit.get());
if (set.isError()) {
return Failure(
"Failed to update container '" + stringify(containerId) +
@@ -172,18 +188,25 @@ Future<Nothing> WindowsMemIsolatorProcess::update(
Future<ResourceStatistics> WindowsMemIsolatorProcess::usage(
const ContainerID& containerId)
{
- if (!pids.contains(containerId)) {
+ ResourceStatistics result;
+ result.set_timestamp(Clock::now().secs());
+
+ if (!infos.contains(containerId)) {
LOG(WARNING) << "No resource usage for unknown container '" << containerId
<< "'";
- return ResourceStatistics();
+ return result;
}
- ResourceStatistics result;
+ if (!infos[containerId].pid.isSome()) {
+ LOG(WARNING) << "No resource usage for container with unknown PID '"
+ << containerId << "'";
- result.set_timestamp(process::Clock::now().secs());
+ return result;
+ }
- const Try<Bytes> mem_total_bytes = os::get_job_mem(pids[containerId]);
+ const Try<Bytes> mem_total_bytes =
+ os::get_job_mem(infos[containerId].pid.get());
if (mem_total_bytes.isSome()) {
result.set_mem_total_bytes(mem_total_bytes.get().bytes());
}
http://git-wip-us.apache.org/repos/asf/mesos/blob/6a98530a/src/slave/containerizer/mesos/isolators/windows/mem.hpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/isolators/windows/mem.hpp b/src/slave/containerizer/mesos/isolators/windows/mem.hpp
index 24426f7..8c4e3fe 100644
--- a/src/slave/containerizer/mesos/isolators/windows/mem.hpp
+++ b/src/slave/containerizer/mesos/isolators/windows/mem.hpp
@@ -64,8 +64,13 @@ public:
process::Future<Nothing> cleanup(const ContainerID& containerId) override;
private:
- hashmap<ContainerID, pid_t> pids;
- hashmap<ContainerID, Bytes> memLimits;
+ struct Info
+ {
+ Option<pid_t> pid;
+ Option<Bytes> limit;
+ };
+
+ hashmap<ContainerID, Info> infos;
WindowsMemIsolatorProcess()
: ProcessBase(process::ID::generate("windows-mem-isolator"))