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/02/04 03:02:07 UTC

[5/9] mesos git commit: Updated the master's HTTP operations to handle MULTI_ROLE changes.

Updated the master's HTTP operations to handle MULTI_ROLE changes.

With the addition of MULTI_ROLE framework support, allocated
resources in the master have `Resource.AllocationInfo` set. This
means that the master's http endpoints that apply operations or
perform checks between unallocated and allocated resources must
be updated to continue to function correctly.

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


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

Branch: refs/heads/master
Commit: 45786b69cd08ae8527204193de2747ba3a5b575e
Parents: a0a1f82
Author: Benjamin Mahler <bm...@apache.org>
Authored: Wed Jan 25 16:26:04 2017 -0800
Committer: Benjamin Mahler <bm...@apache.org>
Committed: Fri Feb 3 18:47:11 2017 -0800

----------------------------------------------------------------------
 src/master/http.cpp          | 13 ++++++++-----
 src/master/quota_handler.cpp |  8 +++++++-
 src/master/validation.cpp    | 30 ++++++++++++++++++++++++------
 3 files changed, 39 insertions(+), 12 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/45786b69/src/master/http.cpp
----------------------------------------------------------------------
diff --git a/src/master/http.cpp b/src/master/http.cpp
index a455f03..d881ad6 100644
--- a/src/master/http.cpp
+++ b/src/master/http.cpp
@@ -4649,7 +4649,7 @@ Future<Response> Master::Http::_operation(
   }
 
   // The resources recovered by rescinding outstanding offers.
-  Resources recovered;
+  Resources totalRecovered;
 
   // We pessimistically assume that what seems like "available"
   // resources in the allocator will be gone. This can happen due to
@@ -4660,12 +4660,15 @@ Future<Response> Master::Http::_operation(
   foreach (Offer* offer, utils::copy(slave->offers)) {
     // If rescinding the offer would not contribute to satisfying
     // the required resources, skip it.
-    if (required == required - offer->resources()) {
+    Resources recovered = offer->resources();
+    recovered.unallocate();
+
+    if (required == required - recovered) {
       continue;
     }
 
-    recovered += offer->resources();
-    required -= offer->resources();
+    totalRecovered += recovered;
+    required -= recovered;
 
     // We explicitly pass 'Filters()' which has a default 'refuse_sec'
     // of 5 seconds rather than 'None()' here, so that we can
@@ -4679,7 +4682,7 @@ Future<Response> Master::Http::_operation(
     master->removeOffer(offer, true); // Rescind!
 
     // If we've rescinded enough offers to cover 'operation', we're done.
-    Try<Resources> updatedRecovered = recovered.apply(operation);
+    Try<Resources> updatedRecovered = totalRecovered.apply(operation);
     if (updatedRecovered.isSome()) {
       break;
     }

http://git-wip-us.apache.org/repos/asf/mesos/blob/45786b69/src/master/quota_handler.cpp
----------------------------------------------------------------------
diff --git a/src/master/quota_handler.cpp b/src/master/quota_handler.cpp
index f4a27ea..3ad28e4 100644
--- a/src/master/quota_handler.cpp
+++ b/src/master/quota_handler.cpp
@@ -193,7 +193,13 @@ void Master::QuotaHandler::rescindOffers(const QuotaInfo& request) const
       master->allocator->recoverResources(
           offer->framework_id(), offer->slave_id(), offer->resources(), None());
 
-      rescinded += offer->resources();
+      auto unallocated = [](const Resources& resources) {
+        Resources result = resources;
+        result.unallocate();
+        return result;
+      };
+
+      rescinded += unallocated(offer->resources());
       master->removeOffer(offer, true);
       agentVisited = true;
     }

http://git-wip-us.apache.org/repos/asf/mesos/blob/45786b69/src/master/validation.cpp
----------------------------------------------------------------------
diff --git a/src/master/validation.cpp b/src/master/validation.cpp
index f45b644..2beee16 100644
--- a/src/master/validation.cpp
+++ b/src/master/validation.cpp
@@ -1714,17 +1714,35 @@ Option<Error> validate(
     const hashmap<FrameworkID, Resources>& usedResources,
     const hashmap<FrameworkID, hashmap<TaskID, TaskInfo>>& pendingTasks)
 {
-  Option<Error> error = resource::validate(destroy.volumes());
+  // The operation can either contain allocated resources
+  // (in the case of a framework accepting offers), or
+  // unallocated resources (in the case of the operator
+  // endpoints). To ensure we can check for the presence
+  // of the volume in the resources in use by tasks and
+  // executors, we unallocate both the volume and the
+  // used resources before performing the contains check.
+  //
+  // TODO(bmahler): This lambda is copied in several places
+  // in the code, consider how best to pull this out.
+  auto unallocated = [](const Resources& resources) {
+    Resources result = resources;
+    result.unallocate();
+    return result;
+  };
+
+  Resources volumes = unallocated(destroy.volumes());
+
+  Option<Error> error = resource::validate(volumes);
   if (error.isSome()) {
     return Error("Invalid resources: " + error.get().message);
   }
 
-  error = resource::validatePersistentVolume(destroy.volumes());
+  error = resource::validatePersistentVolume(volumes);
   if (error.isSome()) {
     return Error("Not a persistent volume: " + error.get().message);
   }
 
-  if (!checkpointedResources.contains(destroy.volumes())) {
+  if (!checkpointedResources.contains(volumes)) {
     return Error("Persistent volumes not found");
   }
 
@@ -1733,8 +1751,8 @@ Option<Error> validate(
   // it is not possible for a non-shared resource to appear in an offer
   // if it is already in use.
   foreachvalue (const Resources& resources, usedResources) {
-    foreach (const Resource& volume, destroy.volumes()) {
-      if (resources.contains(volume)) {
+    foreach (const Resource& volume, volumes) {
+      if (unallocated(resources).contains(volume)) {
         return Error("Persistent volumes in use");
       }
     }
@@ -1754,7 +1772,7 @@ Option<Error> validate(
       }
 
       foreach (const Resource& volume, destroy.volumes()) {
-        if (resources.contains(volume)) {
+        if (unallocated(resources).contains(volume)) {
           return Error("Persistent volume in pending tasks");
         }
       }