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 2014/12/17 04:15:26 UTC

[1/2] mesos git commit: Removed an unnecessary helper in the sorter tests.

Repository: mesos
Updated Branches:
  refs/heads/master ea240f290 -> d244b08cf


Removed an unnecessary helper in the sorter tests.

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


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

Branch: refs/heads/master
Commit: df22aa8d934eca0b327ab0e9527d33eebdc8711b
Parents: ea240f2
Author: Benjamin Mahler <be...@gmail.com>
Authored: Mon Dec 15 21:53:23 2014 -0800
Committer: Benjamin Mahler <be...@gmail.com>
Committed: Tue Dec 16 18:09:51 2014 -0800

----------------------------------------------------------------------
 src/tests/sorter_tests.cpp | 49 ++++++++++++++++-------------------------
 1 file changed, 19 insertions(+), 30 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/df22aa8d/src/tests/sorter_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/sorter_tests.cpp b/src/tests/sorter_tests.cpp
index 0516ab5..b80f44d 100644
--- a/src/tests/sorter_tests.cpp
+++ b/src/tests/sorter_tests.cpp
@@ -21,31 +21,20 @@
 
 #include <gmock/gmock.h>
 
+#include <list>
+#include <string>
+
+#include <mesos/resources.hpp>
+
 #include "master/drf_sorter.hpp"
-#include "master/sorter.hpp"
 
 using namespace mesos;
 
-using mesos::internal::master::allocator::Sorter;
 using mesos::internal::master::allocator::DRFSorter;
 
 using std::list;
 using std::string;
 
-void checkSorter(Sorter& sorter, uint32_t count, ...)
-{
-  va_list args;
-  va_start(args, count);
-  list<string> ordering = sorter.sort();
-  EXPECT_EQ(ordering.size(), count);
-
-  foreach (const string& actual, ordering) {
-    const char* expected = va_arg(args, char*);
-    EXPECT_EQ(actual, expected);
-  }
-  va_end(args);
-}
-
 
 TEST(SorterTest, DRFSorter)
 {
@@ -63,7 +52,7 @@ TEST(SorterTest, DRFSorter)
   sorter.allocated("b", bResources);
 
   // shares: a = .05, b = .06
-  checkSorter(sorter, 2, "a", "b");
+  EXPECT_EQ(list<string>({"a", "b"}), sorter.sort());
 
   Resources cResources = Resources::parse("cpus:1;mem:1").get();
   sorter.add("c");
@@ -74,14 +63,14 @@ TEST(SorterTest, DRFSorter)
   sorter.allocated("d", dResources);
 
   // shares: a = .05, b = .06, c = .01, d = .03
-  checkSorter(sorter, 4, "c", "d", "a", "b");
+  EXPECT_EQ(list<string>({"c", "d", "a", "b"}), sorter.sort());
 
   sorter.remove("a");
   Resources bUnallocated = Resources::parse("cpus:4;mem:4").get();
   sorter.unallocated("b", bUnallocated);
 
   // shares: b = .02, c = .01, d = .03
-  checkSorter(sorter, 3, "c", "b", "d");
+  EXPECT_EQ(list<string>({"c", "b", "d"}), sorter.sort());
 
   Resources eResources = Resources::parse("cpus:1;mem:5").get();
   sorter.add("e");
@@ -92,7 +81,7 @@ TEST(SorterTest, DRFSorter)
   // total resources is now cpus = 50, mem = 100
 
   // shares: b = .04, c = .02, d = .06, e = .05
-  checkSorter(sorter, 4, "c", "b", "e", "d");
+  EXPECT_EQ(list<string>({"c", "b", "e", "d"}), sorter.sort());
 
   Resources addedResources = Resources::parse("cpus:0;mem:100").get();
   sorter.add(addedResources);
@@ -106,7 +95,7 @@ TEST(SorterTest, DRFSorter)
   sorter.allocated("c", cResources2);
 
   // shares: b = .04, c = .08, d = .06, e = .025, f = .1
-  checkSorter(sorter, 5, "e", "b", "d", "c", "f");
+  EXPECT_EQ(list<string>({"e", "b", "d", "c", "f"}), sorter.sort());
 
   EXPECT_TRUE(sorter.contains("b"));
 
@@ -118,13 +107,13 @@ TEST(SorterTest, DRFSorter)
 
   EXPECT_TRUE(sorter.contains("d"));
 
-  checkSorter(sorter, 4, "e", "b", "c", "f");
+  EXPECT_EQ(list<string>({"e", "b", "c", "f"}), sorter.sort());
 
   EXPECT_EQ(sorter.count(), 5);
 
   sorter.activate("d");
 
-  checkSorter(sorter, 5, "e", "b", "d", "c", "f");
+  EXPECT_EQ(list<string>({"e", "b", "d", "c", "f"}), sorter.sort());
 }
 
 
@@ -142,36 +131,36 @@ TEST(SorterTest, WDRFSorter)
   sorter.allocated("b", Resources::parse("cpus:6;mem:6").get());
 
   // shares: a = .05, b = .03
-  checkSorter(sorter, 2, "b", "a");
+  EXPECT_EQ(list<string>({"b", "a"}), sorter.sort());
 
   sorter.add("c");
   sorter.allocated("c", Resources::parse("cpus:4;mem:4").get());
 
   // shares: a = .05, b = .03, c = .04
-  checkSorter(sorter, 3, "b", "c", "a");
+  EXPECT_EQ(list<string>({"b", "c", "a"}), sorter.sort());
 
   sorter.add("d", 10);
   sorter.allocated("d", Resources::parse("cpus:10;mem:20").get());
 
   // shares: a = .05, b = .03, c = .04, d = .02
