svn commit: r1205309 - in /incubator/mesos/trunk/src: slave/slave.cpp tests/master_tests.cpp tests/utils.hpp

[an...@apache.org]

Author: andrew
Date: Wed Nov 23 07:15:55 2011
New Revision: 1205309

URL: http://svn.apache.org/viewvc?rev=1205309&view=rev
Log:
Fixes MESOS-56. Thanks Charles Reiss!

Modified:
    incubator/mesos/trunk/src/slave/slave.cpp
    incubator/mesos/trunk/src/tests/master_tests.cpp
    incubator/mesos/trunk/src/tests/utils.hpp

Modified: incubator/mesos/trunk/src/slave/slave.cpp
URL: http://svn.apache.org/viewvc/incubator/mesos/trunk/src/slave/slave.cpp?rev=1205309&r1=1205308&r2=1205309&view=diff
==============================================================================
--- incubator/mesos/trunk/src/slave/slave.cpp (original)
+++ incubator/mesos/trunk/src/slave/slave.cpp Wed Nov 23 07:15:55 2011
@@ -441,17 +441,19 @@ void Slave::runTask(const FrameworkInfo&
 
       stats.tasks[TASK_STARTING]++;
 
+      // Update the resources.
+      // TODO(Charles Reiss): The isolation module is not guaranteed to update
+      // the resources before the executor acts on its RunTaskMessage.
+      dispatch(isolationModule,
+               &IsolationModule::resourcesChanged,
+               framework->id, executor->id, executor->resources);
+
       RunTaskMessage message;
       message.mutable_framework()->MergeFrom(framework->info);
       message.mutable_framework_id()->MergeFrom(framework->id);
       message.set_pid(framework->pid);
       message.mutable_task()->MergeFrom(task);
       send(executor->pid, message);
-
-      // Now update the resources.
-      dispatch(isolationModule,
-               &IsolationModule::resourcesChanged,
-               framework->id, executor->id, executor->resources);
     }
   } else {
     // Launch an executor for this task.
@@ -747,7 +749,17 @@ void Slave::registerExecutor(const Frame
     // Save the pid for the executor.
     executor->pid = from();
 
-    // Now that the executor is up, set its resource limits.
+    // First account for the tasks we're about to start.
+    foreachvalue (const TaskDescription& task, executor->queuedTasks) {
+      // Add the task to the executor.
+      executor->addTask(task);
+    }
+
+    // Now that the executor is up, set its resource limits including the
+    // currently queued tasks.
+    // TODO(Charles Reiss): We don't actually have a guarantee that this will
+    // be delivered or (where necessary) acted on before the executor gets its
+    // RunTaskMessages.
     dispatch(isolationModule,
              &IsolationModule::resourcesChanged,
              framework->id, executor->id, executor->resources);
@@ -765,11 +777,7 @@ void Slave::registerExecutor(const Frame
     LOG(INFO) << "Flushing queued tasks for framework " << framework->id;
 
     foreachvalue (const TaskDescription& task, executor->queuedTasks) {
-      // Add the task to the executor.
-      executor->addTask(task);
-
       stats.tasks[TASK_STARTING]++;
-
       RunTaskMessage message;
       message.mutable_framework_id()->MergeFrom(framework->id);
       message.mutable_framework()->MergeFrom(framework->info);

Modified: incubator/mesos/trunk/src/tests/master_tests.cpp
URL: http://svn.apache.org/viewvc/incubator/mesos/trunk/src/tests/master_tests.cpp?rev=1205309&r1=1205308&r2=1205309&view=diff
==============================================================================
--- incubator/mesos/trunk/src/tests/master_tests.cpp (original)
+++ incubator/mesos/trunk/src/tests/master_tests.cpp Wed Nov 23 07:15:55 2011
@@ -101,7 +101,7 @@ TEST(MasterTest, TaskRunning)
   vector<Offer> offers;
   TaskStatus status;
 
-  trigger resourceOffersCall, statusUpdateCall;
+  trigger resourceOffersCall, statusUpdateCall, resourcesChangedCall;
 
   EXPECT_CALL(sched, registered(&driver, _))
     .Times(1);
@@ -129,12 +129,17 @@ TEST(MasterTest, TaskRunning)
   vector<TaskDescription> tasks;
   tasks.push_back(task);
 
+  EXPECT_CALL(isolationModule, resourcesChanged(_, _,
+        Resources(offers[0].resources())))
+    .WillOnce(Trigger(&resourcesChangedCall));
+
   driver.launchTasks(offers[0].id(), tasks);
 
   WAIT_UNTIL(statusUpdateCall);
-
   EXPECT_EQ(TASK_RUNNING, status.state());
 
+  WAIT_UNTIL(resourcesChangedCall);
+
   driver.stop();
   driver.join();
 

Modified: incubator/mesos/trunk/src/tests/utils.hpp
URL: http://svn.apache.org/viewvc/incubator/mesos/trunk/src/tests/utils.hpp?rev=1205309&r1=1205308&r2=1205309&view=diff
==============================================================================
--- incubator/mesos/trunk/src/tests/utils.hpp (original)
+++ incubator/mesos/trunk/src/tests/utils.hpp Wed Nov 23 07:15:55 2011
@@ -244,7 +244,11 @@ class TestingIsolationModule : public sl
 {
 public:
   TestingIsolationModule(const std::map<ExecutorID, Executor*>& _executors)
-    : executors(_executors) {}
+    : executors(_executors)
+  {
+    EXPECT_CALL(*this, resourcesChanged(testing::_, testing::_, testing::_))
+      .Times(testing::AnyNumber());
+  }
 
   virtual ~TestingIsolationModule() {}
 
@@ -300,10 +304,10 @@ public:
     }
   }
 
-  virtual void resourcesChanged(const FrameworkID& frameworkId,
-                                const ExecutorID& executorId,
-                                const Resources& resources)
-  {}
+  // Mocked so tests can check that the resources reflect all started tasks.
+  MOCK_METHOD3(resourcesChanged, void(const FrameworkID&,
+                                      const ExecutorID&,
+                                      const Resources&));
 
   std::map<ExecutorID, std::string> directories;