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 2018/05/03 00:01:01 UTC

[05/20] mesos git commit: Tracked resource reservations in the allocator.

Tracked resource reservations in the allocator.

Resource reservations need to be tracked to make
sure quota limit will not be exceeded in the presence
of resource reservations.

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


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

Branch: refs/heads/1.4.x
Commit: 35b0e1cf3da823ca5154989a283e06a37ba5e462
Parents: 69b3aa0
Author: Meng Zhu <mz...@mesosphere.io>
Authored: Mon Dec 11 16:45:49 2017 -0800
Committer: Benjamin Mahler <bm...@apache.org>
Committed: Wed May 2 16:43:35 2018 -0700

----------------------------------------------------------------------
 src/master/allocator/mesos/hierarchical.cpp | 48 ++++++++++++++++++++++++
 src/master/allocator/mesos/hierarchical.hpp | 28 ++++++++++++++
 2 files changed, 76 insertions(+)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/35b0e1cf/src/master/allocator/mesos/hierarchical.cpp
----------------------------------------------------------------------
diff --git a/src/master/allocator/mesos/hierarchical.cpp b/src/master/allocator/mesos/hierarchical.cpp
index b6db12e..b70868d 100644
--- a/src/master/allocator/mesos/hierarchical.cpp
+++ b/src/master/allocator/mesos/hierarchical.cpp
@@ -518,6 +518,8 @@ void HierarchicalAllocatorProcess::addSlave(
     slave.maintenance = Slave::Maintenance(unavailability.get());
   }
 
+  trackReservations(total.reservations());
+
   roleSorter->add(slaveId, total);
 
   // See comment at `quotaRoleSorter` declaration regarding non-revocable.
@@ -590,6 +592,8 @@ void HierarchicalAllocatorProcess::removeSlave(
   // See comment at `quotaRoleSorter` declaration regarding non-revocable.
   quotaRoleSorter->remove(slaveId, slaves.at(slaveId).total.nonRevocable());
 
+  untrackReservations(slaves.at(slaveId).total.reservations());
+
   slaves.erase(slaveId);
   allocationCandidates.erase(slaveId);
 
@@ -2303,6 +2307,42 @@ void HierarchicalAllocatorProcess::untrackFrameworkUnderRole(
 }
 
 
+void HierarchicalAllocatorProcess::trackReservations(
+    const hashmap<std::string, Resources>& reservations)
+{
+  foreachpair (const string& role,
+               const Resources& resources, reservations) {
+    // We remove the static reservation metadata here via `toUnreserved()`.
+    const Resources scalarQuantitesToTrack =
+        resources.createStrippedScalarQuantity().toUnreserved();
+
+    reservationScalarQuantities[role] += scalarQuantitesToTrack;
+  }
+}
+
+
+void HierarchicalAllocatorProcess::untrackReservations(
+    const hashmap<std::string, Resources>& reservations)
+{
+  foreachpair (const string& role,
+               const Resources& resources, reservations) {
+    CHECK(reservationScalarQuantities.contains(role));
+    Resources& currentReservationQuantity =
+        reservationScalarQuantities.at(role);
+
+    // We remove the static reservation metadata here via `toUnreserved()`.
+    const Resources scalarQuantitesToUntrack =
+        resources.createStrippedScalarQuantity().toUnreserved();
+    CHECK(currentReservationQuantity.contains(scalarQuantitesToUntrack));
+    currentReservationQuantity -= scalarQuantitesToUntrack;
+
+    if (currentReservationQuantity.empty()) {
+      reservationScalarQuantities.erase(role);
+    }
+  }
+}
+
+
 bool HierarchicalAllocatorProcess::updateSlaveTotal(
     const SlaveID& slaveId,
     const Resources& total)
@@ -2319,6 +2359,14 @@ bool HierarchicalAllocatorProcess::updateSlaveTotal(
 
   slave.total = total;
 
+  hashmap<std::string, Resources> oldReservations = oldTotal.reservations();
+  hashmap<std::string, Resources> newReservations = total.reservations();
+
+  if (oldReservations != newReservations) {
+    untrackReservations(oldReservations);
+    trackReservations(newReservations);
+  }
+
   // Currently `roleSorter` and `quotaRoleSorter`, being the root-level
   // sorters, maintain all of `slaves[slaveId].total` (or the `nonRevocable()`
   // portion in the case of `quotaRoleSorter`) in their own totals (which

http://git-wip-us.apache.org/repos/asf/mesos/blob/35b0e1cf/src/master/allocator/mesos/hierarchical.hpp
----------------------------------------------------------------------
diff --git a/src/master/allocator/mesos/hierarchical.hpp b/src/master/allocator/mesos/hierarchical.hpp
index 0d30179..3f15b2c 100644
--- a/src/master/allocator/mesos/hierarchical.hpp
+++ b/src/master/allocator/mesos/hierarchical.hpp
@@ -442,6 +442,14 @@ protected:
   // change in the future.
   hashmap<std::string, Quota> quotas;
 
+  // Aggregated resource reservations on all agents tied to a
+  // particular role, if any. These are stripped scalar quantities
+  // that contain no meta-data. Used for accounting resource
+  // reservations for quota limit.
+  //
+  // Only roles with non-empty reservations will be stored in the map.
+  hashmap<std::string, Resources> reservationScalarQuantities;
+
   // Slaves to send offers for.
   Option<hashset<std::string>> whitelist;
 
@@ -529,6 +537,26 @@ private:
       const FrameworkID& frameworkId,
       const std::string& role);
 
+  // `trackReservations` and `untrackReservations` are helpers
+  // to track role resource reservations. We need to keep
+  // track of reservations to enforce role quota limit
+  // in the presence of unallocated reservations. See MESOS-4527.
+  //
+  // TODO(mzhu): Ideally, we want these helpers to instead track the
+  // reservations as *allocated* in the sorters even when the
+  // reservations have not been allocated yet. This will help to:
+  //
+  //   (1) Solve the fairness issue when roles with unallocated
+  //       reservations may game the allocator (See MESOS-8299).
+  //
+  //   (2) Simplify the quota enforcement logic -- the allocator
+  //       would no longer need to track reservations separately.
+  void trackReservations(
+      const hashmap<std::string, Resources>& reservations);
+
+  void untrackReservations(
+      const hashmap<std::string, Resources>& reservations);
+
   // Helper to update the agent's total resources maintained in the allocator
   // and the role and quota sorters (whose total resources match the agent's
   // total resources). Returns true iff the stored agent total was changed.