You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by ne...@apache.org on 2017/04/26 18:20:49 UTC

[03/11] mesos git commit: Avoided storing weights in the allocator.

Avoided storing weights in the allocator.

Previously, DRFSorter only kept track of weights for clients currently
in the sorter; the allocator supplied the current weight when adding a
client to the sorter.

This commit changes the sorter to keep track of all weights, not just
those for the active clients in the sorter. The allocator can now just
pass along role weights to the role sorters, rather than needing to
track them itself.

This commit changes the sorter API.

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


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

Branch: refs/heads/master
Commit: b7f70ca477f5fce84f33658b28f1cde17dbfd452
Parents: d93f224
Author: Neil Conway <ne...@gmail.com>
Authored: Fri Mar 10 14:43:24 2017 -0500
Committer: Neil Conway <ne...@gmail.com>
Committed: Wed Apr 26 14:01:52 2017 -0400

----------------------------------------------------------------------
 src/master/allocator/mesos/hierarchical.cpp | 25 +++-----------
 src/master/allocator/mesos/hierarchical.hpp |  7 ----
 src/master/allocator/sorter/drf/sorter.cpp  | 42 ++++++++++++++----------
 src/master/allocator/sorter/drf/sorter.hpp  | 10 ++++--
 src/master/allocator/sorter/sorter.hpp      | 14 +++++---
 src/tests/sorter_tests.cpp                  | 13 +++++---
 6 files changed, 52 insertions(+), 59 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/b7f70ca4/src/master/allocator/mesos/hierarchical.cpp
