You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by mz...@apache.org on 2019/09/12 22:11:43 UTC

[mesos] 04/04: Tracked offered and allocated resources in the role tree.

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 783fd45c548fdff0c5c4812bc8e92c3aed202e06
Author: Meng Zhu <mz...@mesosphere.io>
AuthorDate: Sat Sep 7 16:01:51 2019 -0700

    Tracked offered and allocated resources in the role tree.
    
    This helpers simplify the quota tracking logic and also paves
    the way to reduce duplicated states in the sorter.
    
    Also documented that shared resources must be uniquely
    identifiable.
    
    Small performance degradation when making allocations due to
    duplicated map construction in `(un)trackAllocatedResources`.
    This will be removed once embeded the sorter in the role tree.
    
    Benchmark `LargeAndSmallQuota/2`:
    
    Master:
    
    Added 3000 agents in 80.648188ms
    Added 3000 frameworks in 19.7006984secs
    Benchmark setup: 3000 agents, 3000 roles, 3000 frameworks,
    with drf sorter
    Made 3500 allocations in 16.044274434secs
    Made 0 allocation in 14.476429451secs
    
    Master + this patch:
    Added 3000 agents in 80.110817ms
    Added 3000 frameworks in 17.25974094secs
    Benchmark setup: 3000 agents, 3000 roles, 3000 frameworks,
    with drf sorter
    Made 3500 allocations in 16.91971379secs
    Made 0 allocation in 13.784476154secs
    
    Review: https://reviews.apache.org/r/71460
---
 include/mesos/mesos.proto                   |   4 +
 include/mesos/v1/mesos.proto                |   4 +
 src/master/allocator/mesos/hierarchical.cpp | 125 +++++++++++++++++-----------
 src/master/allocator/mesos/hierarchical.hpp |  22 +++++
 4 files changed, 106 insertions(+), 49 deletions(-)

diff --git a/include/mesos/mesos.proto b/include/mesos/mesos.proto
index 8fd838e..0cab154 100644
--- a/include/mesos/mesos.proto
+++ b/include/mesos/mesos.proto
@@ -1565,6 +1565,10 @@ message Resource {
   // can be launched using this resource and all of them shall refer
   // to the same physical resource on the cluster. Note that only
   // persistent volumes can be shared currently.
+  //
+  // NOTE: Different shared resources must be uniquely identifiable.
+  // This currently holds as persistent volume should have unique `id`
+  // (this is not validated for enforced though).
   optional SharedInfo shared = 10;
 }
 
diff --git a/include/mesos/v1/mesos.proto b/include/mesos/v1/mesos.proto
index da19256..a2f6dbb 100644
--- a/include/mesos/v1/mesos.proto
+++ b/include/mesos/v1/mesos.proto
@@ -1553,6 +1553,10 @@ message Resource {
   // can be launched using this resource and all of them shall refer
   // to the same physical resource on the cluster. Note that only
   // persistent volumes can be shared currently.
+  //
+  // NOTE: Different shared resources must be uniquely identifiable.
+  // This currently holds as persistent volume should have unique `id`
+  // (this is not validated for enforced though).
   optional SharedInfo shared = 10;
 }
 
diff --git a/src/master/allocator/mesos/hierarchical.cpp b/src/master/allocator/mesos/hierarchical.cpp
index 8432a62..256fdce 100644
--- a/src/master/allocator/mesos/hierarchical.cpp
+++ b/src/master/allocator/mesos/hierarchical.cpp
@@ -249,6 +249,18 @@ Option<const Role*> RoleTree::get(const std::string& role) const
 }
 
 
+Option<Role*> RoleTree::get_(const std::string& role)
+{
+  auto found = roles_.find(role);
+
+  if (found == roles_.end()) {
+    return None();
+  } else {
+    return &(found->second);
+  }
+}
+
+
 Role& RoleTree::operator[](const std::string& rolePath)
 {
   if (roles_.contains(rolePath)) {
@@ -311,6 +323,9 @@ bool RoleTree::tryRemove(const std::string& role)
       (*metrics)->removeRole(current->role);
     }
 
+    CHECK(current->offeredOrAllocatedScalars_.empty())
+      << " role: " << role
+      << " offeredOrAllocated: " << current->offeredOrAllocatedScalars_;
     roles_.erase(current->role);
 
     current = parent;
@@ -401,6 +416,47 @@ void RoleTree::updateWeight(const string& role, double weight)
 }
 
 
