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/11/28 17:35:42 UTC

mesos git commit: Refactored offer operation handling for speculative operations.

Repository: mesos
Updated Branches:
  refs/heads/master dbf2707d9 -> abd300be7


Refactored offer operation handling for speculative operations.

Applying operations has been refactored out of 'addOfferOperation' and
simplified by using 'protobuf::isSpeculativeOperation'.

Review: https://reviews.apache.org/r/64054/


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/abd300be
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/abd300be
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/abd300be

Branch: refs/heads/master
Commit: abd300be7e6e984818e772cc774a831f38dba48b
Parents: dbf2707
Author: Jan Schlicht <ja...@mesosphere.io>
Authored: Tue Nov 28 09:03:23 2017 -0800
Committer: Jie Yu <yu...@gmail.com>
Committed: Tue Nov 28 09:23:49 2017 -0800

----------------------------------------------------------------------
 src/master/master.cpp | 62 ++++++++++++++--------------------------------
 src/slave/slave.cpp   | 44 +++++++++-----------------------
 2 files changed, 30 insertions(+), 76 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/abd300be/src/master/master.cpp
----------------------------------------------------------------------
diff --git a/src/master/master.cpp b/src/master/master.cpp
index 53263e4..700e124 100644
--- a/src/master/master.cpp
+++ b/src/master/master.cpp
@@ -9925,6 +9925,18 @@ void Master::_apply(
 
     addOfferOperation(framework, slave, offerOperation);
 
+    if (protobuf::isSpeculativeOperation(offerOperation->info())) {
+      Offer::Operation strippedOperation = offerOperation->info();
+      protobuf::stripAllocationInfo(&strippedOperation);
+
+      Try<vector<ResourceConversion>> conversions =
+        getResourceConversions(strippedOperation);
+
+      CHECK_SOME(conversions);
+
+      slave->apply(conversions.get());
+    }
+
     ApplyOfferOperationMessage message;
     if (framework != nullptr) {
       message.mutable_framework_id()->CopyFrom(framework->id());
@@ -10864,53 +10876,15 @@ void Slave::addOfferOperation(OfferOperation* operation)
 
   offerOperations.put(uuid.get(), operation);
 
-  Option<Resource> consumed;
-
-  switch (operation->info().type()) {
-    case Offer::Operation::LAUNCH:
-    case Offer::Operation::LAUNCH_GROUP:
-      LOG(FATAL) << "Unexpected offer operation to add on agent " << *this;
-      break;
-    case Offer::Operation::RESERVE:
-    case Offer::Operation::UNRESERVE:
-    case Offer::Operation::CREATE:
-    case Offer::Operation::DESTROY: {
-      // These operations are speculatively applied and will be
-      // reflected in the `totalResources` immediately. Resources used
-      // by those operations are not tracked as used resources.
-
-      // We need to strip the allocation info from the operation's
-      // resources in order to apply the operation successfully
-      // since the agent's total is stored as unallocated resources.
-      Offer::Operation strippedOperation = operation->info();
-      protobuf::stripAllocationInfo(&strippedOperation);
+  if (!protobuf::isSpeculativeOperation(operation->info())) {
+    Try<Resources> consumed = protobuf::getConsumedResources(operation->info());
 
-      Try<vector<ResourceConversion>> conversions =
-        getResourceConversions(strippedOperation);
+    CHECK_SOME(consumed);
 
-      CHECK_SOME(conversions);
-
-      apply(conversions.get());
-      break;
-    }
-    case Offer::Operation::CREATE_VOLUME:
-      consumed = operation->info().create_volume().source();
-      break;
-    case Offer::Operation::DESTROY_VOLUME:
-      consumed = operation->info().destroy_volume().volume();
-      break;
-    case Offer::Operation::CREATE_BLOCK:
-      consumed = operation->info().create_block().source();
-      break;
-    case Offer::Operation::DESTROY_BLOCK:
-      consumed = operation->info().destroy_block().block();
-      break;
-    case Offer::Operation::UNKNOWN:
-      LOG(WARNING) << "Ignoring unknown offer operation";
-      return;
-  }
+    // There isn't support for non-speculative operations using the
+    // operator API. We can assume the framework ID has been set.
+    CHECK(operation->has_framework_id());
 
-  if (consumed.isSome()) {
     usedResources[operation->framework_id()] += consumed.get();
   }
 }

http://git-wip-us.apache.org/repos/asf/mesos/blob/abd300be/src/slave/slave.cpp
----------------------------------------------------------------------
diff --git a/src/slave/slave.cpp b/src/slave/slave.cpp
index 6ed5c78..cd71647 100644
--- a/src/slave/slave.cpp
+++ b/src/slave/slave.cpp
@@ -3738,6 +3738,18 @@ void Slave::applyOfferOperation(const ApplyOfferOperationMessage& message)
 
   addOfferOperation(offerOperation);
 
+  if (protobuf::isSpeculativeOperation(message.operation_info())) {
+    Offer::Operation strippedOperation = message.operation_info();
+    protobuf::stripAllocationInfo(&strippedOperation);
+
+    Try<vector<ResourceConversion>> conversions =
+      getResourceConversions(strippedOperation);
+
+    CHECK_SOME(conversions);
+
+    apply(conversions.get());
+  }
+
   if (resourceProviderId.isSome()) {
     resourceProviderManager.applyOfferOperation(message);
     return;
@@ -6909,38 +6921,6 @@ void Slave::addOfferOperation(OfferOperation* operation)
   CHECK_SOME(uuid);
 
   offerOperations.put(uuid.get(), operation);
-
-  switch (operation->info().type()) {
-    case Offer::Operation::LAUNCH:
-      LOG(FATAL) << "Unexpected LAUNCH operation";
-      break;
-    case Offer::Operation::LAUNCH_GROUP:
-      LOG(FATAL) << "Unexpected LAUNCH_GROUP operation";
-      break;
-    case Offer::Operation::RESERVE:
-    case Offer::Operation::UNRESERVE:
-    case Offer::Operation::CREATE:
-    case Offer::Operation::DESTROY: {
-      Offer::Operation strippedOperation = operation->info();
-      protobuf::stripAllocationInfo(&strippedOperation);
-
-      Try<vector<ResourceConversion>> conversions =
-        getResourceConversions(strippedOperation);
-
-      CHECK_SOME(conversions);
-
-      apply(conversions.get());
-      break;
-    }
-    case Offer::Operation::CREATE_VOLUME:
-    case Offer::Operation::DESTROY_VOLUME:
-    case Offer::Operation::CREATE_BLOCK:
-    case Offer::Operation::DESTROY_BLOCK:
-      break;
-    case Offer::Operation::UNKNOWN:
-      LOG(WARNING) << "Unknown offer operation";
-      break;
-  }
 }