Replaced <= with contains() in C++ Resources. Review: https://reviews.apache.org/r/28094
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/5fbc6264 Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/5fbc6264 Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/5fbc6264 Branch: refs/heads/master Commit: 5fbc6264aab8370925793aecb29d6d1f54d37cba Parents: 7fa0217 Author: Jie Yu <[email protected]> Authored: Fri Nov 14 22:33:28 2014 -0800 Committer: Jie Yu <[email protected]> Committed: Wed Nov 19 00:14:26 2014 -0800 ---------------------------------------------------------------------- include/mesos/resources.hpp | 5 ++- src/cli/execute.cpp | 3 +- src/common/resources.cpp | 36 ++++++++---------- src/examples/low_level_scheduler_libprocess.cpp | 2 +- src/examples/low_level_scheduler_pthread.cpp | 2 +- src/examples/no_executor_framework.cpp | 2 +- src/examples/test_framework.cpp | 2 +- src/master/hierarchical_allocator_process.hpp | 6 +-- src/master/master.cpp | 2 +- src/tests/mesos.hpp | 3 +- src/tests/resources_tests.cpp | 40 ++++++++++---------- src/tests/slave_recovery_tests.cpp | 5 ++- 12 files changed, 53 insertions(+), 55 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mesos/blob/5fbc6264/include/mesos/resources.hpp ---------------------------------------------------------------------- diff --git a/include/mesos/resources.hpp b/include/mesos/resources.hpp index 13a81e2..10777a6 100644 --- a/include/mesos/resources.hpp +++ b/include/mesos/resources.hpp @@ -104,6 +104,9 @@ public: bool empty() const { return resources.size() == 0; } + // Checks if this Resources is a superset of the given Resources. + bool contains(const Resources& that) const; + // Returns all resources in this object that are marked with the // specified role. Resources extract(const std::string& role) const; @@ -162,8 +165,6 @@ public: bool operator == (const Resources& that) const; bool operator != (const Resources& that) const; - bool operator <= (const Resources& that) const; - // NOTE: If any error occurs (e.g., input Resource is not valid or // the first operand is not a superset of the second oprand while // doing subtraction), the semantics is as though the second operand http://git-wip-us.apache.org/repos/asf/mesos/blob/5fbc6264/src/cli/execute.cpp ---------------------------------------------------------------------- diff --git a/src/cli/execute.cpp b/src/cli/execute.cpp index ddaa20d..77deec9 100644 --- a/src/cli/execute.cpp +++ b/src/cli/execute.cpp @@ -170,7 +170,8 @@ public: } foreach (const Offer& offer, offers) { - if (!launched && TASK_RESOURCES.get() <= offer.resources()) { + if (!launched && + Resources(offer.resources()).contains(TASK_RESOURCES.get())) { TaskInfo task; task.set_name(name); task.mutable_task_id()->set_value(name); http://git-wip-us.apache.org/repos/asf/mesos/blob/5fbc6264/src/common/resources.cpp ---------------------------------------------------------------------- diff --git a/src/common/resources.cpp b/src/common/resources.cpp index 3a3c6a6..23fd6a3 100644 --- a/src/common/resources.cpp +++ b/src/common/resources.cpp @@ -113,12 +113,6 @@ bool operator != (const Resource& left, const Resource& right) } -bool operator <= (const Resource& left, const Resource& right) -{ - return contains(right, left); -} - - Resource& operator += (Resource& left, const Resource& right) { // TODO(jieyu): Leverage += for Value to avoid copying. @@ -372,6 +366,18 @@ Resources::Resources( } +bool Resources::contains(const Resources& that) const +{ + foreach (const Resource& resource, that.resources) { + if (!contains(resource)) { + return false; + } + } + + return true; +} + + Resources Resources::extract(const string& role) const { Resources r; @@ -436,10 +442,10 @@ Option<Resources> Resources::find(const Resource& target) const // Need to flatten to ignore the roles in contains(). Resources flattened = Resources(resource).flatten(); - if (remaining <= flattened) { + if (flattened.contains(remaining)) { // Done! return found + remaining.flatten(resource.role()); - } else if (flattened <= remaining) { + } else if (remaining.contains(flattened)) { found += resource; total -= resource; remaining -= flattened; @@ -617,7 +623,7 @@ Resources::operator const google::protobuf::RepeatedPtrField<Resource>& () const bool Resources::operator == (const Resources& that) const { - return *this <= that && that <= *this; + return this->contains(that) && that.contains(*this); } @@ -627,18 +633,6 @@ bool Resources::operator != (const Resources& that) const } -bool Resources::operator <= (const Resources& that) const -{ - foreach (const Resource& resource, resources) { - if (!that.contains(resource)) { - return false; - } - } - - return true; -} - - Resources Resources::operator + (const Resource& that) const { Resources result = *this; http://git-wip-us.apache.org/repos/asf/mesos/blob/5fbc6264/src/examples/low_level_scheduler_libprocess.cpp ---------------------------------------------------------------------- diff --git a/src/examples/low_level_scheduler_libprocess.cpp b/src/examples/low_level_scheduler_libprocess.cpp index 7229797..e5267f7 100644 --- a/src/examples/low_level_scheduler_libprocess.cpp +++ b/src/examples/low_level_scheduler_libprocess.cpp @@ -224,7 +224,7 @@ private: // Launch tasks. vector<TaskInfo> tasks; while (tasksLaunched < totalTasks && - TASK_RESOURCES <= remaining.flatten()) { + remaining.flatten().contains(TASK_RESOURCES)) { int taskId = tasksLaunched++; cout << "Launching task " << taskId << " using offer " http://git-wip-us.apache.org/repos/asf/mesos/blob/5fbc6264/src/examples/low_level_scheduler_pthread.cpp ---------------------------------------------------------------------- diff --git a/src/examples/low_level_scheduler_pthread.cpp b/src/examples/low_level_scheduler_pthread.cpp index 2b012a8..4f9a594 100644 --- a/src/examples/low_level_scheduler_pthread.cpp +++ b/src/examples/low_level_scheduler_pthread.cpp @@ -275,7 +275,7 @@ private: // Launch tasks. vector<TaskInfo> tasks; while (tasksLaunched < totalTasks && - TASK_RESOURCES <= remaining.flatten()) { + remaining.flatten().contains(TASK_RESOURCES)) { int taskId = tasksLaunched++; cout << "Launching task " << taskId << " using offer " http://git-wip-us.apache.org/repos/asf/mesos/blob/5fbc6264/src/examples/no_executor_framework.cpp ---------------------------------------------------------------------- diff --git a/src/examples/no_executor_framework.cpp b/src/examples/no_executor_framework.cpp index 9c84e03..5e47cfc 100644 --- a/src/examples/no_executor_framework.cpp +++ b/src/examples/no_executor_framework.cpp @@ -85,7 +85,7 @@ public: // Launch tasks. vector<TaskInfo> tasks; while (tasksLaunched < totalTasks && - TASK_RESOURCES <= remaining.flatten()) { + remaining.flatten().contains(TASK_RESOURCES)) { int taskId = tasksLaunched++; cout << "Launching task " << taskId << " using offer " http://git-wip-us.apache.org/repos/asf/mesos/blob/5fbc6264/src/examples/test_framework.cpp ---------------------------------------------------------------------- diff --git a/src/examples/test_framework.cpp b/src/examples/test_framework.cpp index ce1616d..e5ec3b9 100644 --- a/src/examples/test_framework.cpp +++ b/src/examples/test_framework.cpp @@ -92,7 +92,7 @@ public: // Launch tasks. vector<TaskInfo> tasks; while (tasksLaunched < totalTasks && - TASK_RESOURCES <= remaining.flatten()) { + remaining.flatten().contains(TASK_RESOURCES)) { int taskId = tasksLaunched++; cout << "Launching task " << taskId << " using offer " http://git-wip-us.apache.org/repos/asf/mesos/blob/5fbc6264/src/master/hierarchical_allocator_process.hpp ---------------------------------------------------------------------- diff --git a/src/master/hierarchical_allocator_process.hpp b/src/master/hierarchical_allocator_process.hpp index 45d45b8..e631b60 100644 --- a/src/master/hierarchical_allocator_process.hpp +++ b/src/master/hierarchical_allocator_process.hpp @@ -246,10 +246,10 @@ public: const process::Timeout& _timeout) : slaveId(_slaveId), resources(_resources), timeout(_timeout) {} - virtual bool filter(const SlaveID& slaveId, const Resources& resources) + virtual bool filter(const SlaveID& _slaveId, const Resources& _resources) { - return slaveId == this->slaveId && - resources <= this->resources && // Refused resources are superset. + return slaveId == _slaveId && + resources.contains(_resources) && // Refused resources are superset. timeout.remaining() > Seconds(0); } http://git-wip-us.apache.org/repos/asf/mesos/blob/5fbc6264/src/master/master.cpp ---------------------------------------------------------------------- diff --git a/src/master/master.cpp b/src/master/master.cpp index 7ab25b3..de42f8e 100644 --- a/src/master/master.cpp +++ b/src/master/master.cpp @@ -2026,7 +2026,7 @@ struct ResourceUsageChecker : TaskInfoVisitor resources += task.executor().resources(); } - if (!(resources + usedResources <= totalResources)) { + if (!totalResources.contains(resources + usedResources)) { return Error( "Task uses more resources " + stringify(resources) + " than available " + stringify(totalResources - usedResources)); http://git-wip-us.apache.org/repos/asf/mesos/blob/5fbc6264/src/tests/mesos.hpp ---------------------------------------------------------------------- diff --git a/src/tests/mesos.hpp b/src/tests/mesos.hpp index 0a5e8ec..f132c6c 100644 --- a/src/tests/mesos.hpp +++ b/src/tests/mesos.hpp @@ -406,7 +406,8 @@ ACTION_P5(LaunchTasks, executor, tasks, cpus, mem, role) std::vector<TaskInfo> tasks; Resources remaining = offer.resources(); - while (TASK_RESOURCES <= remaining.flatten() && launched < numTasks) { + while (remaining.flatten().contains(TASK_RESOURCES) && + launched < numTasks) { TaskInfo task; task.set_name("TestTask"); task.mutable_task_id()->set_value(stringify(nextTaskId++)); http://git-wip-us.apache.org/repos/asf/mesos/blob/5fbc6264/src/tests/resources_tests.cpp ---------------------------------------------------------------------- diff --git a/src/tests/resources_tests.cpp b/src/tests/resources_tests.cpp index c0486f4..73f50ba 100644 --- a/src/tests/resources_tests.cpp +++ b/src/tests/resources_tests.cpp @@ -243,8 +243,8 @@ TEST(ResourcesTest, ScalarSubset) r2 += cpus2; r2 += mem2; - EXPECT_TRUE(r1 <= r2); - EXPECT_FALSE(r2 <= r1); + EXPECT_TRUE(r2.contains(r1)); + EXPECT_FALSE(r1.contains(r2)); } @@ -259,18 +259,18 @@ TEST(ResourcesTest, ScalarSubset2) Resources r2; r2 += cpus2; - EXPECT_FALSE(r1 <= r2); - EXPECT_FALSE(r2 <= r1); + EXPECT_FALSE(r2.contains(r1)); + EXPECT_FALSE(r1.contains(r2)); Resource cpus3 = Resources::parse("cpus", "3", "role1").get(); Resources r3; r3 += cpus3; - EXPECT_FALSE(r3 <= r1); - EXPECT_FALSE(r3 <= r2); - EXPECT_FALSE(r2 <= r3); - EXPECT_LE(r1, r3); + EXPECT_FALSE(r1.contains(r3)); + EXPECT_FALSE(r2.contains(r3)); + EXPECT_FALSE(r3.contains(r2)); + EXPECT_TRUE(r3.contains(r1)); } @@ -429,16 +429,16 @@ TEST(ResourcesTest, RangesSubset) Resources r5; r5 += ports5; - EXPECT_TRUE(r1 <= r2); - EXPECT_FALSE(r2 <= r1); - EXPECT_FALSE(r1 <= r3); - EXPECT_FALSE(r3 <= r1); - EXPECT_TRUE(r3 <= r2); - EXPECT_FALSE(r2 <= r3); - EXPECT_TRUE(r1 <= r4); - EXPECT_TRUE(r4 <= r2); - EXPECT_TRUE(r1 <= r5); - EXPECT_FALSE(r5 <= r1); + EXPECT_TRUE(r2.contains(r1)); + EXPECT_FALSE(r1.contains(r2)); + EXPECT_FALSE(r3.contains(r1)); + EXPECT_FALSE(r1.contains(r3)); + EXPECT_TRUE(r2.contains(r3)); + EXPECT_FALSE(r3.contains(r2)); + EXPECT_TRUE(r4.contains(r1)); + EXPECT_TRUE(r2.contains(r4)); + EXPECT_TRUE(r5.contains(r1)); + EXPECT_FALSE(r1.contains(r5)); } @@ -687,8 +687,8 @@ TEST(ResourcesTest, SetSubset) EXPECT_FALSE(r1.empty()); EXPECT_FALSE(r2.empty()); - EXPECT_TRUE(r1 <= r2); - EXPECT_FALSE(r2 <= r1); + EXPECT_TRUE(r2.contains(r1)); + EXPECT_FALSE(r1.contains(r2)); } http://git-wip-us.apache.org/repos/asf/mesos/blob/5fbc6264/src/tests/slave_recovery_tests.cpp ---------------------------------------------------------------------- diff --git a/src/tests/slave_recovery_tests.cpp b/src/tests/slave_recovery_tests.cpp index 98e059f..782f57a 100644 --- a/src/tests/slave_recovery_tests.cpp +++ b/src/tests/slave_recovery_tests.cpp @@ -1061,8 +1061,9 @@ TYPED_TEST(SlaveRecoveryTest, RemoveNonCheckpointingFramework) Resources::parse("cpus:1;mem:512").get()); tasks.push_back(createTask(offer2, "sleep 1000")); // Long-running task, - ASSERT_LE(Resources(offer1.resources()) + Resources(offer2.resources()), - Resources(offer.resources())); + ASSERT_TRUE(Resources(offer.resources()).contains( + Resources(offer1.resources()) + + Resources(offer2.resources()))); Future<Nothing> update1; Future<Nothing> update2;
