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 2016/03/16 21:36:55 UTC
mesos git commit: Fix for docker containerizer not configuring CFS
quotas correctly.
Repository: mesos
Updated Branches:
refs/heads/master dbb822123 -> be9e86022
Fix for docker containerizer not configuring CFS quotas correctly.
It would be nice to refactor all this isolation code in a way that can
be shared between all containerizers, as this is basically just copied
from the CgroupsCpushareIsolator, but that's a much bigger undertaking.
Review: https://reviews.apache.org/r/33174/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/be9e8602
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/be9e8602
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/be9e8602
Branch: refs/heads/master
Commit: be9e86022ff86620800503c41d2fef0c6387aaba
Parents: dbb8221
Author: Steve Niemitz <sn...@twitter.com>
Authored: Wed Mar 16 13:36:33 2016 -0700
Committer: Jie Yu <yu...@gmail.com>
Committed: Wed Mar 16 13:36:33 2016 -0700
----------------------------------------------------------------------
src/slave/containerizer/docker.cpp | 57 +++++++++++++++++++++++++++++++--
src/slave/containerizer/docker.hpp | 5 ++-
2 files changed, 58 insertions(+), 4 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/be9e8602/src/slave/containerizer/docker.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/docker.cpp b/src/slave/containerizer/docker.cpp
index fb9188a..76d24fc 100644
--- a/src/slave/containerizer/docker.cpp
+++ b/src/slave/containerizer/docker.cpp
@@ -694,7 +694,8 @@ Future<Nothing> DockerContainerizer::update(
process.get(),
&DockerContainerizerProcess::update,
containerId,
- resources);
+ resources,
+ false);
}
@@ -1065,6 +1066,16 @@ Future<bool> DockerContainerizerProcess::launch(
}))
.then(defer(self(), [=]() { return launchExecutorProcess(containerId); }))
.then(defer(self(), [=](pid_t pid) {
+ // Call update to set CPU/CFS/mem quotas at launch.
+ // TODO(steveniemitz): Once the minimum docker version supported
+ // is >= 1.7 this can be changed to pass --cpu-period and --cpu-quota
+ // to the 'docker run' call in launchExecutorProcess.
+ return update(containerId, executorInfo.resources(), true)
+ .then([=]() {
+ return Future<pid_t>(pid);
+ });
+ }))
+ .then(defer(self(), [=](pid_t pid) {
return reapExecutor(containerId, pid);
}));
}
@@ -1092,6 +1103,16 @@ Future<bool> DockerContainerizerProcess::launch(
return launchExecutorContainer(containerId, containerName);
}))
.then(defer(self(), [=](const Docker::Container& dockerContainer) {
+ // Call update to set CPU/CFS/mem quotas at launch.
+ // TODO(steveniemitz): Once the minimum docker version supported
+ // is >= 1.7 this can be changed to pass --cpu-period and --cpu-quota
+ // to the 'docker run' call in launchExecutorContainer.
+ return update(containerId, executorInfo.resources(), true)
+ .then([=]() {
+ return Future<Docker::Container>(dockerContainer);
+ });
+ }))
+ .then(defer(self(), [=](const Docker::Container& dockerContainer) {
return checkpointExecutor(containerId, dockerContainer);
}))
.then(defer(self(), [=](pid_t pid) {
@@ -1296,7 +1317,8 @@ Future<bool> DockerContainerizerProcess::reapExecutor(
Future<Nothing> DockerContainerizerProcess::update(
const ContainerID& containerId,
- const Resources& _resources)
+ const Resources& _resources,
+ bool force)
{
if (!containers_.contains(containerId)) {
LOG(WARNING) << "Ignoring updating unknown container: "
@@ -1312,7 +1334,7 @@ Future<Nothing> DockerContainerizerProcess::update(
return Nothing();
}
- if (container->resources == _resources) {
+ if (container->resources == _resources && !force) {
LOG(INFO) << "Ignoring updating container '" << containerId
<< "' with resources passed to update is identical to "
<< "existing resources";
@@ -1427,6 +1449,35 @@ Future<Nothing> DockerContainerizerProcess::__update(
LOG(INFO) << "Updated 'cpu.shares' to " << shares
<< " at " << path::join(cpuHierarchy.get(), cpuCgroup.get())
<< " for container " << containerId;
+
+ // Set cfs quota if enabled.
+ if (flags.cgroups_enable_cfs) {
+ write = cgroups::cpu::cfs_period_us(
+ cpuHierarchy.get(),
+ cpuCgroup.get(),
+ CPU_CFS_PERIOD);
+
+ if (write.isError()) {
+ return Failure("Failed to update 'cpu.cfs_period_us': " +
+ write.error());
+ }
+
+ Duration quota = std::max(CPU_CFS_PERIOD * cpuShares, MIN_CPU_CFS_QUOTA);
+
+ write = cgroups::cpu::cfs_quota_us(
+ cpuHierarchy.get(),
+ cpuCgroup.get(),
+ quota);
+
+ if (write.isError()) {
+ return Failure("Failed to update 'cpu.cfs_quota_us': " + write.error());
+ }
+
+ LOG(INFO) << "Updated 'cpu.cfs_period_us' to " << CPU_CFS_PERIOD
+ << " and 'cpu.cfs_quota_us' to " << quota
+ << " (cpus " << cpuShares << ")"
+ << " for container " << containerId;
+ }
}
// Now determine the cgroup for the 'memory' subsystem.
http://git-wip-us.apache.org/repos/asf/mesos/blob/be9e8602/src/slave/containerizer/docker.hpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/docker.hpp b/src/slave/containerizer/docker.hpp
index 79cd955..89d450e 100644
--- a/src/slave/containerizer/docker.hpp
+++ b/src/slave/containerizer/docker.hpp
@@ -142,9 +142,12 @@ public:
const process::PID<Slave>& slavePid,
bool checkpoint);
+ // force = true causes the containerizer to update the resources
+ // for the container, even if they match what it has cached.
virtual process::Future<Nothing> update(
const ContainerID& containerId,
- const Resources& resources);
+ const Resources& resources,
+ bool force);
virtual process::Future<ResourceStatistics> usage(
const ContainerID& containerId);