[mesos] 06/10: Use total cluster resources as framework sorter "total".

[bm...@apache.org]

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

bmahler pushed a commit to branch 1.7.x
in repository https://gitbox.apache.org/repos/asf/mesos.git

commit a7e339f547f6f01a28730caa6b44e42dec228fa3
Author: Benjamin Mahler <bm...@apache.org>
AuthorDate: Fri Sep 21 13:48:46 2018 -0700

    Use total cluster resources as framework sorter "total".
    
    The role sorter and framework sorters differed in that the role
    sorter used the total cluster resources as the "total" in which
    the allocation is divided to compute shares, whereas the framework
    sorters used the role's allocation as the "total".
    
    Per a discussion on the mailing list:
    
    https://s.apache.org/differing-DRF
    
    This patch makes the two types of sorters use the total cluster
    as the "total". While this provides slightly different behavior,
    it's unlikely any users were relying on this.
    
    Review: https://reviews.apache.org/r/68833
---
 src/master/allocator/mesos/hierarchical.cpp | 30 ++++++++++++++++++-----------
 1 file changed, 19 insertions(+), 11 deletions(-)

diff --git a/src/master/allocator/mesos/hierarchical.cpp b/src/master/allocator/mesos/hierarchical.cpp
index 0b13d04..fd9e819 100644
--- a/src/master/allocator/mesos/hierarchical.cpp
+++ b/src/master/allocator/mesos/hierarchical.cpp
@@ -553,6 +553,10 @@ void HierarchicalAllocatorProcess::addSlave(
 
   roleSorter->add(slaveId, total);
 
+  foreachvalue (const Owned<Sorter>& sorter, frameworkSorters) {
+    sorter->add(slaveId, total);
+  }
+
   // See comment at `quotaRoleSorter` declaration regarding non-revocable.
   quotaRoleSorter->add(slaveId, total.nonRevocable());
 
@@ -620,6 +624,10 @@ void HierarchicalAllocatorProcess::removeSlave(
 
   roleSorter->remove(slaveId, slaves.at(slaveId).getTotal());
 
+  foreachvalue (const Owned<Sorter>& sorter, frameworkSorters) {
+    sorter->remove(slaveId, slaves.at(slaveId).getTotal());
+  }
+
   // See comment at `quotaRoleSorter` declaration regarding non-revocable.
   quotaRoleSorter->remove(
       slaveId, slaves.at(slaveId).getTotal().nonRevocable());
@@ -945,10 +953,6 @@ void HierarchicalAllocatorProcess::updateAllocation(
 
   updateSlaveTotal(slaveId, updatedTotal.get());
 
-  // Update the total resources in the framework sorter.
-  frameworkSorter->remove(slaveId, offeredResources);
-  frameworkSorter->add(slaveId, updatedOfferedResources);
-
   const Resources updatedFrameworkAllocation =
     frameworkSorter->allocation(frameworkId.value(), slaveId);
 
@@ -2532,6 +2536,11 @@ void HierarchicalAllocatorProcess::trackFrameworkUnderRole(
     CHECK(!frameworkSorters.contains(role));
     frameworkSorters.insert({role, Owned<Sorter>(frameworkSorterFactory())});
     frameworkSorters.at(role)->initialize(fairnessExcludeResourceNames);
+
+    foreachvalue (const Slave& slave, slaves) {
+      frameworkSorters.at(role)->add(slave.info.id(), slave.getTotal());
+    }
+
     metrics.addRole(role);
   }
 
@@ -2637,14 +2646,15 @@ bool HierarchicalAllocatorProcess::updateSlaveTotal(
     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
-  // don't get updated in the allocation runs or during recovery of allocated
-  // resources). So, we update them using the resources in `slave.total`.
+  // Update the totals in the sorters.
   roleSorter->remove(slaveId, oldTotal);
   roleSorter->add(slaveId, total);
 
+  foreachvalue (const Owned<Sorter>& sorter, frameworkSorters) {
+    sorter->remove(slaveId, oldTotal);
+    sorter->add(slaveId, total);
+  }
+
   // See comment at `quotaRoleSorter` declaration regarding non-revocable.
   quotaRoleSorter->remove(slaveId, oldTotal.nonRevocable());
   quotaRoleSorter->add(slaveId, total.nonRevocable());
@@ -2768,7 +2778,6 @@ void HierarchicalAllocatorProcess::trackAllocatedResources(
     CHECK(frameworkSorters.at(role)->contains(frameworkId.value()));
 
     roleSorter->allocated(role, slaveId, allocation);
-    frameworkSorters.at(role)->add(slaveId, allocation);
     frameworkSorters.at(role)->allocated(
         frameworkId.value(), slaveId, allocation);
 
@@ -2804,7 +2813,6 @@ void HierarchicalAllocatorProcess::untrackAllocatedResources(
 
     frameworkSorters.at(role)->unallocated(
         frameworkId.value(), slaveId, allocation);
-    frameworkSorters.at(role)->remove(slaveId, allocation);
 
     roleSorter->unallocated(role, slaveId, allocation);