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 2017/08/24 00:56:09 UTC
[3/8] mesos git commit: Introduced a Framework::idle function in the
agent.
Introduced a Framework::idle function in the agent.
This ensures the call-sites consistently check idleness of the
framework, it also aids readability in that it clarifies that
we remove idle frameworks.
Review: https://reviews.apache.org/r/61642
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/5f7a95a1
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/5f7a95a1
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/5f7a95a1
Branch: refs/heads/1.3.x
Commit: 5f7a95a1f58add34daec51edcf762c3086c84e79
Parents: 080cbb6
Author: Benjamin Mahler <bm...@apache.org>
Authored: Fri Aug 4 15:51:05 2017 -0700
Committer: Benjamin Mahler <bm...@apache.org>
Committed: Wed Aug 23 17:37:35 2017 -0700
----------------------------------------------------------------------
src/slave/slave.cpp | 39 +++++++++++++++++++++------------------
src/slave/slave.hpp | 9 +++++++++
2 files changed, 30 insertions(+), 18 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/5f7a95a1/src/slave/slave.cpp
----------------------------------------------------------------------
diff --git a/src/slave/slave.cpp b/src/slave/slave.cpp
index 34e7842..7eb7f6b 100644
--- a/src/slave/slave.cpp
+++ b/src/slave/slave.cpp
@@ -1798,7 +1798,7 @@ void Slave::_run(
framework->removePendingTask(_task.task_id());
}
- if (framework->executors.empty() && framework->pending.empty()) {
+ if (framework->idle()) {
removeFramework(framework);
}
@@ -1843,7 +1843,7 @@ void Slave::_run(
statusUpdate(update, UPID());
}
- if (framework->executors.empty() && framework->pending.empty()) {
+ if (framework->idle()) {
removeFramework(framework);
}
@@ -1886,7 +1886,7 @@ void Slave::_run(
statusUpdate(update, UPID());
}
- if (framework->executors.empty() && framework->pending.empty()) {
+ if (framework->idle()) {
removeFramework(framework);
}
@@ -1959,7 +1959,7 @@ void Slave::__run(
framework->removePendingTask(_task.task_id());
}
- if (framework->executors.empty() && framework->pending.empty()) {
+ if (framework->idle()) {
removeFramework(framework);
}
@@ -1974,8 +1974,7 @@ void Slave::__run(
if (framework->removePendingTask(_task.task_id())) {
// NOTE: Ideally we would perform the following check here:
//
- // if (framework->executors.empty() &&
- // framework->pending.empty()) {
+ // if (framework->idle()) {
// removeFramework(framework);
// }
//
@@ -2007,7 +2006,7 @@ void Slave::__run(
// Refer to the comment after 'framework->removePendingTask' above
// for why we need this.
- if (framework->executors.empty() && framework->pending.empty()) {
+ if (framework->idle()) {
removeFramework(framework);
}
@@ -2079,7 +2078,7 @@ void Slave::__run(
// Refer to the comment after 'framework->removePendingTask' above
// for why we need this.
- if (framework->executors.empty() && framework->pending.empty()) {
+ if (framework->idle()) {
removeFramework(framework);
}
@@ -2148,7 +2147,7 @@ void Slave::__run(
// Refer to the comment after 'framework->removePendingTask' above
// for why we need this.
- if (framework->executors.empty() && framework->pending.empty()) {
+ if (framework->idle()) {
removeFramework(framework);
}
@@ -2201,7 +2200,7 @@ void Slave::__run(
// Refer to the comment after 'framework->removePendingTask' above
// for why we need this.
- if (framework->executors.empty() && framework->pending.empty()) {
+ if (framework->idle()) {
removeFramework(framework);
}
@@ -2220,7 +2219,7 @@ void Slave::__run(
// Refer to the comment after 'framework->removePendingTask' above
// for why we need this.
- if (framework->executors.empty() && framework->pending.empty()) {
+ if (framework->idle()) {
removeFramework(framework);
}
@@ -3114,7 +3113,7 @@ void Slave::shutdownFramework(
}
// Remove this framework if it has no pending executors and tasks.
- if (framework->executors.empty() && framework->pending.empty()) {
+ if (framework->idle()) {
removeFramework(framework);
}
break;
@@ -3556,7 +3555,7 @@ void Slave::_statusUpdateAcknowledgement(
}
// Remove this framework if it has no pending executors and tasks.
- if (framework->executors.empty() && framework->pending.empty()) {
+ if (framework->idle()) {
removeFramework(framework);
}
}
@@ -5309,7 +5308,7 @@ void Slave::executorTerminated(
}
// Remove this framework if it has no pending executors and tasks.
- if (framework->executors.empty() && framework->pending.empty()) {
+ if (framework->idle()) {
removeFramework(framework);
}
break;
@@ -5421,10 +5420,8 @@ void Slave::removeFramework(Framework* framework)
CHECK(framework->state == Framework::RUNNING ||
framework->state == Framework::TERMINATING);
- // The invariant here is that a framework should not be removed
- // if it has either pending executors or pending tasks.
- CHECK(framework->executors.empty());
- CHECK(framework->pending.empty());
+ // We only remove frameworks once they become idle.
+ CHECK(framework->idle());
// Close all status update streams for this framework.
statusUpdateManager->cleanup(framework->id());
@@ -6953,6 +6950,12 @@ Framework::Framework(
completedExecutors(slaveFlags.max_completed_executors_per_framework) {}
+bool Framework::idle() const
+{
+ return executors.empty() && pendingTasks.empty();
+}
+
+
void Framework::checkpointFramework() const
{
// Checkpoint the framework info.
http://git-wip-us.apache.org/repos/asf/mesos/blob/5f7a95a1/src/slave/slave.hpp
----------------------------------------------------------------------
diff --git a/src/slave/slave.hpp b/src/slave/slave.hpp
index d5a4b81..5037704 100644
--- a/src/slave/slave.hpp
+++ b/src/slave/slave.hpp
@@ -1112,6 +1112,15 @@ struct Framework
~Framework();
+ // Returns whether the framework is idle, where idle is
+ // defined as having no activity:
+ // (1) The framework has no non-terminal tasks and executors.
+ // (2) All status updates have been acknowledged.
+ //
+ // TODO(bmahler): The framework should also not be considered
+ // idle if there are unacknowledged updates for "pending" tasks.
+ bool idle() const;
+
Executor* addExecutor(const ExecutorInfo& executorInfo);
void destroyExecutor(const ExecutorID& executorId);
Executor* getExecutor(const ExecutorID& executorId) const;