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 <[email protected]> Authored: Wed Nov 19 14:54:04 2014 -0800 Committer: Jie Yu <[email protected]> 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()); +}
