mesos-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From bmah...@apache.org
Subject [05/13] mesos git commit: Introduced a Framework::idle function in the agent.
Date Wed, 23 Aug 2017 22:23:48 GMT
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/509147d8
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/509147d8
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/509147d8

Branch: refs/heads/master
Commit: 509147d85f2e60af33434d42a8ebf5d41209604c
Parents: 95aa9e5
Author: Benjamin Mahler <bmahler@apache.org>
Authored: Fri Aug 4 15:51:05 2017 -0700
Committer: Benjamin Mahler <bmahler@apache.org>
Committed: Wed Aug 23 15:09:07 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/509147d8/src/slave/slave.cpp
----------------------------------------------------------------------
diff --git a/src/slave/slave.cpp b/src/slave/slave.cpp
index 534ea23..05505a6 100644
--- a/src/slave/slave.cpp
+++ b/src/slave/slave.cpp
@@ -1896,7 +1896,7 @@ void Slave::_run(
       framework->removePendingTask(_task.task_id());
     }
 
-    if (framework->executors.empty() && framework->pending.empty()) {
+    if (framework->idle()) {
       removeFramework(framework);
     }
 
@@ -1941,7 +1941,7 @@ void Slave::_run(
       statusUpdate(update, UPID());
     }
 
-    if (framework->executors.empty() && framework->pending.empty()) {
+    if (framework->idle()) {
       removeFramework(framework);
     }
 
@@ -1984,7 +1984,7 @@ void Slave::_run(
       statusUpdate(update, UPID());
     }
 
-    if (framework->executors.empty() && framework->pending.empty()) {
+    if (framework->idle()) {
       removeFramework(framework);
     }
 
@@ -2057,7 +2057,7 @@ void Slave::__run(
       framework->removePendingTask(_task.task_id());
     }
 
-    if (framework->executors.empty() && framework->pending.empty()) {
+    if (framework->idle()) {
       removeFramework(framework);
     }
 
@@ -2072,8 +2072,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);
       //   }
       //
@@ -2105,7 +2104,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);
     }
 
@@ -2177,7 +2176,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);
     }
 
@@ -2246,7 +2245,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);
     }
 
@@ -2299,7 +2298,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);
     }
 
@@ -2318,7 +2317,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);
     }
 
@@ -3226,7 +3225,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;
@@ -3671,7 +3670,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);
   }
 }
@@ -5437,7 +5436,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;
@@ -5557,10 +5556,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());
@@ -7105,6 +7102,12 @@ Framework::~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/509147d8/src/slave/slave.hpp
----------------------------------------------------------------------
diff --git a/src/slave/slave.hpp b/src/slave/slave.hpp
index 3965fec..67ffdc9 100644
--- a/src/slave/slave.hpp
+++ b/src/slave/slave.hpp
@@ -851,6 +851,15 @@ public:
 
   ~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;
+
   void checkpointFramework() const;
 
   const FrameworkID id() const { return info.id(); }


Mime
View raw message