You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by ji...@apache.org on 2014/11/21 01:27:30 UTC

[1/3] mesos git commit: Allowed C++ Resources to handle DiskInfo.

Repository: mesos
Updated Branches:
  refs/heads/master f42d2508b -> 455bfff6e


Allowed C++ Resources to handle DiskInfo.

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


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

Branch: refs/heads/master
Commit: 5d2836ba36e709ae544f8b9f41d4db4f0e211999
Parents: c1940a9
Author: Jie Yu <yu...@gmail.com>
Authored: Wed Nov 19 14:54:04 2014 -0800
Committer: Jie Yu <yu...@gmail.com>
Committed: Thu Nov 20 16:10:31 2014 -0800

----------------------------------------------------------------------
 src/common/resources.cpp      | 111 ++++++++++++++++++++++++++-----------
 src/tests/resources_tests.cpp |  57 +++++++++++++++++++
 2 files changed, 137 insertions(+), 31 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/5d2836ba/src/common/resources.cpp
----------------------------------------------------------------------
diff --git a/src/common/resources.cpp b/src/common/resources.cpp
index 5cd64ff..6fabca9 100644
--- a/src/common/resources.cpp
+++ b/src/common/resources.cpp
@@ -39,14 +39,81 @@ namespace mesos {
 // Helper functions.
 /////////////////////////////////////////////////
 
+bool operator == (
+    const Resource::DiskInfo& left,
+    const Resource::DiskInfo& right)
+{
+  // NOTE: We ignore 'volume' inside DiskInfo when doing comparison
+  // because it describes how this resource will be used which has
+  // nothing to do with the Resource object itself. A framework can
+  // use this resource and specify different 'volume' every time it
+  // uses it.
+  if (left.has_persistence() != right.has_persistence()) {
+    return false;
+  }
+
+  if (left.has_persistence()) {
+    return left.persistence().id() == right.persistence().id();
+  }
+
+  return true;
+}
+
+
+bool operator != (
+    const Resource::DiskInfo& left,
+    const Resource::DiskInfo& right)
+{
+  return !(left == right);
+}
+
+
+bool operator == (const Resource& left, const Resource& right)
+{
+  if (left.name() != right.name() ||
+      left.type() != right.type() ||
+      left.role() != right.role()) {
+    return false;
+  }
+
+  // NOTE: Not setting the DiskInfo is the same as setting the
+  // DiskInfo with no 'volume' and 'persistence' (default).
+  if (left.disk() != right.disk()) {
+    return false;
+  }
+
+  if (left.type() == Value::SCALAR) {
+    return left.scalar() == right.scalar();
+  } else if (left.type() == Value::RANGES) {
+    return left.ranges() == right.ranges();
+  } else if (left.type() == Value::SET) {
+    return left.set() == right.set();
+  } else {
+    return false;
+  }
+}
+
+
+bool operator != (const Resource& left, const Resource& right)
+{
+  return !(left == right);
+}
+
+
 // Tests if we can add two Resource objects together resulting in one
 // valid Resource object. For example, two Resource objects with
 // different name, type or role are not addable.
+// TODO(jieyu): Even if two Resource objects with DiskInfo have the
+// same persistence ID, they cannot be added together. In fact, this
+// shouldn't happen if we do not add resources from different
+// namespaces (e.g., slave). Consider adding a warning.
 static bool addable(const Resource& left, const Resource& right)
 {
   return left.name() == right.name() &&
     left.type() == right.type() &&
-    left.role() == right.role();
+    left.role() == right.role() &&
+    !left.disk().has_persistence() &&
+    !right.disk().has_persistence();
 }
 
 
@@ -58,61 +125,43 @@ static bool addable(const Resource& left, const Resource& right)
 // "left = {1, 2}" and "right = {2, 3}", "left" and "right" are
 // subtractable because "left - right = {1}". However, "left" does not
 // contains "right".
+// NOTE: For Resource objects that have DiskInfo, we can only do
+// subtraction if they are equal.
 static bool subtractable(const Resource& left, const Resource& right)
 {
-  return left.name() == right.name() &&
-    left.type() == right.type() &&
-    left.role() == right.role();
-}
-
-
-// Tests if "right" is contained in "left".
-static bool contains(const Resource& left, const Resource& right)
-{
   if (left.name() != right.name() ||
       left.type() != right.type() ||
       left.role() != right.role()) {
     return false;
   }
 
-  if (left.type() == Value::SCALAR) {
-    return right.scalar() <= left.scalar();
-  } else if (left.type() == Value::RANGES) {
-    return right.ranges() <= left.ranges();
-  } else if (left.type() == Value::SET) {
-    return right.set() <= left.set();
-  } else {
-    return false;
+  if (left.has_disk() || right.has_disk()) {
+    return left == right;
   }
+
+  return true;
 }
 
 
-bool operator == (const Resource& left, const Resource& right)
+// Tests if "right" is contained in "left".
+static bool contains(const Resource& left, const Resource& right)
 {
-  if (left.name() != right.name() ||
-      left.type() != right.type() ||
-      left.role() != right.role()) {
+  if (!subtractable(left, right)) {
     return false;
   }
 
   if (left.type() == Value::SCALAR) {
-    return left.scalar() == right.scalar();
+    return right.scalar() <= left.scalar();
   } else if (left.type() == Value::RANGES) {
-    return left.ranges() == right.ranges();
+    return right.ranges() <= left.ranges();
   } else if (left.type() == Value::SET) {
-    return left.set() == right.set();
+    return right.set() <= left.set();
   } else {
     return false;
   }
 }
 
 
-bool operator != (const Resource& left, const Resource& right)
-{
-  return !(left == right);
-}
-
-
 Resource& operator += (Resource& left, const Resource& right)
 {
   if (left.type() == Value::SCALAR) {

http://git-wip-us.apache.org/repos/asf/mesos/blob/5d2836ba/src/tests/resources_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/resources_tests.cpp b/src/tests/resources_tests.cpp
index 3857f8b..93b5a53 100644
--- a/src/tests/resources_tests.cpp
+++ b/src/tests/resources_tests.cpp
@@ -872,3 +872,60 @@ TEST_F(DiskResourcesTest, Validation)
   EXPECT_NONE(
       Resources::validate(createDiskResource("10", "*", None(), "path")));
 }
+
+
+TEST_F(DiskResourcesTest, Equals)
+{
+  Resources r1 = createDiskResource("10", "*", None(), None());
+  Resources r2 = createDiskResource("10", "*", None(), "path1");
+  Resources r3 = createDiskResource("10", "*", None(), "path2");
+  Resources r4 = createDiskResource("10", "role", None(), "path2");
+  Resources r5 = createDiskResource("10", "role", "1", "path1");
+  Resources r6 = createDiskResource("10", "role", "1", "path2");
+  Resources r7 = createDiskResource("10", "role", "2", "path2");
+
+  EXPECT_EQ(r1, r2);
+  EXPECT_EQ(r2, r3);
+  EXPECT_EQ(r5, r6);
+
+  EXPECT_NE(r6, r7);
+  EXPECT_NE(r4, r7);
+}
+
+
+TEST_F(DiskResourcesTest, Addition)
+{
+  Resources r1 = createDiskResource("10", "role", None(), "path");
+  Resources r2 = createDiskResource("10", "role", None(), None());
+  Resources r3 = createDiskResource("20", "role", None(), "path");
+
+  EXPECT_EQ(r3, r1 + r2);
+
+  Resources r4 = createDiskResource("10", "role", "1", "path");
+  Resources r5 = createDiskResource("10", "role", "2", "path");
+  Resources r6 = createDiskResource("20", "role", "1", "path");
+
+  Resources sum = r4 + r5;
+
+  EXPECT_TRUE(sum.contains(r4));
+  EXPECT_TRUE(sum.contains(r5));
+  EXPECT_FALSE(sum.contains(r3));
+  EXPECT_FALSE(sum.contains(r6));
+}
+
+
+TEST_F(DiskResourcesTest, Subtraction)
+{
+  Resources r1 = createDiskResource("10", "role", None(), "path");
+  Resources r2 = createDiskResource("10", "role", None(), None());
+
+  EXPECT_TRUE((r1 - r2).empty());
+
+  Resources r3 = createDiskResource("10", "role", "1", "path");
+  Resources r4 = createDiskResource("10", "role", "2", "path");
+  Resources r5 = createDiskResource("10", "role", "2", "path2");
+
+  EXPECT_EQ(r3, r3 - r4);
+  EXPECT_TRUE((r3 - r3).empty());
+  EXPECT_TRUE((r4 - r5).empty());
+}


[2/3] mesos git commit: Added validation for Resource objects with DiskInfo.

Posted by ji...@apache.org.
Added validation for Resource objects with DiskInfo.

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


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

Branch: refs/heads/master
Commit: c1940a9049d5c2a7afe5200597e49654829e38d2
Parents: f42d250
Author: Jie Yu <yu...@gmail.com>
Authored: Wed Nov 19 12:49:23 2014 -0800
Committer: Jie Yu <yu...@gmail.com>
Committed: Thu Nov 20 16:10:31 2014 -0800

----------------------------------------------------------------------
 src/common/resources.cpp      | 28 +++++++++++++++
 src/tests/resources_tests.cpp | 74 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 102 insertions(+)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/c1940a90/src/common/resources.cpp
----------------------------------------------------------------------
diff --git a/src/common/resources.cpp b/src/common/resources.cpp
index 61d16a8..5cd64ff 100644
--- a/src/common/resources.cpp
+++ b/src/common/resources.cpp
@@ -308,6 +308,34 @@ Option<Error> Resources::validate(const Resource& resource)
     return Error("Unsupported resource type");
   }
 
+  // Checks for 'disk' resource.
+  if (resource.has_disk()) {
+    if (resource.name() != "disk") {
+      return Error(
+          "DiskInfo should not be set for " + resource.name() + " resource");
+    }
+
+    if (resource.disk().has_persistence()) {
+      if (resource.role() == "*") {
+        return Error("Persistent disk volume is disallowed for '*' role");
+      }
+
+      if (!resource.disk().has_volume()) {
+        return Error("Persistent disk should specify a volume");
+      }
+    }
+
+    if (resource.disk().has_volume()) {
+      if (resource.disk().volume().mode() == Volume::RO) {
+        return Error("Do not support RO volume in DiskInfo");
+      }
+
+      if (resource.disk().volume().has_host_path()) {
+        return Error("Volume in DiskInfo should not have 'host_path' set");
+      }
+    }
+  }
+
   return None();
 }
 

http://git-wip-us.apache.org/repos/asf/mesos/blob/c1940a90/src/tests/resources_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/resources_tests.cpp b/src/tests/resources_tests.cpp
index 73f50ba..3857f8b 100644
--- a/src/tests/resources_tests.cpp
+++ b/src/tests/resources_tests.cpp
@@ -798,3 +798,77 @@ TEST(ResourcesTest, Find)
 
   EXPECT_NONE(resources4.find(targets4));
 }
+
+
+class DiskResourcesTest : public ::testing::Test
+{
+public:
+  Resource::DiskInfo createDiskInfo(
+      const Option<string>& persistenceID,
+      const Option<string>& containerPath)
+  {
+    Resource::DiskInfo info;
+
+    if (persistenceID.isSome()) {
+      Resource::DiskInfo::Persistence persistence;
+      persistence.set_id(persistenceID.get());
+      info.mutable_persistence()->CopyFrom(persistence);
+    }
+
+    if (containerPath.isSome()) {
+      Volume volume;
+      volume.set_container_path(containerPath.get());
+      volume.set_mode(Volume::RW);
+      info.mutable_volume()->CopyFrom(volume);
+    }
+
+    return info;
+  }
+
+  Resource createDiskResource(
+      const string& value,
+      const string& role,
+      const Option<string>& persistenceID,
+      const Option<string>& containerPath)
+  {
+    Resource resource = Resources::parse("disk", value, role).get();
+
+    if (persistenceID.isSome() || containerPath.isSome()) {
+      resource.mutable_disk()->CopyFrom(
+          createDiskInfo(persistenceID, containerPath));
+    }
+
+    return resource;
+  }
+};
+
+
+TEST_F(DiskResourcesTest, Validation)
+{
+  Resource cpus = Resources::parse("cpus", "2", "*").get();
+  cpus.mutable_disk()->CopyFrom(createDiskInfo("1", "path"));
+
+  Option<Error> error = Resources::validate(cpus);
+  ASSERT_SOME(error);
+  EXPECT_EQ(
+      "Resource with DiskInfo does not have the name 'disk'",
+      error.get().message);
+
+  error = Resources::validate(createDiskResource("10", "*", "1", "path"));
+  ASSERT_SOME(error);
+  EXPECT_EQ(
+      "Do not allow a persistent disk volume without reservation",
+      error.get().message);
+
+  error = Resources::validate(createDiskResource("10", "role", "1", None()));
+  ASSERT_SOME(error);
+  EXPECT_EQ(
+      "Persistent disk should specify a volume",
+      error.get().message);
+
+  EXPECT_NONE(
+      Resources::validate(createDiskResource("10", "role", "1", "path")));
+
+  EXPECT_NONE(
+      Resources::validate(createDiskResource("10", "*", None(), "path")));
+}


[3/3] mesos git commit: Fixed a bug due to missing canonicalize.

Posted by ji...@apache.org.
Fixed a bug due to missing canonicalize.

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


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

Branch: refs/heads/master
Commit: 455bfff6e9d7e849b78c590dbd4e1f828151c0d8
Parents: 5d2836b
Author: Jie Yu <yu...@gmail.com>
Authored: Thu Nov 20 16:21:36 2014 -0800
Committer: Jie Yu <yu...@gmail.com>
Committed: Thu Nov 20 16:21:49 2014 -0800

----------------------------------------------------------------------
 3rdparty/libprocess/include/process/delay.hpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/455bfff6/3rdparty/libprocess/include/process/delay.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/include/process/delay.hpp b/3rdparty/libprocess/include/process/delay.hpp
index 40627ea..be2d36b 100644
--- a/3rdparty/libprocess/include/process/delay.hpp
+++ b/3rdparty/libprocess/include/process/delay.hpp
@@ -39,7 +39,7 @@ Timer delay(const Duration& duration,
     lambda::bind(internal::dispatch,
                  pid,
                  dispatcher,
-                 internal::canonicalize(method));
+                 &typeid(method));
 
   return Clock::timer(duration, dispatch);
 }
@@ -86,7 +86,7 @@ Timer delay(const Duration& duration,
       lambda::bind(internal::dispatch,                                  \
                    pid,                                                 \
                    dispatcher,                                          \
-                   internal::canonicalize(method));                     \
+                   &typeid(method));                                    \
                                                                         \
     return Clock::timer(duration, dispatch);                            \
   }                                                                     \