[mesos] 01/05: Made `allocated` mean "allocated but not offered" in `allocator::Slave`.

[mz...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

mzhu pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mesos.git

commit ebe4a4a4985ba2205bf9f242a57b56a72d0f013f
Author: Andrei Sekretenko <as...@mesosphere.io>
AuthorDate: Tue Sep 3 10:26:49 2019 -0700

    Made `allocated` mean "allocated but not offered" in `allocator::Slave`.
    
    This patch makes the `allocator::Slave` code more consistent
    with naming of different subsets of resources in the master
    (where "resource belongs to `offered`" is equivalent to "there is an
    offer with this resource", "resource belongs to `allocated`"
    is equivalent to "there is a task/executor with this resource" and
    `offered` has no intersection with `allocated`).
    
    Review: https://reviews.apache.org/r/71398/
---
 src/master/allocator/mesos/hierarchical.cpp | 28 +++++++++++++++-------------
 src/master/allocator/mesos/hierarchical.hpp | 27 ++++++++++++++-------------
 2 files changed, 29 insertions(+), 26 deletions(-)

diff --git a/src/master/allocator/mesos/hierarchical.cpp b/src/master/allocator/mesos/hierarchical.cpp
index dd73d5b..ed965b1 100644
--- a/src/master/allocator/mesos/hierarchical.cpp
+++ b/src/master/allocator/mesos/hierarchical.cpp
@@ -851,9 +851,10 @@ void HierarchicalAllocatorProcess::addSlave(
     resume();
   }
 
-  LOG(INFO) << "Added agent " << slaveId << " (" << slave.info.hostname() << ")"
-            << " with " << slave.getTotal()
-            << " (allocated: " << slave.getAllocated() << ")";
+  LOG(INFO)
+    << "Added agent " << slaveId << " (" << slave.info.hostname() << ")"
+    << " with " << slave.getTotal()
+    << " (offered or allocated: " << slave.getOfferedOrAllocated() << ")";
 
   allocate(slaveId);
 }