----------------------------------------------------------------------
diff --git a/src/master/allocator/mesos/hierarchical.cpp b/src/master/allocator/mesos/hierarchical.cpp
index 57a5e82..984a0a4 100644
--- a/src/master/allocator/mesos/hierarchical.cpp
+++ b/src/master/allocator/mesos/hierarchical.cpp
@@ -1276,7 +1276,7 @@ void HierarchicalAllocatorProcess::setQuota(
   // Persist quota in memory and add the role into the corresponding
   // allocation group.
   quotas[role] = quota;
-  quotaRoleSorter->add(role, roleWeight(role));
+  quotaRoleSorter->add(role);
 
   // Copy allocation information for the quota'ed role.
   if (roleSorter->contains(role)) {
@@ -1336,18 +1336,11 @@ void HierarchicalAllocatorProcess::updateWeights(
 {
   CHECK(initialized);
 
-  // Update the weight for each specified role.
   foreach (const WeightInfo& weightInfo, weightInfos) {
     CHECK(weightInfo.has_role());
-    weights[weightInfo.role()] = weightInfo.weight();
 
-    if (quotas.contains(weightInfo.role())) {
-      quotaRoleSorter->update(weightInfo.role(), weightInfo.weight());
-    }
-
-    if (roleSorter->contains(weightInfo.role())) {
-      roleSorter->update(weightInfo.role(), weightInfo.weight());
-    }
+    quotaRoleSorter->updateWeight(weightInfo.role(), weightInfo.weight());
+    roleSorter->updateWeight(weightInfo.role(), weightInfo.weight());
   }
 
   // NOTE: Since weight changes do not result in rebalancing of
@@ -2047,16 +2040,6 @@ void HierarchicalAllocatorProcess::expire(
 }
 
 
-double HierarchicalAllocatorProcess::roleWeight(const string& name) const
-{
-  if (weights.contains(name)) {
-    return weights.at(name);
-  } else {
-    return 1.0; // Default weight.
-  }
-}
-
-
 bool HierarchicalAllocatorProcess::isWhitelisted(
     const SlaveID& slaveId) const
 {
@@ -2235,7 +2218,7 @@ void HierarchicalAllocatorProcess::trackFrameworkUnderRole(
   if (!roles.contains(role)) {
     roles[role] = {};
     CHECK(!roleSorter->contains(role));
-    roleSorter->add(role, roleWeight(role));
+    roleSorter->add(role);
 
     CHECK(!frameworkSorters.contains(role));
     frameworkSorters.insert({role, Owned<Sorter>(frameworkSorterFactory())});

http://git-wip-us.apache.org/repos/asf/mesos/blob/b7f70ca4/src/master/allocator/mesos/hierarchical.hpp
----------------------------------------------------------------------
diff --git a/src/master/allocator/mesos/hierarchical.hpp b/src/master/allocator/mesos/hierarchical.hpp
index f84b057..219f508 100644
--- a/src/master/allocator/mesos/hierarchical.hpp
+++ b/src/master/allocator/mesos/hierarchical.hpp
@@ -256,9 +256,6 @@ protected:
       const SlaveID& slaveId,
       InverseOfferFilter* inverseOfferFilter);
 
-  // Returns the weight of the specified role name.
-  double roleWeight(const std::string& name) const;
-
   // Checks whether the slave is whitelisted.
   bool isWhitelisted(const SlaveID& slaveId) const;
 
@@ -432,10 +429,6 @@ protected:
   // (e.g. some tasks and/or executors are consuming resources under the role).
   hashmap<std::string, hashset<FrameworkID>> roles;
 
-  // Configured weight for each role, if any; if a role does not
-  // appear here, it has the default weight of 1.
-  hashmap<std::string, double> weights;
-
   // Configured quota for each role, if any. Setting quota for a role
   // changes the order that the role's frameworks are offered
   // resources. Quota comes before fair share, hence setting quota moves

http://git-wip-us.apache.org/repos/asf/mesos/blob/b7f70ca4/src/master/allocator/sorter/drf/sorter.cpp
----------------------------------------------------------------------
diff --git a/src/master/allocator/sorter/drf/sorter.cpp b/src/master/allocator/sorter/drf/sorter.cpp
index ed54680..d2952b8 100644
--- a/src/master/allocator/sorter/drf/sorter.cpp
+++ b/src/master/allocator/sorter/drf/sorter.cpp
@@ -67,7 +67,7 @@ void DRFSorter::initialize(
 }
 
 
-void DRFSorter::add(const string& name, double weight)
+void DRFSorter::add(const string& name)
 {
   CHECK(!contains(name));
 
@@ -75,7 +75,6 @@ void DRFSorter::add(const string& name, double weight)
   clients.insert(client);
 
   allocations[name] = Allocation();
-  weights[name] = weight;
 
   if (metrics.isSome()) {
     metrics->add(name);
@@ -83,20 +82,6 @@ void DRFSorter::add(const string& name, double weight)
 }
 
 
-void DRFSorter::update(const string& name, double weight)
-{
-  CHECK(weights.contains(name));
-  weights[name] = weight;
-
-  // If the total resources have changed, we're going to
-  // recalculate all the shares, so don't bother just
-  // updating this client.
-  if (!dirty) {
-    updateShare(name);
-  }
-}
-
-
 void DRFSorter::remove(const string& name)
 {
   CHECK(contains(name));
@@ -108,7 +93,6 @@ void DRFSorter::remove(const string& name)
   }
 
   allocations.erase(name);
-  weights.erase(name);
 
   if (metrics.isSome()) {
     metrics->remove(name);
@@ -144,6 +128,16 @@ void DRFSorter::deactivate(const string& name)
 }
 
 
+void DRFSorter::updateWeight(const string& name, double weight)
+{
+  weights[name] = weight;
+
+  // It would be possible to avoid dirtying the tree here (in some
+  // cases), but it doesn't seem worth the complexity.
+  dirty = true;
+}
+
+
 void DRFSorter::allocated(
     const string& name,
     const SlaveID& slaveId,
@@ -482,7 +476,19 @@ double DRFSorter::calculateShare(const string& name) const
     }
   }
 
-  return share / weights.at(name);
+  return share / clientWeight(name);
+}
+
+
+double DRFSorter::clientWeight(const string& name) const
+{
+  Option<double> weight = weights.get(name);
+
+  if (weight.isNone()) {
+    return 1.0;
+  }
+
+  return weight.get();
 }
 
 

http://git-wip-us.apache.org/repos/asf/mesos/blob/b7f70ca4/src/master/allocator/sorter/drf/sorter.hpp
----------------------------------------------------------------------
diff --git a/src/master/allocator/sorter/drf/sorter.hpp b/src/master/allocator/sorter/drf/sorter.hpp
index 7632922..19fa41b 100644
--- a/src/master/allocator/sorter/drf/sorter.hpp
+++ b/src/master/allocator/sorter/drf/sorter.hpp
@@ -78,9 +78,7 @@ public:
   virtual void initialize(
       const Option<std::set<std::string>>& fairnessExcludeResourceNames);
 
-  virtual void add(const std::string& name, double weight = 1);
-
-  virtual void update(const std::string& name, double weight);
+  virtual void add(const std::string& name);
 
   virtual void remove(const std::string& name);
 
@@ -88,6 +86,8 @@ public:
 
   virtual void deactivate(const std::string& name);
 
+  virtual void updateWeight(const std::string& name, double weight);
+
   virtual void allocated(
       const std::string& name,
       const SlaveID& slaveId,
@@ -140,6 +140,10 @@ private:
   // Resources (by name) that will be excluded from fair sharing.
   Option<std::set<std::string>> fairnessExcludeResourceNames;
 
+  // Returns the weight associated with the given path. If no weight
+  // has been configured, the default weight (1.0) is returned.
+  double clientWeight(const std::string& name) const;
+
   // Returns an iterator to the specified client, if
   // it exists in this Sorter.
   std::set<Client, DRFComparator>::iterator find(const std::string& name);

http://git-wip-us.apache.org/repos/asf/mesos/blob/b7f70ca4/src/master/allocator/sorter/sorter.hpp
----------------------------------------------------------------------
diff --git a/src/master/allocator/sorter/sorter.hpp b/src/master/allocator/sorter/sorter.hpp
index b3029fc..4de249e 100644
--- a/src/master/allocator/sorter/sorter.hpp
+++ b/src/master/allocator/sorter/sorter.hpp
@@ -58,11 +58,7 @@ public:
 
   // Adds a client to allocate resources to. A client
   // may be a user or a framework.
-  virtual void add(const std::string& client, double weight = 1) = 0;
-
-  // Updates the weight of a client. The client must have previously
-  // be added to the sorter, but it may currently be inactive.
-  virtual void update(const std::string& client, double weight) = 0;
+  virtual void add(const std::string& client) = 0;
 
   // Removes a client.
   virtual void remove(const std::string& client) = 0;
@@ -75,6 +71,14 @@ public:
   // It is a no-op if the client is already not in the sort.
   virtual void deactivate(const std::string& client) = 0;
 
+  // Updates the weight of a client name. The sorter will store this
+  // weight regardless of whether a client with this name has been
+  // added. If a client's weight is not changed, the default weight
+  // (1.0) is used. This interface does not support unsetting
+  // previously set weights; instead, a weight should be reset to the
+  // default value.
+  virtual void updateWeight(const std::string& client, double weight) = 0;
+
   // Specify that resources have been allocated to the given client.
   virtual void allocated(
       const std::string& client,

http://git-wip-us.apache.org/repos/asf/mesos/blob/b7f70ca4/src/tests/sorter_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/sorter_tests.cpp b/src/tests/sorter_tests.cpp
index 43bd857..8e86e4c 100644
--- a/src/tests/sorter_tests.cpp
+++ b/src/tests/sorter_tests.cpp
@@ -138,7 +138,8 @@ TEST(SorterTest, WDRFSorter)
 
   sorter.allocated("a", slaveId, Resources::parse("cpus:5;mem:5").get());
 
-  sorter.add("b", 2);
+  sorter.add("b");
+  sorter.updateWeight("b", 2);
   sorter.allocated("b", slaveId, Resources::parse("cpus:6;mem:6").get());
 
   // shares: a = .05, b = .03
@@ -150,7 +151,8 @@ TEST(SorterTest, WDRFSorter)
   // shares: a = .05, b = .03, c = .04
   EXPECT_EQ(vector<string>({"b", "c", "a"}), sorter.sort());
 
-  sorter.add("d", 10);
+  sorter.add("d");
+  sorter.updateWeight("d", 10);
   sorter.allocated("d", slaveId, Resources::parse("cpus:10;mem:20").get());
 
   // shares: a = .05, b = .03, c = .04, d = .02
@@ -165,7 +167,8 @@ TEST(SorterTest, WDRFSorter)
   // shares: a = .05, c = .04, d = .045
   EXPECT_EQ(vector<string>({"c", "d", "a"}), sorter.sort());
 
-  sorter.add("e", .1);
+  sorter.add("e");
+  sorter.updateWeight("e", 0.1);
   sorter.allocated("e", slaveId, Resources::parse("cpus:1;mem:1").get());
 
   // shares: a = .05, c = .04, d = .045, e = .1
@@ -197,8 +200,8 @@ TEST(SorterTest, WDRFSorterUpdateWeight)
   // shares: a = .05, b = .06
   EXPECT_EQ(vector<string>({"a", "b"}), sorter.sort());
 
-  // Increase b's  weight to flip the sort order.
-  sorter.update("b", 2);
+  // Increase b's weight to flip the sort order.
+  sorter.updateWeight("b", 2);
 
   // shares: a = .05, b = .03
   EXPECT_EQ(vector<string>({"b", "a"}), sorter.sort());