mesos-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ji...@apache.org
Subject [12/15] mesos git commit: Updated Resources::apply for new operations.
Date Mon, 30 Oct 2017 15:18:58 GMT
Updated Resources::apply for new operations.

This patch updated Resources::apply method to support new operations:
CREATE_VOLUME, DESTROY_VOLUME, CREATE_BLOCK and DESTROY_BLOCK. We
introduced an optional `convertedResources` field to this method for
new operations.

This patch also made sure that we don't call Resources::apply for
LAUNCH and LAUNCH_GROUP, because it does not really "convert" the
resources.

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


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

Branch: refs/heads/master
Commit: e5823186989912b1dc0121617a68771526d8418d
Parents: 2a620fd
Author: Jie Yu <yujie.jay@gmail.com>
Authored: Tue Oct 24 20:03:22 2017 +0200
Committer: Jie Yu <yujie.jay@gmail.com>
Committed: Mon Oct 30 16:18:31 2017 +0100

----------------------------------------------------------------------
 include/mesos/resources.hpp                  | 13 ++--
 include/mesos/v1/resources.hpp               | 13 ++--
 src/common/resources.cpp                     | 94 ++++++++++++++++++++---
 src/examples/persistent_volume_framework.cpp |  5 +-
 src/master/allocator/mesos/hierarchical.cpp  | 59 ++++++++++++--
 src/v1/resources.cpp                         | 94 ++++++++++++++++++++---
 6 files changed, 237 insertions(+), 41 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/e5823186/include/mesos/resources.hpp
----------------------------------------------------------------------
diff --git a/include/mesos/resources.hpp b/include/mesos/resources.hpp
index 2f76ea7..53152b3 100644
--- a/include/mesos/resources.hpp
+++ b/include/mesos/resources.hpp
@@ -464,12 +464,13 @@ public:
   // example frameworks to leverage.
   Option<Resources> find(const Resources& targets) const;
 
-  // Certain offer operations (e.g., RESERVE, UNRESERVE, CREATE or
-  // DESTROY) alter the offered resources. The following methods
-  // provide a convenient way to get the transformed resources by
-  // applying the given offer operation(s). Returns an Error if the
-  // offer operation(s) cannot be applied.
-  Try<Resources> apply(const Offer::Operation& operation) const;
+  // Certain offer operations alter the offered resources. The
+  // following methods provide a convenient way to get the transformed
+  // resources by applying the given offer operation(s). Returns an
+  // Error if the offer operation(s) cannot be applied.
+  Try<Resources> apply(
+      const Offer::Operation& operation,
+      const Option<Resources>& convertedResources = None()) const;
 
   template <typename Iterable>
   Try<Resources> apply(const Iterable& operations) const

http://git-wip-us.apache.org/repos/asf/mesos/blob/e5823186/include/mesos/v1/resources.hpp
----------------------------------------------------------------------
diff --git a/include/mesos/v1/resources.hpp b/include/mesos/v1/resources.hpp
index 7666c76..6c2191c 100644
--- a/include/mesos/v1/resources.hpp
+++ b/include/mesos/v1/resources.hpp
@@ -464,12 +464,13 @@ public:
   // example frameworks to leverage.
   Option<Resources> find(const Resources& targets) const;
 
-  // Certain offer operations (e.g., RESERVE, UNRESERVE, CREATE or
-  // DESTROY) alter the offered resources. The following methods
-  // provide a convenient way to get the transformed resources by
-  // applying the given offer operation(s). Returns an Error if the
-  // offer operation(s) cannot be applied.
-  Try<Resources> apply(const Offer::Operation& operation) const;
+  // Certain offer operations alter the offered resources. The
+  // following methods provide a convenient way to get the transformed
+  // resources by applying the given offer operation(s). Returns an
+  // Error if the offer operation(s) cannot be applied.
+  Try<Resources> apply(
+      const Offer::Operation& operation,
+      const Option<Resources>& convertedResources = None()) const;
 
   template <typename Iterable>
   Try<Resources> apply(const Iterable& operations) const

