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:48:31 UTC

[1/2] mesos git commit: Fixed crash with tasks that use very small resource values.

Repository: mesos
Updated Branches:
  refs/heads/1.2.x 12890ba05 -> e88229982


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/d6b34409
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/d6b34409
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/d6b34409

Branch: refs/heads/1.2.x
Commit: d6b3440998097ea24437c6fe61dba19c70e4eac7
Parents: 12890ba
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:54 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/d6b34409/src/common/resources.cpp
----------------------------------------------------------------------
diff --git a/src/common/resources.cpp b/src/common/resources.cpp
index 388e3ef..db29cee 100644
--- a/src/common/resources.cpp
+++ b/src/common/resources.cpp
@@ -841,7 +841,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/d6b34409/src/tests/master_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/master_tests.cpp b/src/tests/master_tests.cpp
index 3b4123b..23d81e6 100644
--- a/src/tests/master_tests.cpp
+++ b/src/tests/master_tests.cpp
@@ -6325,6 +6325,53 @@ TEST_F(MasterTest, AgentRestartNoReregisterRateLimit)
   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/d6b34409/src/tests/resources_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/resources_tests.cpp b/src/tests/resources_tests.cpp
index 2bdce3c..3896bd8 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.
@@ -3310,6 +3323,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/d6b34409/src/v1/resources.cpp
----------------------------------------------------------------------
diff --git a/src/v1/resources.cpp b/src/v1/resources.cpp
index e47c4d4..e6197a6 100644
--- a/src/v1/resources.cpp
+++ b/src/v1/resources.cpp
@@ -830,7 +830,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) {


[2/2] mesos git commit: Added MESOS-7197 to CHANGELOG for 1.2.1.

Posted by ne...@apache.org.
Added MESOS-7197 to CHANGELOG for 1.2.1.


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

Branch: refs/heads/1.2.x
Commit: e88229982d169e5a366868534566ab0ce7b241fa
Parents: d6b3440
Author: Neil Conway <ne...@gmail.com>
Authored: Thu Mar 23 14:23:03 2017 -0700
Committer: Neil Conway <ne...@gmail.com>
Committed: Thu Mar 23 15:47:59 2017 -0700

----------------------------------------------------------------------
 CHANGELOG | 1 +
 1 file changed, 1 insertion(+)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/e8822998/CHANGELOG
----------------------------------------------------------------------
diff --git a/CHANGELOG b/CHANGELOG
index 9558c43..501a1b4 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -4,6 +4,7 @@ Release Notes - Mesos - Version 1.2.1 (WIP)
 
 All Issues:
 ** Bug
+  * [MESOS-7197] - Requesting tiny amount of CPU crashes master.
   * [MESOS-7208] - Persistent volume ownership is set to root when task is running with non-root user
   * [MESOS-7237] - Enabling cgroups_limit_swap can lead to "invalid argument" error.
   * [MESOS-7261] - maintenance.html is missing during packaging.