You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by jo...@apache.org on 2016/01/20 00:09:20 UTC

[1/2] mesos git commit: Quota: Calculated 'remainingClusterResources' globally in 'allocate'.

Repository: mesos
Updated Branches:
  refs/heads/master f5e1bee6b -> 46ac43a8f


Quota: Calculated 'remainingClusterResources' globally in 'allocate'.

Event-triggered allocations do not include all available agents. If we
calculate remaining resources in the cluster using the partial view,
we may overlook already laid away resources for quota'ed roles and lay
away more. Hence we may unnecessarily deprive non-quota'ed frameworks
of resources.

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


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

Branch: refs/heads/master
Commit: ca5cf5c5b290e4dc8edcf890743e356c31d5f468
Parents: f5e1bee
Author: Klaus Ma <kl...@gmail.com>
Authored: Tue Jan 19 14:51:43 2016 -0800
Committer: Joris Van Remoortere <jo...@gmail.com>
Committed: Tue Jan 19 15:06:43 2016 -0800

----------------------------------------------------------------------
 src/master/allocator/mesos/hierarchical.cpp | 12 +++++++++---
 src/tests/hierarchical_allocator_tests.cpp  | 21 +++++++++++++++------
 2 files changed, 24 insertions(+), 9 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/ca5cf5c5/src/master/allocator/mesos/hierarchical.cpp