http://git-wip-us.apache.org/repos/asf/mesos/blob/e5823186/src/common/resources.cpp
----------------------------------------------------------------------
diff --git a/src/common/resources.cpp b/src/common/resources.cpp
index 791b9cf..914d3e2 100644
--- a/src/common/resources.cpp
+++ b/src/common/resources.cpp
@@ -1623,20 +1623,25 @@ Option<Resources> Resources::find(const Resources& targets)
const
 }
 
 
-Try<Resources> Resources::apply(const Offer::Operation& operation) const
+Try<Resources> Resources::apply(
+    const Offer::Operation& operation,
+    const Option<Resources>& convertedResources) const
 {
   Resources result = *this;
 
   switch (operation.type()) {
     case Offer::Operation::LAUNCH:
-      // Launch operation does not alter the offered resources.
-      break;
+      return Error("Cannot apply LAUNCH Operation");
 
     case Offer::Operation::LAUNCH_GROUP:
-      // LaunchGroup operation does not alter the offered resources.
-      break;
+      return Error("Cannot apply LAUNCH_GROUP Operation");
 
     case Offer::Operation::RESERVE: {
+      if (convertedResources.isSome()) {
+        return Error(
+            "Converted resources not expected for RESERVE Operation");
+      }
+
       Option<Error> error = validate(operation.reserve().resources());
       if (error.isSome()) {
         return Error("Invalid RESERVE Operation: " + error->message);
@@ -1666,6 +1671,11 @@ Try<Resources> Resources::apply(const Offer::Operation& operation)
const
     }
 
     case Offer::Operation::UNRESERVE: {
+      if (convertedResources.isSome()) {
+        return Error(
+            "Converted resources not expected for UNRESERVE Operation");
+      }
+
       Option<Error> error = validate(operation.unreserve().resources());
       if (error.isSome()) {
         return Error("Invalid UNRESERVE Operation: " + error->message);
@@ -1695,6 +1705,11 @@ Try<Resources> Resources::apply(const Offer::Operation& operation)
const
     }
 
     case Offer::Operation::CREATE: {
+      if (convertedResources.isSome()) {
+        return Error(
+            "Converted resources not expected for CREATE Operation");
+      }
+
       Option<Error> error = validate(operation.create().volumes());
       if (error.isSome()) {
         return Error("Invalid CREATE Operation: " + error->message);
@@ -1738,6 +1753,11 @@ Try<Resources> Resources::apply(const Offer::Operation& operation)
const
     }
 
     case Offer::Operation::DESTROY: {
+      if (convertedResources.isSome()) {
+        return Error(
+            "Converted resources not expected for DESTROY Operation");
+      }
+
       Option<Error> error = validate(operation.destroy().volumes());
       if (error.isSome()) {
         return Error("Invalid DESTROY Operation: " + error->message);
@@ -1785,22 +1805,78 @@ Try<Resources> Resources::apply(const Offer::Operation&
operation) const
     }
 
     case Offer::Operation::CREATE_VOLUME: {
-      // CreateVolume operation does not alter the offered resources.
+      if (convertedResources.isNone()) {
+        return Error(
+            "Converted resources not specified for CREATE_VOLUME Operation");
+      }
+
+      const Resource& consumed = operation.create_volume().source();
+
+      if (!result.contains(consumed)) {
+        return Error(
+            "Invalid CREATE_VOLUME Operation: " + stringify(result) +
+            " does not contain " + stringify(consumed));
+      }
+
+      result.subtract(consumed);
+      result += convertedResources.get();
       break;
     }
 
     case Offer::Operation::DESTROY_VOLUME: {
-      // DestroyVolume operation does not alter the offered resources.
+      if (convertedResources.isNone()) {
+        return Error(
+            "Converted resources not specified for DESTROY_VOLUME Operation");
+      }
+
+      const Resource& consumed = operation.destroy_volume().volume();
+
+      if (!result.contains(consumed)) {
+        return Error(
+            "Invalid DESTROY_VOLUME Operation: " + stringify(result) +
+            " does not contain " + stringify(consumed));
+      }
+
+      result.subtract(consumed);
+      result += convertedResources.get();
       break;
     }
 
     case Offer::Operation::CREATE_BLOCK: {
-      // CreateBlock operation does not alter the offered resources.
+      if (convertedResources.isNone()) {
+        return Error(
+            "Converted resources not specified for CREATE_BLOCK Operation");
+      }
+
+      const Resource& consumed = operation.create_block().source();
+
+      if (!result.contains(consumed)) {
+        return Error(
+            "Invalid CREATE_BLOCK Operation: " + stringify(result) +
+            " does not contain " + stringify(consumed));
+      }
+
+      result.subtract(consumed);
+      result += convertedResources.get();
       break;
     }
 
     case Offer::Operation::DESTROY_BLOCK: {
-      // DestroyBlock operation does not alter the offered resources.
+      if (convertedResources.isNone()) {
+        return Error(
+            "Converted resources not specified for DESTROY_BLOCK Operation");
+      }
+
+      const Resource& consumed = operation.destroy_block().block();
+
+      if (!result.contains(consumed)) {
+        return Error(
+            "Invalid DESTROY_BLOCK Operation: " + stringify(result) +
+            " does not contain " + stringify(consumed));
+      }
+
+      result.subtract(consumed);
+      result += convertedResources.get();
       break;
     }
 

http://git-wip-us.apache.org/repos/asf/mesos/blob/e5823186/src/examples/persistent_volume_framework.cpp
----------------------------------------------------------------------
diff --git a/src/examples/persistent_volume_framework.cpp b/src/examples/persistent_volume_framework.cpp
index 1f8f4be..674d58a 100644
--- a/src/examples/persistent_volume_framework.cpp
+++ b/src/examples/persistent_volume_framework.cpp
@@ -239,9 +239,8 @@ public:
               operations.push_back(CREATE(volume));
               operations.push_back(LAUNCH({task}));
 
-              resources = offered.apply(vector<Offer::Operation>{
-                  CREATE(volume),
-                  LAUNCH({task})});
+              resources = offered.apply(
+                  vector<Offer::Operation>{CREATE(volume)});
 
               CHECK_SOME(resources);
               offered = resources.get();

http://git-wip-us.apache.org/repos/asf/mesos/blob/e5823186/src/master/allocator/mesos/hierarchical.cpp
----------------------------------------------------------------------
diff --git a/src/master/allocator/mesos/hierarchical.cpp b/src/master/allocator/mesos/hierarchical.cpp
index 5b6efe5..848e9da 100644
--- a/src/master/allocator/mesos/hierarchical.cpp
+++ b/src/master/allocator/mesos/hierarchical.cpp
@@ -778,11 +778,32 @@ void HierarchicalAllocatorProcess::updateAllocation(
     //  }
 
     // Update the offered resources based on this operation.
-    Try<Resources> _updatedOfferedResources = updatedOfferedResources.apply(
-        operation);
-
-    CHECK_SOME(_updatedOfferedResources);
-    updatedOfferedResources = _updatedOfferedResources.get();
+    switch (operation.type()) {
+      case Offer::Operation::LAUNCH:
+      case Offer::Operation::LAUNCH_GROUP:
+        // No need to apply LAUNCH and LAUNCH_GROUP.
+        break;
+      case Offer::Operation::RESERVE:
+      case Offer::Operation::UNRESERVE:
+      case Offer::Operation::CREATE:
+      case Offer::Operation::DESTROY: {
+        Try<Resources> _updatedOfferedResources =
+          updatedOfferedResources.apply(operation);
+
+        CHECK_SOME(_updatedOfferedResources);
+        updatedOfferedResources = _updatedOfferedResources.get();
+        break;
+      }
+      case Offer::Operation::CREATE_VOLUME:
+      case Offer::Operation::DESTROY_VOLUME:
+      case Offer::Operation::CREATE_BLOCK:
+      case Offer::Operation::DESTROY_BLOCK:
+        // TODO(jieyu): Add implementations here.
+        break;
+      case Offer::Operation::UNKNOWN:
+        UNREACHABLE();
+        break;
+    }
 
     if (operation.type() == Offer::Operation::LAUNCH) {
       foreach (const TaskInfo& task, operation.launch().task_infos()) {
@@ -862,13 +883,35 @@ void HierarchicalAllocatorProcess::updateAllocation(
 
   // We strip `AllocationInfo` from operations in order to apply them
   // successfully, since agent's total is stored as unallocated resources.
-  vector<Offer::Operation> strippedOperations = operations;
-  foreach (Offer::Operation& operation, strippedOperations) {
-    protobuf::stripAllocationInfo(&operation);
+  vector<Offer::Operation> strippedOperations;
+  foreach (Offer::Operation operation, operations) {
+    switch (operation.type()) {
+      case Offer::Operation::LAUNCH:
+      case Offer::Operation::LAUNCH_GROUP:
+        // No need to apply LAUNCH and LAUNCH_GROUP.
+        break;
+      case Offer::Operation::RESERVE:
+      case Offer::Operation::UNRESERVE:
+      case Offer::Operation::CREATE:
+      case Offer::Operation::DESTROY:
+        protobuf::stripAllocationInfo(&operation);
+        strippedOperations.push_back(operation);
+        break;
+      case Offer::Operation::CREATE_VOLUME:
+      case Offer::Operation::DESTROY_VOLUME:
+      case Offer::Operation::CREATE_BLOCK:
+      case Offer::Operation::DESTROY_BLOCK:
+        // TODO(jieyu): Add implementations here.
+        break;
+      case Offer::Operation::UNKNOWN:
+        UNREACHABLE();
+        break;
+    }
   }
 
   Try<Resources> updatedTotal = slave.total.apply(strippedOperations);
   CHECK_SOME(updatedTotal);
+
   updateSlaveTotal(slaveId, updatedTotal.get());
 
   // Update the total resources in the framework sorter.

http://git-wip-us.apache.org/repos/asf/mesos/blob/e5823186/src/v1/resources.cpp
----------------------------------------------------------------------
diff --git a/src/v1/resources.cpp b/src/v1/resources.cpp
index 3ef4941..58568b8 100644
--- a/src/v1/resources.cpp
+++ b/src/v1/resources.cpp
@@ -1654,20 +1654,25 @@ Option<Resources> Resources::find(const Resources& targets)
const
 }
 
 
-Try<Resources> Resources::apply(const Offer::Operation& operation) const
+Try<Resources> Resources::apply(
+    const Offer::Operation& operation,
+    const Option<Resources>& convertedResources) const
 {
   Resources result = *this;
 
   switch (operation.type()) {
     case Offer::Operation::LAUNCH:
-      // Launch operation does not alter the offered resources.
-      break;
+      return Error("Cannot apply LAUNCH Operation");
 
     case Offer::Operation::LAUNCH_GROUP:
-      // LaunchGroup operation does not alter the offered resources.
-      break;
+      return Error("Cannot apply LAUNCH_GROUP Operation");
 
     case Offer::Operation::RESERVE: {
+      if (convertedResources.isSome()) {
+        return Error(
+            "Converted resources not expected for RESERVE Operation");
+      }
+
       Option<Error> error = validate(operation.reserve().resources());
       if (error.isSome()) {
         return Error("Invalid RESERVE Operation: " + error->message);
@@ -1696,6 +1701,11 @@ Try<Resources> Resources::apply(const Offer::Operation& operation)
const
     }
 
     case Offer::Operation::UNRESERVE: {
+      if (convertedResources.isSome()) {
+        return Error(
+            "Converted resources not expected for UNRESERVE Operation");
+      }
+
       Option<Error> error = validate(operation.unreserve().resources());
       if (error.isSome()) {
         return Error("Invalid UNRESERVE Operation: " + error->message);
@@ -1724,6 +1734,11 @@ Try<Resources> Resources::apply(const Offer::Operation& operation)
const
     }
 
     case Offer::Operation::CREATE: {
+      if (convertedResources.isSome()) {
+        return Error(
+            "Converted resources not expected for CREATE Operation");
+      }
+
       Option<Error> error = validate(operation.create().volumes());
       if (error.isSome()) {
         return Error("Invalid CREATE Operation: " + error->message);
@@ -1767,6 +1782,11 @@ Try<Resources> Resources::apply(const Offer::Operation& operation)
const
     }
 
     case Offer::Operation::DESTROY: {
+      if (convertedResources.isSome()) {
+        return Error(
+            "Converted resources not expected for DESTROY Operation");
+      }
+
       Option<Error> error = validate(operation.destroy().volumes());
       if (error.isSome()) {
         return Error("Invalid DESTROY Operation: " + error->message);
@@ -1814,22 +1834,78 @@ Try<Resources> Resources::apply(const Offer::Operation&
operation) const
     }
 
     case Offer::Operation::CREATE_VOLUME: {
-      // CreateVolume operation does not alter the offered resources.
+      if (convertedResources.isNone()) {
+        return Error(
+            "Converted resources not specified for CREATE_VOLUME Operation");
+      }
+
+      const Resource& consumed = operation.create_volume().source();
+
+      if (!result.contains(consumed)) {
+        return Error(
+            "Invalid CREATE_VOLUME Operation: " + stringify(result) +
+            " does not contain " + stringify(consumed));
+      }
+
+      result.subtract(consumed);
+      result += convertedResources.get();
       break;
     }
 
     case Offer::Operation::DESTROY_VOLUME: {
-      // DestroyVolume operation does not alter the offered resources.
+      if (convertedResources.isNone()) {
+        return Error(
+            "Converted resources not specified for DESTROY_VOLUME Operation");
+      }
+
+      const Resource& consumed = operation.destroy_volume().volume();
+
+      if (!result.contains(consumed)) {
+        return Error(
+            "Invalid DESTROY_VOLUME Operation: " + stringify(result) +
+            " does not contain " + stringify(consumed));
+      }
+
+      result.subtract(consumed);
+      result += convertedResources.get();
       break;
     }
 
     case Offer::Operation::CREATE_BLOCK: {
-      // CreateBlock operation does not alter the offered resources.
+      if (convertedResources.isNone()) {
+        return Error(
+            "Converted resources not specified for CREATE_BLOCK Operation");
+      }
+
+      const Resource& consumed = operation.create_block().source();
+
+      if (!result.contains(consumed)) {
+        return Error(
+            "Invalid CREATE_BLOCK Operation: " + stringify(result) +
+            " does not contain " + stringify(consumed));
+      }
+
+      result.subtract(consumed);
+      result += convertedResources.get();
       break;
     }
 
     case Offer::Operation::DESTROY_BLOCK: {
-      // DestroyBlock operation does not alter the offered resources.
+      if (convertedResources.isNone()) {
+        return Error(
+            "Converted resources not specified for DESTROY_BLOCK Operation");
+      }
+
+      const Resource& consumed = operation.destroy_block().block();
+
+      if (!result.contains(consumed)) {
+        return Error(
+            "Invalid DESTROY_BLOCK Operation: " + stringify(result) +
+            " does not contain " + stringify(consumed));
+      }
+
+      result.subtract(consumed);
+      result += convertedResources.get();
       break;
     }
 


Mime
View raw message