@@ -986,7 +987,7 @@ void HierarchicalAllocatorProcess::addResourceProvider(
 
   Slave& slave = *CHECK_NOTNONE(getSlave(slaveId));
   updateSlaveTotal(slaveId, slave.getTotal() + total);
-  slave.allocate(Resources::sum(used));
+  slave.decreaseAvailable(Resources::sum(used));
 
   VLOG(1)
     << "Grew agent " << slaveId << " by "
@@ -1113,8 +1114,8 @@ void HierarchicalAllocatorProcess::updateAllocation(
   const Resources& updatedOfferedResources = _updatedOfferedResources.get();
 
   // Update the per-slave allocation.
-  slave.unallocate(offeredResources);
-  slave.allocate(updatedOfferedResources);
+  slave.increaseAvailable(offeredResources);
+  slave.decreaseAvailable(updatedOfferedResources);
 
   // Update the allocation in the framework sorter.
   frameworkSorter->update(
@@ -1441,15 +1442,16 @@ void HierarchicalAllocatorProcess::recoverResources(
   Option<Slave*> slave = getSlave(slaveId);
 
   if (slave.isSome()) {
-    CHECK((*slave)->getAllocated().contains(resources))
+    CHECK((*slave)->getOfferedOrAllocated().contains(resources))
       << "agent " << slaveId << " resources "
-      << (*slave)->getAllocated() << " do not contain " << resources;
+      << (*slave)->getOfferedOrAllocated() << " do not contain " << resources;
 
-    (*slave)->unallocate(resources);
+    (*slave)->increaseAvailable(resources);
 
     VLOG(1) << "Recovered " << resources
             << " (total: " << (*slave)->getTotal()
-            << ", allocated: " << (*slave)->getAllocated() << ")"
+            << ", offered or allocated: "
+            << (*slave)->getOfferedOrAllocated() << ")"
             << " on agent " << slaveId
             << " from framework " << frameworkId;
   }
@@ -2166,7 +2168,7 @@ void HierarchicalAllocatorProcess::__allocate()
           ResourceQuantities::fromScalarResources(guaranteesAllocation);
         availableHeadroom -= increasedQuotaConsumption;
 
-        slave.allocate(toAllocate);
+        slave.decreaseAvailable(toAllocate);
 
         trackAllocatedResources(slaveId, frameworkId, toAllocate);
       }
@@ -2313,7 +2315,7 @@ void HierarchicalAllocatorProcess::__allocate()
 
         availableHeadroom -= increasedQuotaConsumption;
 
-        slave.allocate(toAllocate);
+        slave.decreaseAvailable(toAllocate);
 
         trackAllocatedResources(slaveId, frameworkId, toAllocate);
       }
@@ -2652,7 +2654,7 @@ double HierarchicalAllocatorProcess::_resources_offered_or_allocated(
 
   foreachvalue (const Slave& slave, slaves) {
     Option<Value::Scalar> value =
-      slave.getAllocated().get<Value::Scalar>(resource);
+      slave.getOfferedOrAllocated().get<Value::Scalar>(resource);
 
     if (value.isSome()) {
       offered_or_allocated += value->value();
diff --git a/src/master/allocator/mesos/hierarchical.hpp b/src/master/allocator/mesos/hierarchical.hpp
index 65d103e..d466cbf 100644
--- a/src/master/allocator/mesos/hierarchical.hpp
+++ b/src/master/allocator/mesos/hierarchical.hpp
@@ -253,12 +253,12 @@ public:
       const protobuf::slave::Capabilities& _capabilities,
       bool _activated,
       const Resources& _total,
-      const Resources& _allocated)
+      const Resources& _offeredOrAllocated)
     : info(_info),
       capabilities(_capabilities),
       activated(_activated),
       total(_total),
-      allocated(_allocated),
+      offeredOrAllocated(_offeredOrAllocated),
       shared(_total.shared()),
       hasGpu_(_total.gpus().getOrElse(0) > 0)
   {
@@ -267,7 +267,7 @@ public:
 
   const Resources& getTotal() const { return total; }
 
-  const Resources& getAllocated() const { return allocated; }
+  const Resources& getOfferedOrAllocated() const { return offeredOrAllocated; }
 
   const Resources& getAvailable() const { return available; }
 
@@ -281,16 +281,16 @@ public:
     updateAvailable();
   }
 
-  void allocate(const Resources& toAllocate)
+  void decreaseAvailable(const Resources& offeredOrAllocated_)
   {
-    allocated += toAllocate;
+    offeredOrAllocated += offeredOrAllocated_;
 
     updateAvailable();
   }
 
-  void unallocate(const Resources& toUnallocate)
+  void increaseAvailable(const Resources& offeredOrAllocated_)
   {
-    allocated -= toUnallocate;
+    offeredOrAllocated -= offeredOrAllocated_;
 
     updateAvailable();
   }
@@ -345,8 +345,8 @@ private:
   void updateAvailable() {
     // In order to subtract from the total,
     // we strip the allocation information.
-    Resources allocated_ = allocated;
-    allocated_.unallocate();
+    Resources offeredOrAllocated_ = offeredOrAllocated;
+    offeredOrAllocated_.unallocate();
 
     // Calling `nonShared()` currently copies the underlying resources
     // and is therefore rather expensive. We avoid it in the common
@@ -357,18 +357,19 @@ private:
     // `nonShared()` performs no copying and instead points to a
     // subset of the original `Resource` objects.
     if (shared.empty()) {
-      available = total - allocated_;
+      available = total - offeredOrAllocated_;
     } else {
       // Since shared resources are offerable even when they are in use, we
       // always include them as part of available resources.
-      available = (total.nonShared() - allocated_.nonShared()) + shared;
+      available =
+        (total.nonShared() - offeredOrAllocated_.nonShared()) + shared;
     }
   }
 
   // Total amount of regular *and* oversubscribed resources.
   Resources total;
 
-  // Regular *and* oversubscribed resources that are allocated.
+  // Regular *and* oversubscribed resources that are offered or allocated.
   //
   // NOTE: We maintain multiple copies of each shared resource allocated
   // to a slave, where the number of copies represents the number of times
@@ -379,7 +380,7 @@ private:
   // having that information in sorters. This is because the
   // information in sorters is not accurate if some framework
   // hasn't reregistered. See MESOS-2919 for details.
-  Resources allocated;
+  Resources offeredOrAllocated;
 
   // We track the total and allocated resources on the slave to
   // avoid calculating it in place every time.