Repository: mesos Updated Branches: refs/heads/1.1.x 1edce53c2 -> 9bc466ec5
Fixed crash with tasks that use very small resource values. When parsing resources, inputs such as "cpus:0" are considered "empty" and are omitted from the resulting `Resources` object. However, a very small resource value like "cpus:0.00001" was considered non-empty, despite the fact that the numerical behavior for resources means that these two values compare as equal. This inconsistency resulted in a `CHECK` failure in the sorter when a task that used a tiny amount of resources reached a terminal state. This commit fixes the resource parsing logic to treat such very small resource values as empty resources, which resolves the inconsistency above. The resulting behavior might be a bit surprising to users (since the task will launch with zero resources instead of a very small resource value), but improving this is left as future work (MESOS-1807). Review: https://reviews.apache.org/r/57752 Project: http://git-wip-us.apache.org/repos/asf/mesos/repo Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/b5e38405 Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/b5e38405 Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/b5e38405 Branch: refs/heads/1.1.x Commit: b5e384050d68521fbdfe92c849f3bdcfb02a0b7b Parents: 1edce53 Author: Neil Conway <[email protected]> Authored: Thu Mar 9 15:46:53 2017 -0500 Committer: Neil Conway <[email protected]> Committed: Thu Mar 23 15:48:44 2017 -0700 ---------------------------------------------------------------------- src/common/resources.cpp | 4 +++- src/tests/master_tests.cpp | 47 ++++++++++++++++++++++++++++++++++++++ src/tests/resources_tests.cpp | 44 +++++++++++++++++++++++++++++++++++ src/v1/resources.cpp | 4 +++- 4 files changed, 97 insertions(+), 2 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mesos/blob/b5e38405/src/common/resources.cpp ---------------------------------------------------------------------- diff --git a/src/common/resources.cpp b/src/common/resources.cpp index 4bb9bef..7c634e6 100644 --- a/src/common/resources.cpp +++ b/src/common/resources.cpp @@ -776,7 +776,9 @@ Option<Error> Resources::validate(const RepeatedPtrField<Resource>& resources) bool Resources::isEmpty(const Resource& resource) { if (resource.type() == Value::SCALAR) { - return resource.scalar().value() == 0; + Value::Scalar zero; + zero.set_value(0); + return resource.scalar() == zero; } else if (resource.type() == Value::RANGES) { return resource.ranges().range_size() == 0; } else if (resource.type() == Value::SET) { http://git-wip-us.apache.org/repos/asf/mesos/blob/b5e38405/src/tests/master_tests.cpp ---------------------------------------------------------------------- diff --git a/src/tests/master_tests.cpp b/src/tests/master_tests.cpp index ac8cce3..f20ed80 100644 --- a/src/tests/master_tests.cpp +++ b/src/tests/master_tests.cpp @@ -5074,6 +5074,53 @@ TEST_F(MasterTest, RecoverResourcesOrphanedTask) .Times(AtMost(1)); } + +TEST_F(MasterTest, TaskWithTinyResources) +{ + Try<Owned<cluster::Master>> master = StartMaster(); + ASSERT_SOME(master); + + Owned<MasterDetector> detector = master.get()->createDetector(); + Try<Owned<cluster::Slave>> slave = StartSlave(detector.get()); + ASSERT_SOME(slave); + + MockScheduler sched; + MesosSchedulerDriver driver( + &sched, DEFAULT_FRAMEWORK_INFO, master.get()->pid, DEFAULT_CREDENTIAL); + + EXPECT_CALL(sched, registered(&driver, _, _)); + + Future<vector<Offer>> offers; + EXPECT_CALL(sched, resourceOffers(&driver, _)) + .WillOnce(FutureArg<1>(&offers)) + .WillRepeatedly(Return()); // Ignore subsequent offers. + + driver.start(); + + AWAIT_READY(offers); + ASSERT_FALSE(offers->empty()); + + Offer offer = offers.get()[0]; + + TaskInfo task = createTask( + offer.slave_id(), + Resources::parse("cpus:0.00001;mem:1").get(), + SLEEP_COMMAND(1000)); + + Future<TaskStatus> runningStatus; + EXPECT_CALL(sched, statusUpdate(&driver, _)) + .WillOnce(FutureArg<1>(&runningStatus)); + + driver.launchTasks(offer.id(), {task}); + + AWAIT_READY(runningStatus); + EXPECT_EQ(TASK_RUNNING, runningStatus->state()); + EXPECT_EQ(task.task_id(), runningStatus->task_id()); + + driver.stop(); + driver.join(); +} + } // namespace tests { } // namespace internal { } // namespace mesos { http://git-wip-us.apache.org/repos/asf/mesos/blob/b5e38405/src/tests/resources_tests.cpp ---------------------------------------------------------------------- diff --git a/src/tests/resources_tests.cpp b/src/tests/resources_tests.cpp index 6a12783..cd05c16 100644 --- a/src/tests/resources_tests.cpp +++ b/src/tests/resources_tests.cpp @@ -1685,6 +1685,19 @@ TEST(ResourcesTest, PrecisionRounding) } +TEST(ResourcesTest, PrecisionVerySmallValue) +{ + Resources r1 = Resources::parse("cpus:2;mem:1024").get(); + Resources r2 = Resources::parse("cpus:0.00001;mem:1").get(); + + Resources r3 = r1 - (r1 - r2); + EXPECT_TRUE(r3.contains(r2)); + + Resources r4 = Resources::parse("cpus:0;mem:1").get(); + EXPECT_EQ(r2, r4); +} + + TEST(ReservedResourcesTest, Validation) { // Unreserved. @@ -2954,6 +2967,37 @@ TEST_F(Resources_Filter_BENCHMARK_Test, Filters) << " on " << stringify(reserved) << endl; } + +class Resources_Parse_BENCHMARK_Test + : public MesosTest, + public ::testing::WithParamInterface<size_t> {}; + + +INSTANTIATE_TEST_CASE_P( + Resources_Parse, + Resources_Parse_BENCHMARK_Test, + ::testing::Values(1000U, 10000U, 50000U)); + + +TEST_P(Resources_Parse_BENCHMARK_Test, Parse) +{ + const size_t iterationCount = GetParam(); + const size_t resourcesCount = 100; + + vector<string> rawResources; + + for (size_t i = 0; i < resourcesCount; i++) { + rawResources.push_back("res" + stringify(i) + ":" + stringify(i)); + } + + string inputString = strings::join(";", rawResources); + + for (size_t i = 0; i < iterationCount; i++) { + Try<Resources> resource = Resources::parse(inputString); + EXPECT_SOME(resource); + } +} + } // namespace tests { } // namespace internal { } // namespace mesos { http://git-wip-us.apache.org/repos/asf/mesos/blob/b5e38405/src/v1/resources.cpp ---------------------------------------------------------------------- diff --git a/src/v1/resources.cpp b/src/v1/resources.cpp index 46cc00f..bdf115d 100644 --- a/src/v1/resources.cpp +++ b/src/v1/resources.cpp @@ -776,7 +776,9 @@ Option<Error> Resources::validate(const RepeatedPtrField<Resource>& resources) bool Resources::isEmpty(const Resource& resource) { if (resource.type() == Value::SCALAR) { - return resource.scalar().value() == 0; + Value::Scalar zero; + zero.set_value(0); + return resource.scalar() == zero; } else if (resource.type() == Value::RANGES) { return resource.ranges().range_size() == 0; } else if (resource.type() == Value::SET) {