----------------------------------------------------------------------
diff --git a/src/master/allocator/mesos/hierarchical.cpp b/src/master/allocator/mesos/hierarchical.cpp
index 48acde6..541c58a 100644
--- a/src/master/allocator/mesos/hierarchical.cpp
+++ b/src/master/allocator/mesos/hierarchical.cpp
@@ -1225,10 +1225,16 @@ void HierarchicalAllocatorProcess::allocate(
   // Calculate how many resources (including revocable and reserved) are
   // available for allocation in the next round. We need this in order to
   // ensure we do not over-allocate resources during the second stage.
+  //
+  // NOTE: We use all active agents and not just those visible in the current
+  // `allocate()` call so that frameworks in roles without quota are not
+  // unnecessarily deprived of resources.
   Resources remainingClusterResources;
-  foreach (const SlaveID& slaveId, slaveIds) {
-    remainingClusterResources +=
-      slaves[slaveId].total - slaves[slaveId].allocated;
+  foreachkey (const SlaveID& slaveId, slaves) {
+    if (isWhitelisted(slaveId) && slaves[slaveId].activated) {
+      remainingClusterResources +=
+        slaves[slaveId].total - slaves[slaveId].allocated;
+    }
   }
 
   // Frameworks in a quota'ed role may temporarily reject resources by

http://git-wip-us.apache.org/repos/asf/mesos/blob/ca5cf5c5/src/tests/hierarchical_allocator_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/hierarchical_allocator_tests.cpp b/src/tests/hierarchical_allocator_tests.cpp
index 3ac05e9..0176ee8 100644
--- a/src/tests/hierarchical_allocator_tests.cpp
+++ b/src/tests/hierarchical_allocator_tests.cpp
@@ -1807,13 +1807,15 @@ TEST_F(HierarchicalAllocatorTest, QuotaAbsentFramework)
 
   initialize();
 
-  SlaveInfo agent1 = createSlaveInfo("cpus:2;mem:1024;disk:0");
-  SlaveInfo agent2 = createSlaveInfo("cpus:1;mem:512;disk:0");
-
-  allocator->addSlave(agent1.id(), agent1, None(), agent1.resources(), EMPTY);
-  allocator->addSlave(agent2.id(), agent2, None(), agent2.resources(), EMPTY);
+  // We want to start with the following cluster setup.
+  // Total cluster resources: cpus=3, mem=1536.
+  // QUOTA_ROLE share = 1 (cpus=2, mem=1024) [quota: cpus=2, mem=1024]
+  //   no framework
+  // NO_QUOTA_ROLE share = 0.5 (cpus=1, mem=512)
+  //   framework1 share = 1
 
-  // Set quota for the quota'ed role.
+  // Set quota for the quota'ed role. This role isn't registered with
+  // the allocator yet.
   const Quota quota1 = createQuota(QUOTA_ROLE, "cpus:2;mem:1024");
   allocator->setQuota(QUOTA_ROLE, quota1);
 
@@ -1822,6 +1824,13 @@ TEST_F(HierarchicalAllocatorTest, QuotaAbsentFramework)
   allocator->addFramework(
       framework1.id(), framework1, hashmap<SlaveID, Resources>());
 
+  SlaveInfo agent1 = createSlaveInfo("cpus:2;mem:1024;disk:0");
+  SlaveInfo agent2 = createSlaveInfo("cpus:1;mem:512;disk:0");
+
+  // NOTE: Each `addSlave()` triggers an event-based allocation.
+  allocator->addSlave(agent1.id(), agent1, None(), agent1.resources(), EMPTY);
+  allocator->addSlave(agent2.id(), agent2, None(), agent2.resources(), EMPTY);
+
   // `framework1` can only be allocated resources on `agent2`. This
   // is due to the coarse-grained nature of the allocations. All the
   // free resources on `agent1` would be considered to construct an


[2/2] mesos git commit: Quota: Updated comments around hierarchical tests.

Posted by jo...@apache.org.
Quota: Updated comments around hierarchical tests.

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


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

Branch: refs/heads/master
Commit: 46ac43a8fa7a603fd75bb998de4201c91584b578
Parents: ca5cf5c
Author: Alexander Rukletsov <ru...@gmail.com>
Authored: Tue Jan 19 14:52:03 2016 -0800
Committer: Joris Van Remoortere <jo...@gmail.com>
Committed: Tue Jan 19 15:08:59 2016 -0800

----------------------------------------------------------------------
 src/master/allocator/mesos/hierarchical.cpp |  5 ++-
 src/tests/hierarchical_allocator_tests.cpp  | 49 ++++++++++++++++--------
 2 files changed, 37 insertions(+), 17 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/46ac43a8/src/master/allocator/mesos/hierarchical.cpp
----------------------------------------------------------------------
diff --git a/src/master/allocator/mesos/hierarchical.cpp b/src/master/allocator/mesos/hierarchical.cpp
index 541c58a..129190c 100644
--- a/src/master/allocator/mesos/hierarchical.cpp
+++ b/src/master/allocator/mesos/hierarchical.cpp
@@ -1227,8 +1227,9 @@ void HierarchicalAllocatorProcess::allocate(
   // ensure we do not over-allocate resources during the second stage.
   //
   // NOTE: We use all active agents and not just those visible in the current
-  // `allocate()` call so that frameworks in roles without quota are not
-  // unnecessarily deprived of resources.
+  // allocation (i.e. provided as an argument to the `allocate()` call) so
+  // that frameworks in roles without quota are not unnecessarily deprived
+  // of resources.
   Resources remainingClusterResources;
   foreachkey (const SlaveID& slaveId, slaves) {
     if (isWhitelisted(slaveId) && slaves[slaveId].activated) {

http://git-wip-us.apache.org/repos/asf/mesos/blob/46ac43a8/src/tests/hierarchical_allocator_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/hierarchical_allocator_tests.cpp b/src/tests/hierarchical_allocator_tests.cpp
index 0176ee8..ddabf66 100644
--- a/src/tests/hierarchical_allocator_tests.cpp
+++ b/src/tests/hierarchical_allocator_tests.cpp
@@ -1682,7 +1682,7 @@ TEST_F(HierarchicalAllocatorTest, QuotaAgainstStarvation)
 
   initialize();
 
-  // We want to start with the following cluster setup.
+  // We start with the following cluster setup.
   // Total cluster resources (2 identical agents): cpus=2, mem=1024.
   // QUOTA_ROLE share = 0.5 (cpus=1, mem=512)
   //   framework1 share = 1
@@ -1794,10 +1794,15 @@ TEST_F(HierarchicalAllocatorTest, QuotaAgainstStarvation)
 }
 
 
-// This test checks that quota is respected even for roles that don't
-// have any frameworks currently registered.
+// This test checks that quota is respected even for roles that do not
+// have any frameworks currently registered. It also ensures an event-
+// triggered allocation does not unnecessarily deprive non-quota'ed
+// frameworks of resources.
 TEST_F(HierarchicalAllocatorTest, QuotaAbsentFramework)
 {
+  // Pausing the clock is not necessary, but ensures that the test
+  // doesn't rely on the periodic allocation in the allocator, which
+  // would slow down the test.
   Clock::pause();
 
   const string QUOTA_ROLE{"quota-role"};
@@ -1807,31 +1812,45 @@ TEST_F(HierarchicalAllocatorTest, QuotaAbsentFramework)
 
   initialize();
 
-  // We want to start with the following cluster setup.
-  // Total cluster resources: cpus=3, mem=1536.
-  // QUOTA_ROLE share = 1 (cpus=2, mem=1024) [quota: cpus=2, mem=1024]
-  //   no framework
-  // NO_QUOTA_ROLE share = 0.5 (cpus=1, mem=512)
-  //   framework1 share = 1
+  // We start with the following cluster setup.
+  // Total cluster resources (2 agents): cpus=3, mem=1536.
+  // QUOTA_ROLE share = 0 [quota: cpus=2, mem=1024]
+  //   no frameworks
+  // NO_QUOTA_ROLE share = 0
+  //   framework share = 1
 
   // Set quota for the quota'ed role. This role isn't registered with
   // the allocator yet.
   const Quota quota1 = createQuota(QUOTA_ROLE, "cpus:2;mem:1024");
   allocator->setQuota(QUOTA_ROLE, quota1);
 
-  // Add `framework1` in the non-quota'ed role.
-  FrameworkInfo framework1 = createFrameworkInfo(NO_QUOTA_ROLE);
+  // Add `framework` in the non-quota'ed role.
+  FrameworkInfo framework = createFrameworkInfo(NO_QUOTA_ROLE);
   allocator->addFramework(
-      framework1.id(), framework1, hashmap<SlaveID, Resources>());
+      framework.id(), framework, hashmap<SlaveID, Resources>());
+
+  // Process all triggered allocation events.
+  // NOTE: No allocations happen because there are no resources to allocate.
+  Clock::settle();
 
   SlaveInfo agent1 = createSlaveInfo("cpus:2;mem:1024;disk:0");
   SlaveInfo agent2 = createSlaveInfo("cpus:1;mem:512;disk:0");
 
-  // NOTE: Each `addSlave()` triggers an event-based allocation.
+  // Each `addSlave()` triggers an event-based allocation.
+  // NOTE: The second event-based allocation for `agent2` takes into account
+  // that `agent1`'s resources are laid away for `QUOTA_ROLE`'s quota and
+  // hence freely allocates for the non-quota'ed `NO_QUOTA_ROLE` role.
   allocator->addSlave(agent1.id(), agent1, None(), agent1.resources(), EMPTY);
   allocator->addSlave(agent2.id(), agent2, None(), agent2.resources(), EMPTY);
 
-  // `framework1` can only be allocated resources on `agent2`. This
+  // Total cluster resources (2 agents): cpus=3, mem=1536.
+  // QUOTA_ROLE share = 0 [quota: cpus=2, mem=1024], but
+  //                    some resources (cpus=2, mem=1024) are laid away
+  //   no frameworks
+  // NO_QUOTA_ROLE share = 0.33
+  //   framework share = 1 (cpus=1, mem=512)
+
+  // `framework` can only be allocated resources on `agent2`. This
   // is due to the coarse-grained nature of the allocations. All the
   // free resources on `agent1` would be considered to construct an
   // offer, and that would exceed the resources allowed to be offered
@@ -1843,7 +1862,7 @@ TEST_F(HierarchicalAllocatorTest, QuotaAbsentFramework)
   // framework side, so we make due with this instead.
   Future<Allocation> allocation = allocations.get();
   AWAIT_READY(allocation);
-  EXPECT_EQ(framework1.id(), allocation.get().frameworkId);
+  EXPECT_EQ(framework.id(), allocation.get().frameworkId);
   EXPECT_EQ(agent2.resources(), Resources::sum(allocation.get().resources));
 }