You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by ne...@apache.org on 2017/03/23 22:47:34 UTC
mesos git commit: Fixed crash with tasks that use very small resource
values.
Repository: mesos
Updated Branches:
refs/heads/master 322300ff0 -> 904092c39
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/904092c3
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/904092c3
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/904092c3
Branch: refs/heads/master
Commit: 904092c399402563ae456f7cb89a3da112496d98
Parents: 322300f
Author: Neil Conway <ne...@gmail.com>
Authored: Thu Mar 9 15:46:53 2017 -0500
Committer: Neil Conway <ne...@gmail.com>
Committed: Thu Mar 23 15:47:28 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/904092c3/src/common/resources.cpp
----------------------------------------------------------------------
diff --git a/src/common/resources.cpp b/src/common/resources.cpp
index b64fd76..ca1add1 100644
--- a/src/common/resources.cpp
+++ b/src/common/resources.cpp
@@ -838,7 +838,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/904092c3/src/tests/master_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/master_tests.cpp b/src/tests/master_tests.cpp
index 5fdb949..d1828eb 100644
--- a/src/tests/master_tests.cpp
+++ b/src/tests/master_tests.cpp
@@ -6732,6 +6732,53 @@ TEST_F(MasterTest, MultiRoleFrameworkReceivesOffers)
driver.join();
}
+
+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/904092c3/src/tests/resources_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/resources_tests.cpp b/src/tests/resources_tests.cpp
index 18e8b67..343cab2 100644
--- a/src/tests/resources_tests.cpp
+++ b/src/tests/resources_tests.cpp
@@ -1701,6 +1701,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.
@@ -3233,6 +3246,37 @@ TEST_P(Resources_Contains_BENCHMARK_Test, Contains)
<< 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/904092c3/src/v1/resources.cpp
----------------------------------------------------------------------
diff --git a/src/v1/resources.cpp b/src/v1/resources.cpp
index 4ffe950..6008a9d 100644
--- a/src/v1/resources.cpp
+++ b/src/v1/resources.cpp
@@ -840,7 +840,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) {