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 2017/02/22 20:10:34 UTC
mesos git commit: Fixed nested container agent flapping issue after
reboot.
Repository: mesos
Updated Branches:
refs/heads/master 807b2343f -> 020b37ee9
Fixed nested container agent flapping issue after reboot.
When recovering containers in provisioner, there is a particular case
that after the machine reboots the container runtime directory and
slave state is gone but the provisioner directory still exists since
it is under the agent work_dir(e.g., agent work_dir is /var/lib/mesos).
Then, all checkpointed containers will be cleaned up as unknown
containers in provisioner during recovery. However, the semantic that
a child container is always cleaned up before its parent container
cannot be guaranteed for this particular case. Ideally, we should
put the provisioner directory under the container runtime dir but this
is not backward compactible. It is an unfortunate that we have to
make the provisioner::destroy() to be recursive.
Review: https://reviews.apache.org/r/56808/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/020b37ee
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/020b37ee
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/020b37ee
Branch: refs/heads/master
Commit: 020b37ee9c44007ecd0016fbbf6012054953dd5b
Parents: 807b234
Author: Gilbert Song <so...@gmail.com>
Authored: Wed Feb 22 09:10:58 2017 -0800
Committer: Jie Yu <yu...@gmail.com>
Committed: Wed Feb 22 09:11:11 2017 -0800
----------------------------------------------------------------------
.../mesos/provisioner/provisioner.cpp | 78 +++++++++++++++-----
.../mesos/provisioner/provisioner.hpp | 11 ++-
2 files changed, 70 insertions(+), 19 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/020b37ee/src/slave/containerizer/mesos/provisioner/provisioner.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/provisioner/provisioner.cpp b/src/slave/containerizer/mesos/provisioner/provisioner.cpp
index 8c20d40..478ab09 100644
--- a/src/slave/containerizer/mesos/provisioner/provisioner.cpp
+++ b/src/slave/containerizer/mesos/provisioner/provisioner.cpp
@@ -486,32 +486,68 @@ Future<bool> ProvisionerProcess::destroy(const ContainerID& containerId)
return false;
}
+ if (infos[containerId]->destroying) {
+ return infos[containerId]->termination.future();
+ }
+
+ infos[containerId]->destroying = true;
+
// Provisioner destroy can be invoked from:
// 1. Provisioner `recover` to destroy all unknown orphans.
// 2. Containerizer `recover` to destroy known orphans.
// 3. Containerizer `destroy` on one specific container.
//
- // In the above cases, we assume that the container being destroyed
- // has no corresponding child containers. We fail fast if this
- // condition is not satisfied.
+ // NOTE: For (2) and (3), we expect the container being destory
+ // has no any child contain remain running. However, for case (1),
+ // if the container runtime directory does not survive after the
+ // machine reboots and the provisioner directory under the agent
+ // work dir still exists, all containers will be regarded as
+ // unkown containers and will be destroyed. In this case, a parent
+ // container may be destoryed before its child containers are
+ // cleaned up. So we have to make `destroy()` recursively for
+ // this particular case.
//
- // NOTE: This check is expensive since it traverses the entire
- // `infos` hashmap. This is acceptable because we generally expect
- // the number of containers on a single agent to be on the order of
- // tens or hundreds of containers.
+ // TODO(gilbert): Move provisioner directory to the container
+ // runtime directory after a deprecation cycle to avoid
+ // making `provisioner::destroy()` being recursive.
+ list<Future<bool>> destroys;
+
foreachkey (const ContainerID& entry, infos) {
- if (entry.has_parent()) {
- CHECK(entry.parent() != containerId)
- << "Failed to destroy container "
- << containerId << " since its nested container "
- << entry << " has not been destroyed yet";
+ if (entry.has_parent() && entry.parent() == containerId) {
+ destroys.push_back(destroy(entry));
}
}
- // Unregister the container first. If destroy() fails, we can rely
- // on recover() to retry it later.
- Owned<Info> info = infos[containerId];
- infos.erase(containerId);
+ return await(destroys)
+ .then(defer(self(), &Self::_destroy, containerId, lambda::_1));
+}
+
+
+Future<bool> ProvisionerProcess::_destroy(
+ const ContainerID& containerId,
+ const list<Future<bool>>& destroys)
+{
+ CHECK(infos.contains(containerId));
+ CHECK(infos[containerId]->destroying);
+
+ vector<string> errors;
+ foreach (const Future<bool>& future, destroys) {
+ if (!future.isReady()) {
+ errors.push_back(future.isFailed()
+ ? future.failure()
+ : "discarded");
+ }
+ }
+
+ if (!errors.empty()) {
+ ++metrics.remove_container_errors;
+
+ return Failure(
+ "Failed to destory nested containers: " +
+ strings::join("; ", errors));
+ }
+
+ const Owned<Info>& info = infos[containerId];
list<Future<bool>> futures;
foreachkey (const string& backend, info->rootfses) {
@@ -541,12 +577,15 @@ Future<bool> ProvisionerProcess::destroy(const ContainerID& containerId)
// TODO(xujyan): Revisit the usefulness of this return value.
return collect(futures)
- .then(defer(self(), &ProvisionerProcess::_destroy, containerId));
+ .then(defer(self(), &ProvisionerProcess::__destroy, containerId));
}
-Future<bool> ProvisionerProcess::_destroy(const ContainerID& containerId)
+Future<bool> ProvisionerProcess::__destroy(const ContainerID& containerId)
{
+ CHECK(infos.contains(containerId));
+ CHECK(infos[containerId]->destroying);
+
// This should be fairly cheap as the directory should only
// contain a few empty sub-directories at this point.
//
@@ -566,6 +605,9 @@ Future<bool> ProvisionerProcess::_destroy(const ContainerID& containerId)
++metrics.remove_container_errors;
}
+ infos[containerId]->termination.set(true);
+ infos.erase(containerId);
+
return true;
}
http://git-wip-us.apache.org/repos/asf/mesos/blob/020b37ee/src/slave/containerizer/mesos/provisioner/provisioner.hpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/provisioner/provisioner.hpp b/src/slave/containerizer/mesos/provisioner/provisioner.hpp
index ff52e35..7d6c1b9 100644
--- a/src/slave/containerizer/mesos/provisioner/provisioner.hpp
+++ b/src/slave/containerizer/mesos/provisioner/provisioner.hpp
@@ -135,7 +135,11 @@ private:
const std::string& backend,
const ImageInfo& imageInfo);
- process::Future<bool> _destroy(const ContainerID& containerId);
+ process::Future<bool> _destroy(
+ const ContainerID& containerId,
+ const std::list<process::Future<bool>>& destroys);
+
+ process::Future<bool> __destroy(const ContainerID& containerId);
// Absolute path to the provisioner root directory. It can be
// derived from '--work_dir' but we keep a separate copy here
@@ -158,6 +162,11 @@ private:
{
// Mappings: backend -> {rootfsId, ...}
hashmap<std::string, hashset<std::string>> rootfses;
+
+ process::Promise<bool> termination;
+
+ // The container status in provisioner.
+ bool destroying = false;
};
hashmap<ContainerID, process::Owned<Info>> infos;