-  checkSorter(sorter, 4, "d", "b", "c", "a");
+  EXPECT_EQ(list<string>({"d", "b", "c", "a"}), sorter.sort());
 
   sorter.remove("b");
 
-  checkSorter(sorter, 3, "d", "c", "a");
+  EXPECT_EQ(list<string>({"d", "c", "a"}), sorter.sort());
 
   sorter.allocated("d", Resources::parse("cpus:10;mem:25").get());
 
   // shares: a = .05, c = .04, d = .045
-  checkSorter(sorter, 3, "c", "d", "a");
+  EXPECT_EQ(list<string>({"c", "d", "a"}), sorter.sort());
 
   sorter.add("e", .1);
   sorter.allocated("e", Resources::parse("cpus:1;mem:1").get());
 
   // shares: a = .05, c = .04, d = .045, e = .1
-  checkSorter(sorter, 4, "c", "d", "a", "e");
+  EXPECT_EQ(list<string>({"c", "d", "a", "e"}), sorter.sort());
 
   sorter.remove("a");
 
-  checkSorter(sorter, 3, "c", "d", "e");
+  EXPECT_EQ(list<string>({"c", "d", "e"}), sorter.sort());
 }


[2/2] mesos git commit: Fixed the DRFSorter share calculation to not assume flattened resources.

Posted by bm...@apache.org.
Fixed the DRFSorter share calculation to not assume flattened resources.

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


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

Branch: refs/heads/master
Commit: d244b08cf1dad3e4494eb42cf902807754f355a3
Parents: df22aa8
Author: Benjamin Mahler <be...@gmail.com>
Authored: Mon Dec 15 21:26:05 2014 -0800
Committer: Benjamin Mahler <be...@gmail.com>
Committed: Tue Dec 16 18:39:30 2014 -0800

----------------------------------------------------------------------
 src/master/drf_sorter.cpp  | 26 +++++++++++++++++---------
 src/tests/sorter_tests.cpp | 30 ++++++++++++++++++++++++++++++
 2 files changed, 47 insertions(+), 9 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/d244b08c/src/master/drf_sorter.cpp
----------------------------------------------------------------------
diff --git a/src/master/drf_sorter.cpp b/src/master/drf_sorter.cpp
index 0ad6c52..ebc0284 100644
--- a/src/master/drf_sorter.cpp
+++ b/src/master/drf_sorter.cpp
@@ -217,24 +217,32 @@ double DRFSorter::calculateShare(const string& name)
 {
   double share = 0;
 
-  // TODO(benh): This implementaion of "dominant resource fairness"
+  // TODO(benh): This implementation of "dominant resource fairness"
   // currently does not take into account resources that are not
   // scalars.
 
+  // Scalar resources may be spread across multiple 'Resource'
+  // objects. E.g. persistent disks. So we first collect the names
+  // of the scalar resources, before computing the totals.
+  hashset<string> scalars;
   foreach (const Resource& resource, resources) {
     if (resource.type() == Value::SCALAR) {
-      double total = resource.scalar().value();
+      scalars.insert(resource.name());
+    }
+  }
 
-      if (total > 0) {
-        Option<Value::Scalar> scalar =
-          allocations[name].get<Value::Scalar>(resource.name());
+  foreach (const string& scalar, scalars) {
+    Option<Value::Scalar> total = resources.get<Value::Scalar>(scalar);
 
-        if (scalar.isNone()) {
-          scalar = Value::Scalar();
-        }
+    if (total.isSome() && total.get().value() > 0) {
+      Option<Value::Scalar> allocation =
+        allocations[name].get<Value::Scalar>(scalar);
 
-        share = std::max(share, scalar.get().value() / total);
+      if (allocation.isNone()) {
+        allocation = Value::Scalar();
       }
+
+      share = std::max(share, allocation.get().value() / total.get().value());
     }
   }
 

http://git-wip-us.apache.org/repos/asf/mesos/blob/d244b08c/src/tests/sorter_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/sorter_tests.cpp b/src/tests/sorter_tests.cpp
index b80f44d..c2f4aa1 100644
--- a/src/tests/sorter_tests.cpp
+++ b/src/tests/sorter_tests.cpp
@@ -164,3 +164,33 @@ TEST(SorterTest, WDRFSorter)
 
   EXPECT_EQ(list<string>({"c", "d", "e"}), sorter.sort());
 }
+
+
+// Some resources are split across multiple resource objects
+// (e.g. persistent disks). This test ensures that the shares
+// for these are accounted correctly.
+TEST(SorterTest, SplitResourceShares)
+{
+  DRFSorter sorter;
+
+  sorter.add("a");
+  sorter.add("b");
+
+  Resource disk1 = Resources::parse("disk", "5", "*").get();
+  disk1.mutable_disk()->mutable_persistence()->set_id("ID2");
+  disk1.mutable_disk()->mutable_volume()->set_container_path("data");
+
+  Resource disk2 = Resources::parse("disk", "5", "*").get();
+  disk2.mutable_disk()->mutable_persistence()->set_id("ID2");
+  disk2.mutable_disk()->mutable_volume()->set_container_path("data");
+
+  sorter.add(Resources::parse("cpus:100;mem:100;disk:95").get()
+             + disk1 + disk2);
+
+  // Now, allocate resources to "a" and "b". Note that "b" will have
+  // more disk if the shares are accounted correctly!
+  sorter.allocated("a", Resources::parse("cpus:9;mem:9;disk:9").get());
+  sorter.allocated("b", Resources::parse("cpus:9;mem:9").get() + disk1 + disk2);
+
+  EXPECT_EQ(list<string>({"a", "b"}), sorter.sort());
+}