+void RoleTree::trackOfferedOrAllocated(const Resources& resources_)
+{
+  // TODO(mzhu): avoid building a map by traversing `resources`
+  // and look for the allocation role of individual resource.
+  // However, due to MESOS-9242, this currently does not work
+  // as traversing resources would lose the shared count.
+  foreachpair (
+      const string& role,
+      const Resources& resources,
+      resources_.scalars().allocations()) {
+    // Track it hierarchically up to the root.
+    for (Role* current = CHECK_NOTNONE(get_(role)); current != nullptr;
+         current = current->parent) {
+      current->offeredOrAllocatedScalars_ += resources;
+    }
+  }
+}
+
+
+void RoleTree::untrackOfferedOrAllocated(const Resources& resources_)
+{
+  // TODO(mzhu): avoid building a map by traversing `resources`
+  // and look for the allocation role of individual resource.
+  // However, due to MESOS-9242, this currently does not work
+  // as traversing resources would lose the shared count.
+  foreachpair (
+      const string& role,
+      const Resources& resources,
+      resources_.scalars().allocations()) {
+    // Untrack it hierarchically up to the root.
+    for (Role* current = CHECK_NOTNONE(get_(role)); current != nullptr;
+         current = current->parent) {
+      CHECK_CONTAINS(current->offeredOrAllocatedScalars_, resources)
+        << " Role: " << current->role
+        << " offeredOrAllocated: " << current->offeredOrAllocatedScalars_;
+      current->offeredOrAllocatedScalars_ -= resources;
+    }
+  }
+}
+
+
 std::string RoleTree::toJSON() const
 {
   std::function<void(JSON::ObjectWriter*, const Role*)> json =
@@ -412,6 +468,8 @@ std::string RoleTree::toJSON() const
       writer->field("limits", role->quota_.limits);
       writer->field(
           "reservation_quantities", role->reservationScalarQuantities_);
+      writer->field(
+          "offered_or_allocated_scalars", role->offeredOrAllocatedScalars_);
 
       writer->field("frameworks", [&](JSON::ArrayWriter* writer) {
         foreach (const FrameworkID& id, role->frameworks_) {
@@ -1108,15 +1166,16 @@ void HierarchicalAllocatorProcess::updateAllocation(
   //  foreach (const ResourceConversion& conversion, conversions) {
   //    CHECK_NONE(validateConversionOnAllocatedResources(conversion));
   //  }
-  Try<Resources> _updatedOfferedResources = offeredResources.apply(conversions);
-  CHECK_SOME(_updatedOfferedResources);
-
-  const Resources& updatedOfferedResources = _updatedOfferedResources.get();
+  Resources updatedOfferedResources =
+    CHECK_NOTERROR(offeredResources.apply(conversions));
 
   // Update the per-slave allocation.
   slave.increaseAvailable(frameworkId, offeredResources);
   slave.decreaseAvailable(frameworkId, updatedOfferedResources);
 
+  roleTree.untrackOfferedOrAllocated(offeredResources);
+  roleTree.trackOfferedOrAllocated(updatedOfferedResources);
+
   // Update the allocation in the framework sorter.
   frameworkSorter->update(
       frameworkId.value(),
@@ -1804,7 +1863,7 @@ void HierarchicalAllocatorProcess::__generateOffers()
   //
   //   Consumed Quota = reservations + unreserved allocation
 
-  // First add reservations.
+  // Add reservations and unreserved offeredOrAllocated.
   //
   // Currently, only top level roles can have quota set and thus
   // we only track consumed quota for top level roles.
@@ -1813,34 +1872,10 @@ void HierarchicalAllocatorProcess::__generateOffers()
     // these as metrics.
     if (r->quota() != DEFAULT_QUOTA) {
       logHeadroomInfo = true;
-      // Note, `reservationScalarQuantities` in `struct role`
-      // is hierarchical aware, thus it also includes subrole reservations.
-      rolesConsumedQuota[r->role] += r->reservationScalarQuantities();
-    }
-  }
-
-  // Then add the unreserved allocation.
-  //
-  // TODO(mzhu): make allocation tracking hierarchical, so that we only
-  // need to look at the top-level node.
-  foreachpair (const string& role, const Role& r, roleTree.roles()) {
-    if (r.frameworks().empty()) {
-      continue;
-    }
-
-    const string& topLevelRole =
-      strings::contains(role, "/") ? role.substr(0, role.find('/')) : role;
-
-    if (getQuota(topLevelRole) == DEFAULT_QUOTA) {
-      continue;
-    }
-
-    if (roleSorter->contains(role)) {
-      foreachvalue (const Resources& resources, roleSorter->allocation(role)) {
-        rolesConsumedQuota[topLevelRole] +=
-          ResourceQuantities::fromScalarResources(
-              resources.unreserved().nonRevocable().scalars());
-      }
+      rolesConsumedQuota[r->role] +=
+        r->reservationScalarQuantities() +
+        ResourceQuantities::fromScalarResources(
+            r->offeredOrAllocatedScalars().unreserved().nonRevocable());
     }
   }
 
@@ -1882,24 +1917,12 @@ void HierarchicalAllocatorProcess::__generateOffers()
   // Subtract allocated resources from the total.
   availableHeadroom -= roleSorter->allocationScalarQuantities();
 
-  // TODO(mzhu): make allocation tracking hierarchical, so that we only
-  // need to look at the top-level node.
-  ResourceQuantities totalOfferedOrAllocatedReservation;
-  foreachkey (const string& role, roleTree.roles()) {
-    if (!roleSorter->contains(role)) {
-      continue; // This role has no allocation.
-    }
-
-    foreachvalue (const Resources& resources, roleSorter->allocation(role)) {
-      totalOfferedOrAllocatedReservation +=
-        ResourceQuantities::fromScalarResources(resources.reserved().scalars());
-    }
-  }
-
   // Subtract total unallocated reservations.
   // unallocated reservations = total reservations - allocated reservations
-  availableHeadroom -= roleTree.root()->reservationScalarQuantities() -
-                       totalOfferedOrAllocatedReservation;
+  availableHeadroom -=
+    roleTree.root()->reservationScalarQuantities() -
+    ResourceQuantities::fromScalarResources(
+        roleTree.root()->offeredOrAllocatedScalars().reserved());
 
   // Subtract revocable resources.
   foreachvalue (const Slave& slave, slaves) {
@@ -2948,6 +2971,8 @@ void HierarchicalAllocatorProcess::trackAllocatedResources(
     CHECK_CONTAINS(*frameworkSorter, frameworkId.value())
       << " for role " << role;
 
+    roleTree.trackOfferedOrAllocated(allocation);
+
     roleSorter->allocated(role, slaveId, allocation);
     frameworkSorter->allocated(
         frameworkId.value(), slaveId, allocation);
@@ -2980,6 +3005,8 @@ void HierarchicalAllocatorProcess::untrackAllocatedResources(
     CHECK_CONTAINS(*frameworkSorter, frameworkId.value())
       << "for role " << role;
 
+    roleTree.untrackOfferedOrAllocated(allocation);
+
     frameworkSorter->unallocated(
         frameworkId.value(), slaveId, allocation);
 
diff --git a/src/master/allocator/mesos/hierarchical.hpp b/src/master/allocator/mesos/hierarchical.hpp
index 9e6570a..5ea3791 100644
--- a/src/master/allocator/mesos/hierarchical.hpp
+++ b/src/master/allocator/mesos/hierarchical.hpp
@@ -122,6 +122,11 @@ public:
     return reservationScalarQuantities_;
   }
 
+  const Resources& offeredOrAllocatedScalars() const
+  {
+    return offeredOrAllocatedScalars_;
+  }
+
   const hashset<FrameworkID>& frameworks() const { return frameworks_; }
 
   const Quota& quota() const { return quota_; }
@@ -166,6 +171,13 @@ private:
   // IDs of the frameworks subscribed to the role, if any.
   hashset<FrameworkID> frameworks_;
 
+  // Total allocated or offered scalar resources to this role, including
+  // meta data. This field dose not affect role's lifecycle. However, since
+  // any offered or allocated resources should be tied to a framework,
+  // an empty role (that has no registered framework) must have
+  // empty offeredOrAllocated resources.
+  Resources offeredOrAllocatedScalars_;
+
   // Aggregated reserved scalar resource quantities on all agents tied to this
   // role, if any. This includes both its own reservations as well as
   // reservations of any of its subroles (i.e. it is hierarchical aware).
@@ -217,10 +229,16 @@ public:
 
   void updateWeight(const std::string& role, double weight);
 
+  void trackOfferedOrAllocated(const Resources& resources);
+  void untrackOfferedOrAllocated(const Resources& resources);
+
   // Dump the role tree state in JSON format for debugging.
   std::string toJSON() const;
 
 private:
+  // Private helper to get non-const pointers.
+  Option<Role*> get_(const std::string& role);
+
   // Lookup or add the role struct associated with the role. Ancestor roles
   // along the tree path will be created if necessary.
   Role& operator[](const std::string& role);
@@ -798,6 +816,8 @@ private:
   //
   // TODO(asekretenko): rename `(un)trackAllocatedResources()` to reflect the
   // fact that these methods do not distinguish between offered and allocated.
+  //
+  // TODO(mzhu): replace this with `RoleTree::trackOfferedOrAllocated`.
   void trackAllocatedResources(
       const SlaveID& slaveId,
       const FrameworkID& frameworkId,
@@ -805,6 +825,8 @@ private:
 
   // Helper to untrack resources that are no longer offered or allocated
   // on an agent.
+  //
+  // TODO(mzhu): replace this with `RoleTree::untrackOfferedOrAllocated`.
   void untrackAllocatedResources(
       const SlaveID& slaveId,
       const FrameworkID& frameworkId,