You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by mz...@apache.org on 2019/01/09 02:26:21 UTC

[mesos] branch 1.6.x updated (a092c2c -> 360e002)

This is an automated email from the ASF dual-hosted git repository.

mzhu pushed a change to branch 1.6.x
in repository https://gitbox.apache.org/repos/asf/mesos.git.


    from a092c2c  Sent SIGKILL to I/O switchboard server as a safeguard.
     new 9883e06  Disallowed nan, inf and so on when parsing Value::Scalar.
     new 2762d74  Pulled up the resource quantities class for more general use.
     new 8c6f52a  Added tests for `ResourceQuantities`.
     new 610f9c6  Added a `Resources` method `contains(ResourceQuantities)`.
     new 360e002  Extended `min_allocatable_resources` flag to cover non-scalar resources.

The 5 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 docs/configuration/master.md                  |  22 ++--
 include/mesos/allocator/allocator.hpp         |   4 +-
 include/mesos/resources.hpp                   |  11 ++
 include/mesos/v1/resources.hpp                |  11 ++
 src/CMakeLists.txt                            |   1 +
 src/Makefile.am                               |   3 +
 src/common/resource_quantities.cpp            | 138 +++++++++++++++++++++
 src/common/resource_quantities.hpp            | 127 +++++++++++++++++++
 src/common/resources.cpp                      |  44 +++++++
 src/common/values.cpp                         |  15 +++
 src/master/allocator/mesos/allocator.hpp      |  25 ++--
 src/master/allocator/mesos/hierarchical.cpp   |  11 +-
 src/master/allocator/mesos/hierarchical.hpp   |   5 +-
 src/master/allocator/sorter/drf/sorter.cpp    |  14 ++-
 src/master/allocator/sorter/drf/sorter.hpp    |   2 +
 src/master/allocator/sorter/random/sorter.hpp |   2 +
 src/master/flags.cpp                          |  22 ++--
 src/master/master.cpp                         |  54 ++++----
 src/tests/CMakeLists.txt                      |   1 +
 src/tests/allocator.hpp                       |   2 +-
 src/tests/hierarchical_allocator_tests.cpp    |  13 +-
 src/tests/master_allocator_tests.cpp          | 106 ++++++++++++++++
 src/tests/resource_quantities_tests.cpp       | 171 ++++++++++++++++++++++++++
 src/tests/resources_tests.cpp                 |  96 +++++++++++++++
 src/tests/values_tests.cpp                    |   9 ++
 src/v1/resources.cpp                          |  44 +++++++
 src/v1/values.cpp                             |  15 +++
 27 files changed, 890 insertions(+), 78 deletions(-)
 create mode 100644 src/common/resource_quantities.cpp
 create mode 100644 src/common/resource_quantities.hpp
 create mode 100644 src/tests/resource_quantities_tests.cpp


[mesos] 03/05: Added tests for `ResourceQuantities`.

Posted by mz...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

mzhu pushed a commit to branch 1.6.x
in repository https://gitbox.apache.org/repos/asf/mesos.git

commit 8c6f52a98ef6065d123d23cccf6e03de77c5be8a
Author: Meng Zhu <mz...@mesosphere.io>
AuthorDate: Fri Jan 4 11:13:21 2019 -0800

    Added tests for `ResourceQuantities`.
    
    Added parsing and insertion tests for `ResourceQuantities`.
    
    Review: https://reviews.apache.org/r/69600
---
 src/Makefile.am                         |   1 +
 src/tests/CMakeLists.txt                |   1 +
 src/tests/resource_quantities_tests.cpp | 171 ++++++++++++++++++++++++++++++++
 3 files changed, 173 insertions(+)

diff --git a/src/Makefile.am b/src/Makefile.am
index 9499a20..1485229 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -2543,6 +2543,7 @@ mesos_tests_SOURCES =						\
   tests/resource_offers_tests.cpp				\
   tests/resource_provider_manager_tests.cpp			\
   tests/resource_provider_validation_tests.cpp			\
+  tests/resource_quantities_tests.cpp			        \
   tests/resources_tests.cpp					\
   tests/resources_utils.cpp					\
   tests/role_tests.cpp						\
diff --git a/src/tests/CMakeLists.txt b/src/tests/CMakeLists.txt
index 1fef060..758bb94 100644
--- a/src/tests/CMakeLists.txt
+++ b/src/tests/CMakeLists.txt
@@ -117,6 +117,7 @@ set(MESOS_TESTS_SRC
   resource_offers_tests.cpp
   resource_provider_manager_tests.cpp
   resource_provider_validation_tests.cpp
+  resource_quantities_tests.cpp
   resources_tests.cpp
   role_tests.cpp
   scheduler_driver_tests.cpp
diff --git a/src/tests/resource_quantities_tests.cpp b/src/tests/resource_quantities_tests.cpp
new file mode 100644
index 0000000..435a494
--- /dev/null
+++ b/src/tests/resource_quantities_tests.cpp
@@ -0,0 +1,171 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include <string>
+#include <vector>
+
+#include <gtest/gtest.h>
+
+#include <stout/check.hpp>
+#include <stout/gtest.hpp>
+#include <stout/try.hpp>
+
+#include <mesos/values.hpp>
+
+#include "common/resource_quantities.hpp"
+
+using std::pair;
+using std::string;
+using std::vector;
+
+using mesos::internal::ResourceQuantities;
+
+namespace mesos {
+namespace internal {
+namespace tests {
+
+
+TEST(QuantitiesTest, FromStringValid)
+{
+  // A single resource.
+  ResourceQuantities resourceQuantities =
+    CHECK_NOTERROR(ResourceQuantities::fromString("cpus:10"));
+  EXPECT_EQ(1u, resourceQuantities.size());
+  EXPECT_EQ("cpus", resourceQuantities.begin()->first);
+  EXPECT_DOUBLE_EQ(10, resourceQuantities.begin()->second.value());
+
+  resourceQuantities =
+    CHECK_NOTERROR(ResourceQuantities::fromString("cpus:3.14"));
+  EXPECT_EQ(1u, resourceQuantities.size());
+  EXPECT_EQ("cpus", resourceQuantities.begin()->first);
+  EXPECT_DOUBLE_EQ(3.14, resourceQuantities.begin()->second.value());
+
+  // Zero value is retained.
+  resourceQuantities =
+    CHECK_NOTERROR(ResourceQuantities::fromString("cpus:0"));
+  EXPECT_EQ(1u, resourceQuantities.size());
+  EXPECT_EQ("cpus", resourceQuantities.begin()->first);
+  EXPECT_DOUBLE_EQ(0, resourceQuantities.begin()->second.value());
+
+  // Whitespace is trimmed.
+  resourceQuantities =
+    CHECK_NOTERROR(ResourceQuantities::fromString(" cpus : 3.14 ; disk : 10 "));
+  EXPECT_EQ(2u, resourceQuantities.size());
+  auto it = resourceQuantities.begin();
+  EXPECT_EQ("cpus", it->first);
+  EXPECT_DOUBLE_EQ(3.14, it->second.value());
+  ++it;
+  EXPECT_EQ("disk", it->first);
+  EXPECT_DOUBLE_EQ(10, it->second.value());
+
+  // Two resources.
+  resourceQuantities =
+    CHECK_NOTERROR(ResourceQuantities::fromString("cpus:10.5;ports:1"));
+  EXPECT_EQ(2u, resourceQuantities.size());
+  it = resourceQuantities.begin();
+  EXPECT_EQ("cpus", it->first);
+  EXPECT_DOUBLE_EQ(10.5, it->second.value());
+  ++it;
+  EXPECT_EQ("ports", it->first);
+  EXPECT_DOUBLE_EQ(1, it->second.value());
+
+  // Two resources with names out of alphabetical order.
+  resourceQuantities =
+    CHECK_NOTERROR(ResourceQuantities::fromString("ports:3;cpus:10.5"));
+  EXPECT_EQ(2u, resourceQuantities.size());
+  it = resourceQuantities.begin();
+  EXPECT_EQ("cpus", it->first);
+  EXPECT_DOUBLE_EQ(10.5, it->second.value());
+  ++it;
+  EXPECT_EQ("ports", it->first);
+  EXPECT_DOUBLE_EQ(3, it->second.value());
+
+  // Duplicate resource names.
+  resourceQuantities =
+    CHECK_NOTERROR(ResourceQuantities::fromString("ports:3;cpus:1;cpus:10.5"));
+  EXPECT_EQ(2u, resourceQuantities.size());
+  it = resourceQuantities.begin();
+  EXPECT_EQ("cpus", it->first);
+  EXPECT_DOUBLE_EQ(11.5, it->second.value());
+  ++it;
+  EXPECT_EQ("ports", it->first);
+  EXPECT_DOUBLE_EQ(3, it->second.value());
+}
+
+
+TEST(QuantitiesTest, FromStringInvalid)
+{
+  // Invalid scalar.
+  Try<ResourceQuantities> resourceQuantities =
+    ResourceQuantities::fromString("cpus:a10");
+  EXPECT_ERROR(resourceQuantities);
+
+  resourceQuantities = ResourceQuantities::fromString("cpus:3.14c");
+  EXPECT_ERROR(resourceQuantities);
+
+  // Missing semicolon.
+  resourceQuantities = ResourceQuantities::fromString("ports:3,cpus:1");
+  EXPECT_ERROR(resourceQuantities);
+
+  // Negative value.
+  resourceQuantities = ResourceQuantities::fromString("ports:3,cpus:-1");
+  EXPECT_ERROR(resourceQuantities);
+
+  resourceQuantities = ResourceQuantities::fromString("cpus:nan");
+  EXPECT_ERROR(resourceQuantities);
+
+  resourceQuantities = ResourceQuantities::fromString("cpus:-nan");
+  EXPECT_ERROR(resourceQuantities);
+
+  resourceQuantities = ResourceQuantities::fromString("cpus:inf");
+  EXPECT_ERROR(resourceQuantities);
+
+  resourceQuantities = ResourceQuantities::fromString("cpus:-inf");
+  EXPECT_ERROR(resourceQuantities);
+
+  resourceQuantities = ResourceQuantities::fromString("cpus:infinity");
+  EXPECT_ERROR(resourceQuantities);
+
+  resourceQuantities = ResourceQuantities::fromString("cpus:-infinity");
+  EXPECT_ERROR(resourceQuantities);
+}
+
+
+TEST(QuantitiesTest, Insertion)
+{
+  ResourceQuantities resourceQuantities =
+    CHECK_NOTERROR(ResourceQuantities::fromString("cpus:10"));
+
+  resourceQuantities["disk"] = Value::Scalar();
+  resourceQuantities["alpha"] = Value::Scalar();
+
+  auto it = resourceQuantities.begin();
+  EXPECT_EQ("alpha", it->first);
+  EXPECT_DOUBLE_EQ(0, it->second.value());
+
+  ++it;
+  EXPECT_EQ("cpus", it->first);
+  EXPECT_DOUBLE_EQ(10, it->second.value());
+
+  ++it;
+  EXPECT_EQ("disk", it->first);
+  EXPECT_DOUBLE_EQ(0, it->second.value());
+}
+
+
+} // namespace tests {
+} // namespace internal {
+} // namespace mesos {


[mesos] 01/05: Disallowed nan, inf and so on when parsing Value::Scalar.

Posted by mz...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

mzhu pushed a commit to branch 1.6.x
in repository https://gitbox.apache.org/repos/asf/mesos.git

commit 9883e068dd631abb5d5e33c2ef47797d517b0efb
Author: Meng Zhu <mz...@mesosphere.io>
AuthorDate: Sat Jan 5 13:28:27 2019 -0800

    Disallowed nan, inf and so on when parsing Value::Scalar.
    
    Scalar values are intended to be finite numbers. This
    patch checks `nan`, `inf` and so on when parsing
    `Value::Scalar`. Only normal or zero numbers (as defined
    in `std::fpclassify()`) are allowed.
    
    Also added related tests.
    
    Review: https://reviews.apache.org/r/69673
---
 src/common/values.cpp      | 15 +++++++++++++++
 src/tests/values_tests.cpp |  9 +++++++++
 src/v1/values.cpp          | 15 +++++++++++++++
 3 files changed, 39 insertions(+)

diff --git a/src/common/values.cpp b/src/common/values.cpp
index afe4137..648967b 100644
--- a/src/common/values.cpp
+++ b/src/common/values.cpp
@@ -612,6 +612,21 @@ Try<Value> parse(const string& text)
     } else if (index == string::npos) {
       Try<double> value_ = numify<double>(temp);
       if (!value_.isError()) {
+        Option<Error> error = [value_]() -> Option<Error> {
+          switch (std::fpclassify(value_.get())) {
+            case FP_NORMAL:     return None();
+            case FP_ZERO:       return None();
+            case FP_INFINITE:   return Error("Infinite values not supported");
+            case FP_NAN:        return Error("NaN not supported");
+            case FP_SUBNORMAL:  return Error("Subnormal values not supported");
+            default:            return Error("Unknown error");
+          }
+        }();
+
+        if (error.isSome()) {
+          return Error("Invalid scalar value '" + temp + "':" + error->message);
+        }
+
         // This is a scalar.
         Value::Scalar* scalar = value.mutable_scalar();
         value.set_type(Value::SCALAR);
diff --git a/src/tests/values_tests.cpp b/src/tests/values_tests.cpp
index 2f5abed..fc853a1 100644
--- a/src/tests/values_tests.cpp
+++ b/src/tests/values_tests.cpp
@@ -89,6 +89,15 @@ TEST(ValuesTest, InvalidInput)
 
   // Test when giving empty string.
   EXPECT_ERROR(parse("  "));
+
+  EXPECT_ERROR(parse("nan"));
+  EXPECT_ERROR(parse("-nan"));
+
+  EXPECT_ERROR(parse("inf"));
+  EXPECT_ERROR(parse("-inf"));
+
+  EXPECT_ERROR(parse("infinity"));
+  EXPECT_ERROR(parse("-infinity"));
 }
 
 
diff --git a/src/v1/values.cpp b/src/v1/values.cpp
index d2c31f6..c047404 100644
--- a/src/v1/values.cpp
+++ b/src/v1/values.cpp
@@ -640,6 +640,21 @@ Try<Value> parse(const string& text)
     } else if (index == string::npos) {
       Try<double> value_ = numify<double>(temp);
       if (!value_.isError()) {
+        Option<Error> error = [value_]() -> Option<Error> {
+          switch (std::fpclassify(value_.get())) {
+            case FP_NORMAL:     return None();
+            case FP_ZERO:       return None();
+            case FP_INFINITE:   return Error("Infinite values not supported");
+            case FP_NAN:        return Error("NaN not supported");
+            case FP_SUBNORMAL:  return Error("Subnormal values not supported");
+            default:            return Error("Unknown error");
+          }
+        }();
+
+        if (error.isSome()) {
+          return Error("Invalid scalar value '" + temp + "':" + error->message);
+        }
+
         // This is a scalar.
         Value::Scalar* scalar = value.mutable_scalar();
         value.set_type(Value::SCALAR);


[mesos] 05/05: Extended `min_allocatable_resources` flag to cover non-scalar resources.

Posted by mz...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

mzhu pushed a commit to branch 1.6.x
in repository https://gitbox.apache.org/repos/asf/mesos.git

commit 360e0023fc89d64d67e3a5effc6faa28923818b8
Author: Meng Zhu <mz...@mesosphere.io>
AuthorDate: Sun Dec 30 20:55:06 2018 -0800

    Extended `min_allocatable_resources` flag to cover non-scalar resources.
    
    The master flag min_allocatable_resources currently only supports
    scalar resources. Sometimes it is desirable to control the quantities
    of non-scalar resources in the offer as well, for example, to
    ensure offers always contain port resources.
    
    This patch extended the flag to cover non-scalar resources.
    For RANGES and SET type resources, their quantities are the
    number of different instances in the range or set. For example,
    range:[1-5] has a quantity of 5 and set:{a,b} has a quantity of 2.
    
    This patch also updated affected tests and modified the existing
    test for min_allocatable_resources to cover the updated use cases.
    
    Review: https://reviews.apache.org/r/69603
---
 docs/configuration/master.md                |  22 +++---
 include/mesos/allocator/allocator.hpp       |   4 +-
 src/master/allocator/mesos/allocator.hpp    |  25 +++----
 src/master/allocator/mesos/hierarchical.cpp |  11 +--
 src/master/allocator/mesos/hierarchical.hpp |   5 +-
 src/master/flags.cpp                        |  22 +++---
 src/master/master.cpp                       |  54 +++++++-------
 src/tests/allocator.hpp                     |   2 +-
 src/tests/hierarchical_allocator_tests.cpp  |  13 ++--
 src/tests/master_allocator_tests.cpp        | 106 ++++++++++++++++++++++++++++
 10 files changed, 192 insertions(+), 72 deletions(-)

diff --git a/docs/configuration/master.md b/docs/configuration/master.md
index eac505b..528ec14 100644
--- a/docs/configuration/master.md
+++ b/docs/configuration/master.md
@@ -197,15 +197,19 @@ load an alternate allocator module using <code>--modules</code>.
     --min_allocatable_resources=VALUE
   </td>
   <td>
-One or more sets of resources that define the minimum allocatable
-resources for the allocator. The allocator will only offer resources
-that contain at least one of the specified sets. The resources in each
-set should be delimited by semicolons, and the sets should be delimited
-by the pipe character.
-(Example: <code>disk:1|cpu:1;mem:32</code>
-configures the allocator to only offer resources if they contain a disk
-resource of at least 1 megabyte, or if they contain both 1 cpu and
-32 megabytes of memory.) (default: cpus:0.01|mem:32).
+One or more sets of resource quantities that define the minimum allocatable
+resource for the allocator. The allocator will only offer resources that
+meets the quantity requirement of at least one of the specified sets. For
+`SCALAR` type resources, its quantity is its scalar value. For `RANGES` and
+`SET` type, their quantities are the number of different instances in the
+range or set. For example, `range:[1-5]` has a quantity of 5 and `set:{a,b}`
+has a quantity of 2. The resources in each set should be delimited by
+semicolons (acting as logical AND), and each set should be delimited by the
+pipe character (acting as logical OR).
+(Example: `disk:1|cpu:1;mem:32;port:1` configures the allocator to only offer
+resources if they contain a disk resource of at least 1 megabyte, or if they
+at least contain 1 cpu, 32 megabytes of memory and 1 port.)
+(default: cpus:0.01|mem:32).
   </td>
 </tr>
 
diff --git a/include/mesos/allocator/allocator.hpp b/include/mesos/allocator/allocator.hpp
index c19ab64..df64faf 100644
--- a/include/mesos/allocator/allocator.hpp
+++ b/include/mesos/allocator/allocator.hpp
@@ -38,6 +38,8 @@
 #include <stout/option.hpp>
 #include <stout/try.hpp>
 
+#include "common/resource_quantities.hpp"
+
 namespace mesos {
 namespace allocator {
 
@@ -102,7 +104,7 @@ public:
         fairnessExcludeResourceNames = None(),
       bool filterGpuResources = true,
       const Option<DomainInfo>& domain = None(),
-      const Option<std::vector<Resources>>&
+      const Option<std::vector<mesos::internal::ResourceQuantities>>&
         minAllocatableResources = None()) = 0;
 
   /**
diff --git a/src/master/allocator/mesos/allocator.hpp b/src/master/allocator/mesos/allocator.hpp
index 900c8ee..32dd40c 100644
--- a/src/master/allocator/mesos/allocator.hpp
+++ b/src/master/allocator/mesos/allocator.hpp
@@ -48,19 +48,19 @@ public:
 
   void initialize(
       const Duration& allocationInterval,
-      const lambda::function<
-          void(const FrameworkID&,
-               const hashmap<std::string, hashmap<SlaveID, Resources>>&)>&
-                   offerCallback,
-      const lambda::function<
-          void(const FrameworkID&,
-               const hashmap<SlaveID, UnavailableResources>&)>&
+      const lambda::function<void(
+          const FrameworkID&,
+          const hashmap<std::string, hashmap<SlaveID, Resources>>&)>&
+        offerCallback,
+      const lambda::function<void(
+          const FrameworkID&, const hashmap<SlaveID, UnavailableResources>&)>&
         inverseOfferCallback,
-      const Option<std::set<std::string>>&
-        fairnessExcludeResourceNames = None(),
+      const Option<std::set<std::string>>& fairnessExcludeResourceNames =
+        None(),
       bool filterGpuResources = true,
       const Option<DomainInfo>& domain = None(),
-      const Option<std::vector<Resources>>& minAllocatableResources = None());
+      const Option<std::vector<mesos::internal::ResourceQuantities>>&
+        minAllocatableResources = None());
 
   void recover(
       const int expectedAgentCount,
@@ -207,7 +207,7 @@ public:
         fairnessExcludeResourceNames = None(),
       bool filterGpuResources = true,
       const Option<DomainInfo>& domain = None(),
-      const Option<std::vector<Resources>>&
+      const Option<std::vector<mesos::internal::ResourceQuantities>>&
         minAllocatableResources = None()) = 0;
 
   virtual void recover(
@@ -364,7 +364,8 @@ inline void MesosAllocator<AllocatorProcess>::initialize(
     const Option<std::set<std::string>>& fairnessExcludeResourceNames,
     bool filterGpuResources,
     const Option<DomainInfo>& domain,
-    const Option<std::vector<Resources>>& minAllocatableResources)
+    const Option<std::vector<mesos::internal::ResourceQuantities>>&
+      minAllocatableResources)
 {
   process::dispatch(
       process,
diff --git a/src/master/allocator/mesos/hierarchical.cpp b/src/master/allocator/mesos/hierarchical.cpp
index f6aaa45..e63f886 100644
--- a/src/master/allocator/mesos/hierarchical.cpp
+++ b/src/master/allocator/mesos/hierarchical.cpp
@@ -41,6 +41,7 @@
 #include <stout/stringify.hpp>
 
 #include "common/protobuf_utils.hpp"
+#include "common/resource_quantities.hpp"
 
 using std::set;
 using std::string;
@@ -156,7 +157,8 @@ void HierarchicalAllocatorProcess::initialize(
     const Option<set<string>>& _fairnessExcludeResourceNames,
     bool _filterGpuResources,
     const Option<DomainInfo>& _domain,
-    const Option<std::vector<Resources>>& _minAllocatableResources)
+    const Option<std::vector<mesos::internal::ResourceQuantities>>&
+      _minAllocatableResources)
 {
   allocationInterval = _allocationInterval;
   offerCallback = _offerCallback;
@@ -2457,11 +2459,10 @@ bool HierarchicalAllocatorProcess::allocatable(const Resources& resources)
     return true;
   }
 
-  // We remove the static reservation metadata here via `toUnreserved()`.
-  Resources quantity = resources.createStrippedScalarQuantity().toUnreserved();
   foreach (
-      const Resources& minResources, CHECK_NOTNONE(minAllocatableResources)) {
-    if (quantity.contains(minResources)) {
+      const ResourceQuantities& resourceQuantities,
+      CHECK_NOTNONE(minAllocatableResources)) {
+    if (resources.contains(resourceQuantities)) {
       return true;
     }
   }
diff --git a/src/master/allocator/mesos/hierarchical.hpp b/src/master/allocator/mesos/hierarchical.hpp
index 0f6c0e9..396f201 100644
--- a/src/master/allocator/mesos/hierarchical.hpp
+++ b/src/master/allocator/mesos/hierarchical.hpp
@@ -112,7 +112,7 @@ public:
         fairnessExcludeResourceNames = None(),
       bool filterGpuResources = true,
       const Option<DomainInfo>& domain = None(),
-      const Option<std::vector<Resources>>&
+      const Option<std::vector<mesos::internal::ResourceQuantities>>&
         minAllocatableResources = None());
 
   void recover(
@@ -526,7 +526,8 @@ protected:
   Option<DomainInfo> domain;
 
   // The minimum allocatable resources, if any.
-  Option<std::vector<Resources>> minAllocatableResources;
+  Option<std::vector<mesos::internal::ResourceQuantities>>
+    minAllocatableResources;
 
   // There are two stages of allocation:
   //
diff --git a/src/master/flags.cpp b/src/master/flags.cpp
index 6ad53ed..38ba888 100644
--- a/src/master/flags.cpp
+++ b/src/master/flags.cpp
@@ -488,14 +488,20 @@ mesos::internal::master::Flags::Flags()
 
   add(&Flags::min_allocatable_resources,
       "min_allocatable_resources",
-      "One or more sets of resources that define the minimum allocatable\n"
-      "resources for the allocator. The allocator will only offer resources\n"
-      "that contain at least one of the specified sets. The resources in\n"
-      "each set should be delimited by semicolons, and the sets should be\n"
-      "delimited by the pipe character.\n"
-      "(Example: `disk:1|cpu:1;mem:32` configures the allocator to only offer\n"
-      "resources if they contain a disk resource of at least 1 megabyte, or\n"
-      "if they contain both 1 cpu and 32 megabytes of memory.)\n",
+      "One or more sets of resource quantities that define the minimum\n"
+      "allocatable resources for the allocator. The allocator will only offer\n"
+      "resources that meets the quantity requirement of at least one of the\n"
+      "specified sets. For `SCALAR` type resources, its quantity is its\n"
+      "scalar value. For `RANGES` and `SET` type, their quantities are the\n"
+      "number of different instances in the range or set. For example,\n"
+      "`range:[1-5]` has a quantity of 5 and `set:{a,b}` has a quantity of 2.\n"
+      "The resources in each set should be delimited by semicolons (acting as\n"
+      "logical AND), and each set should be delimited by the pipe character\n"
+      "(acting as logical OR).\n"
+      "(Example: `disk:1|cpu:1;mem:32;port:1` configures the allocator to\n"
+      "only offer resources if they contain a disk resource of at least\n"
+      "1 megabyte, or if they at least contain 1 cpu, 32 megabytes of memory\n"
+      "and 1 port.)\n",
       "cpus:" + stringify(MIN_CPUS) +
         "|mem:" + stringify((double)MIN_MEM.bytes() / Bytes::MEGABYTES));
 
diff --git a/src/master/master.cpp b/src/master/master.cpp
index 2aaff39..8776663 100644
--- a/src/master/master.cpp
+++ b/src/master/master.cpp
@@ -78,6 +78,7 @@
 #include "common/build.hpp"
 #include "common/http.hpp"
 #include "common/protobuf_utils.hpp"
+#include "common/resource_quantities.hpp"
 #include "common/status_utils.hpp"
 
 #include "credentials/credentials.hpp"
@@ -148,6 +149,7 @@ using mesos::master::contender::MasterContender;
 
 using mesos::master::detector::MasterDetector;
 
+using mesos::internal::ResourceQuantities;
 
 static bool isValidFailoverTimeout(const FrameworkInfo& frameworkInfo);
 
@@ -769,39 +771,31 @@ void Master::initialize()
   }
 
   // Parse min_allocatable_resources.
-  Try<vector<Resources>> minAllocatableResources =
-    [](const string& resourceString) -> Try<vector<Resources>> {
-      vector<Resources> result;
-
-      foreach (const string& token, strings::tokenize(resourceString, "|")) {
-        Try<vector<Resource>> resourceVector =
-          Resources::fromSimpleString(token);
+  vector<ResourceQuantities> minAllocatableResources;
+  foreach (
+      const string& token,
+      strings::tokenize(flags.min_allocatable_resources, "|")) {
+    Try<ResourceQuantities> resourceQuantities =
+      ResourceQuantities::fromString(token);
 
-        if (resourceVector.isError()) {
-          return Error(resourceVector.error());
-        }
+    if (resourceQuantities.isError()) {
+      EXIT(EXIT_FAILURE) << "Error parsing min_allocatable_resources '"
+                         << flags.min_allocatable_resources
+                         << "': " << resourceQuantities.error();
+    }
 
-        result.push_back(Resources(CHECK_NOTERROR(resourceVector)));
+    // We check the configuration against first-class resources and warn
+    // against possible mis-configuration (e.g. typo).
+    set<string> firstClassResources = {"cpus", "mem", "disk", "ports", "gpus"};
+    for (auto it = resourceQuantities->begin(); it != resourceQuantities->end();
+         ++it) {
+      if (firstClassResources.count(it->first) == 0) {
+        LOG(WARNING) << "Non-first-class resource '" << it->first
+                     << "' is configured as part of min_allocatable_resources";
       }
-
-      return result;
-  }(flags.min_allocatable_resources);
-
-  if (minAllocatableResources.isError()) {
-    EXIT(EXIT_FAILURE) << "Error parsing min_allocatable_resources: '"
-                       << flags.min_allocatable_resources
-                       << "': " << minAllocatableResources.error();
-  }
-
-  // Validate that configured minimum resources are "pure" scalar quantities.
-  foreach (
-      const Resources& resources, CHECK_NOTERROR(minAllocatableResources)) {
-    if (!Resources::isScalarQuantity(resources)) {
-      EXIT(EXIT_FAILURE) << "Invalid min_allocatable_resources: '"
-                         << flags.min_allocatable_resources << "': "
-                         << "minimum allocatable resources should only"
-                         << "have name, type (scalar) and value set";
     }
+
+    minAllocatableResources.push_back(resourceQuantities.get());
   }
 
   // Initialize the allocator.
@@ -812,7 +806,7 @@ void Master::initialize()
       flags.fair_sharing_excluded_resource_names,
       flags.filter_gpu_resources,
       flags.domain,
-      CHECK_NOTERROR(minAllocatableResources));
+      minAllocatableResources);
 
   // Parse the whitelist. Passing Allocator::updateWhitelist()
   // callback is safe because we shut down the whitelistWatcher in
diff --git a/src/tests/allocator.hpp b/src/tests/allocator.hpp
index 73fc060..c57d208 100644
--- a/src/tests/allocator.hpp
+++ b/src/tests/allocator.hpp
@@ -379,7 +379,7 @@ public:
       const Option<std::set<std::string>>&,
       bool,
       const Option<DomainInfo>&,
-      const Option<std::vector<Resources>>&));
+      const Option<std::vector<mesos::internal::ResourceQuantities>>&));
 
   MOCK_METHOD2(recover, void(
       const int expectedAgentCount,
diff --git a/src/tests/hierarchical_allocator_tests.cpp b/src/tests/hierarchical_allocator_tests.cpp
index 597469e..5dc7815 100644
--- a/src/tests/hierarchical_allocator_tests.cpp
+++ b/src/tests/hierarchical_allocator_tests.cpp
@@ -40,6 +40,8 @@
 #include <stout/stopwatch.hpp>
 #include <stout/utils.hpp>
 
+#include "common/resource_quantities.hpp"
+
 #include "master/constants.hpp"
 #include "master/flags.hpp"
 
@@ -61,6 +63,8 @@ using mesos::internal::protobuf::createLabel;
 
 using mesos::internal::slave::AGENT_CAPABILITIES;
 
+using mesos::internal::ResourceQuantities;
+
 using mesos::allocator::Allocator;
 
 using process::Clock;
@@ -176,11 +180,12 @@ protected:
         };
     }
 
-    vector<Resources> minAllocatableResources;
+    vector<ResourceQuantities> minAllocatableResources;
+    minAllocatableResources.push_back(CHECK_NOTERROR(
+        ResourceQuantities::fromString("cpus:" + stringify(MIN_CPUS))));
     minAllocatableResources.push_back(
-        CHECK_NOTERROR(Resources::parse("cpus:" + stringify(MIN_CPUS))));
-    minAllocatableResources.push_back(CHECK_NOTERROR(Resources::parse(
-        "mem:" + stringify((double)MIN_MEM.bytes() / Bytes::MEGABYTES))));
+        CHECK_NOTERROR(ResourceQuantities::fromString(
+            "mem:" + stringify((double)MIN_MEM.bytes() / Bytes::MEGABYTES))));
 
     allocator->initialize(
         flags.allocation_interval,
diff --git a/src/tests/master_allocator_tests.cpp b/src/tests/master_allocator_tests.cpp
index 58821a4..72c549f 100644
--- a/src/tests/master_allocator_tests.cpp
+++ b/src/tests/master_allocator_tests.cpp
@@ -1235,6 +1235,112 @@ TYPED_TEST(MasterAllocatorTest, MemoryOnlyOfferedAndTaskLaunched)
 }
 
 
+// This test verifies that the allocator honors the `min_allocatable_resources`
+// flag and only offers resources that are more than at least one of the
+// specified resources quantities.
+TYPED_TEST(MasterAllocatorTest, MinAllocatableResources)
+{
+  Clock::pause();
+
+  master::Flags masterFlags = this->CreateMasterFlags();
+  masterFlags.min_allocatable_resources =
+    "cpus:1;mem:100|disk:100|cpus:1;ports:10";
+
+  TestAllocator<TypeParam> allocator;
+
+  Try<Owned<cluster::Master>> master =
+    this->StartMaster(&allocator, masterFlags);
+  ASSERT_SOME(master);
+
+  Owned<MasterDetector> detector = master.get()->createDetector();
+
+  // Prevent agents from destruction until end of test.
+  vector<Owned<cluster::Slave>> slaves;
+
+  // Add agents with non-allocatable resources.
+  vector<string> nonAllocatableAgentResources = {
+    "cpus:1;mem:10;disk:10;ports:[1-5]",
+    "cpus:0.5;mem:100;disk:10;ports:[6-10]",
+    "cpus:0.5;mem:200;disk:10;ports:[11-15]",
+    "cpus:0.5;mem:10;disk:50;ports:[16-20]"};
+
+  foreach (const string& resourcesString, nonAllocatableAgentResources) {
+    Future<SlaveRegisteredMessage> slaveRegisteredMessage =
+      FUTURE_PROTOBUF(SlaveRegisteredMessage(), _, _);
+
+    slave::Flags flags = this->CreateSlaveFlags();
+    flags.resources = resourcesString;
+
+    Try<Owned<cluster::Slave>> slave = this->StartSlave(detector.get(), flags);
+    ASSERT_SOME(slave);
+
+    // Advance the clock to trigger agent registration.
+    Clock::advance(flags.registration_backoff_factor);
+    Clock::settle();
+
+    AWAIT_READY(slaveRegisteredMessage);
+
+    slaves.push_back(slave.get());
+  }
+
+  // Add agents with allocatable resources.
+  vector<string> allocatableAgentResources = {
+    "cpus:1;mem:100;disk:0;ports:[1-5]",    // contains `cpus:1;mem:100`
+    "cpus:0.5;mem:10;disk:100;ports:[1-5]", // contains `disk:100`
+    "cpus:1;mem:100;disk:50",               // contains `cpus:1;mem:100`
+    "cpus:1;mem:10;disk:10;ports:[1-10]",   // contains `cpus:1;ports:10`
+  };
+
+  hashset<SlaveID> allocatableAgents;
+  foreach (const string& resourcesString, allocatableAgentResources) {
+    Future<SlaveRegisteredMessage> slaveRegisteredMessage =
+      FUTURE_PROTOBUF(SlaveRegisteredMessage(), _, _);
+
+    slave::Flags flags = this->CreateSlaveFlags();
+    flags.resources = resourcesString;
+
+    Try<Owned<cluster::Slave>> slave = this->StartSlave(detector.get(), flags);
+    ASSERT_SOME(slave);
+
+    // Advance the clock to trigger agent registration.
+    Clock::advance(flags.registration_backoff_factor);
+    Clock::settle();
+
+    AWAIT_READY(slaveRegisteredMessage);
+
+    slaves.push_back(slave.get());
+    allocatableAgents.insert(slaveRegisteredMessage->slave_id());
+  }
+
+  // Add a framework. This will trigger an event-driven allocation.
+  // Since it is the only framework, it will get all offers.
+  MockScheduler sched;
+  MesosSchedulerDriver driver(
+      &sched, DEFAULT_FRAMEWORK_INFO, master.get()->pid, DEFAULT_CREDENTIAL);
+
+  EXPECT_CALL(sched, registered(_, _, _));
+
+  Future<vector<Offer>> offers;
+  EXPECT_CALL(sched, resourceOffers(&driver, _))
+    .WillOnce(FutureArg<1>(&offers));
+
+  driver.start();
+
+  AWAIT_READY(offers);
+
+  // All allocatable agents are offered. None of the non-allocatable
+  // agents should be offered.
+  EXPECT_EQ(offers->size(), allocatableAgents.size());
+
+  foreach (const Offer& offer, offers.get()) {
+    EXPECT_TRUE(allocatableAgents.count(offer.slave_id()) != 0);
+  }
+
+  driver.stop();
+  driver.join();
+}
+
+
 // Checks that changes to the whitelist are sent to the allocator.
 // The allocator whitelisting is tested in the allocator unit tests.
 // TODO(bmahler): Move this to a whitelist unit test.


[mesos] 04/05: Added a `Resources` method `contains(ResourceQuantities)`.

Posted by mz...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

mzhu pushed a commit to branch 1.6.x
in repository https://gitbox.apache.org/repos/asf/mesos.git

commit 610f9c6c06509db2131891083f7dd803caa6737f
Author: Meng Zhu <mz...@mesosphere.io>
AuthorDate: Wed Dec 19 14:49:11 2018 -0800

    Added a `Resources` method `contains(ResourceQuantities)`.
    
    This method checks if the quantities of this `Resources` is a
    superset of the given `ResourceQuantities`. If a `Resource` object
    is `SCALAR` type, its quantity is its scalar value. For `RANGES`
    and `SET` type, their quantities are the number of different
    instances in the range or set. For example, "range:[1-5]" has a
    quantity of 5 and "set:{a,b}" has a quantity of 2.
    
    Also added a dedicated test.
    
    Review: https://reviews.apache.org/r/69601
---
 include/mesos/resources.hpp    | 11 +++++
 include/mesos/v1/resources.hpp | 11 +++++
 src/common/resources.cpp       | 44 +++++++++++++++++++
 src/tests/resources_tests.cpp  | 96 ++++++++++++++++++++++++++++++++++++++++++
 src/v1/resources.cpp           | 44 +++++++++++++++++++
 5 files changed, 206 insertions(+)

diff --git a/include/mesos/resources.hpp b/include/mesos/resources.hpp
index 175833c..d8481a7 100644
--- a/include/mesos/resources.hpp
+++ b/include/mesos/resources.hpp
@@ -57,6 +57,8 @@ namespace mesos {
 // Forward declaration.
 class ResourceConversion;
 
+namespace internal { class ResourceQuantities; }
+
 
 // Helper functions.
 bool operator==(
@@ -400,6 +402,15 @@ public:
   // Checks if this Resources contains the given Resource.
   bool contains(const Resource& that) const;
 
+  // Checks if the quantities of this `Resources` is a superset of the
+  // given `ResourceQuantities`. If a `Resource` object is `SCALAR` type,
+  // its quantity is its scalar value. For `RANGES` and `SET` type, their
+  // quantities are the number of different instances in the range or set.
+  // For example, "range:[1-5]" has a quantity of 5 and "set:{a,b}" has a
+  // quantity of 2.
+  bool contains(
+      const mesos::internal::ResourceQuantities& quantities) const;
+
   // Count the Resource objects that match the specified value.
   //
   // NOTE:
diff --git a/include/mesos/v1/resources.hpp b/include/mesos/v1/resources.hpp
index b607b68..7da6e41 100644
--- a/include/mesos/v1/resources.hpp
+++ b/include/mesos/v1/resources.hpp
@@ -52,6 +52,9 @@
 // but instead just written for correct semantics.
 
 namespace mesos {
+
+namespace internal { class ResourceQuantities; }
+
 namespace v1 {
 
 // Forward declaration.
@@ -400,6 +403,14 @@ public:
   // Checks if this Resources contains the given Resource.
   bool contains(const Resource& that) const;
 
+  // Checks if the quantities of this `Resources` is a superset of the
+  // given `ResourceQuantities`. If a `Resource` object is `SCALAR` type,
+  // its quantity is its scalar value. For `RANGES` and `SET` type, their
+  // quantities are the number of different instances in the range or set.
+  // For example, "range:[1-5]" has a quantity of 5 and "set:{a,b}" has a
+  // quantity of 2.
+  bool contains(
+      const mesos::internal::ResourceQuantities& quantities) const;
   // Count the Resource objects that match the specified value.
   //
   // NOTE:
diff --git a/src/common/resources.cpp b/src/common/resources.cpp
index 253b8bc..06327d4 100644
--- a/src/common/resources.cpp
+++ b/src/common/resources.cpp
@@ -38,16 +38,20 @@
 #include <stout/strings.hpp>
 #include <stout/unreachable.hpp>
 
+#include "common/resource_quantities.hpp"
 #include "common/resources_utils.hpp"
 
 using std::map;
 using std::ostream;
+using std::pair;
 using std::set;
 using std::string;
 using std::vector;
 
 using google::protobuf::RepeatedPtrField;
 
+using mesos::internal::ResourceQuantities;
+
 namespace mesos {
 
 /////////////////////////////////////////////////
@@ -1467,6 +1471,46 @@ bool Resources::contains(const Resource& that) const
 }
 
 
+// This function assumes all quantities with the same name are merged
+// in the input `quantities` which is a guaranteed property of
+// `ResourceQuantities`.
+bool Resources::contains(const ResourceQuantities& quantities) const
+{
+  foreach (auto& quantity, quantities){
+    double remaining = quantity.second.value();
+
+    foreach (const Resource& r, get(quantity.first)) {
+      switch (r.type()) {
+        case Value::SCALAR: remaining -= r.scalar().value(); break;
+        case Value::SET:    remaining -= r.set().item_size(); break;
+        case Value::RANGES:
+          foreach (const Value::Range& range, r.ranges().range()) {
+            remaining -= range.end() - range.begin() + 1;
+            if (remaining <= 0) {
+              break;
+            }
+          }
+          break;
+        case Value::TEXT:
+          LOG(FATAL) << "Unexpected TEXT type resource " << r << " in "
+                     << *this;
+          break;
+      }
+
+      if (remaining <= 0) {
+        break;
+      }
+    }
+
+    if (remaining > 0) {
+      return false;
+    }
+  }
+
+  return true;
+}
+
+
 size_t Resources::count(const Resource& that) const
 {
   foreach (const Resource_& resource_, resources) {
diff --git a/src/tests/resources_tests.cpp b/src/tests/resources_tests.cpp
index da3d313..489e867 100644
--- a/src/tests/resources_tests.cpp
+++ b/src/tests/resources_tests.cpp
@@ -31,6 +31,7 @@
 
 #include <mesos/v1/resources.hpp>
 
+#include "common/resource_quantities.hpp"
 #include "common/resources_utils.hpp"
 
 #include "internal/evolve.hpp"
@@ -57,6 +58,8 @@ using mesos::internal::evolve;
 
 using mesos::internal::protobuf::createLabel;
 
+using mesos::internal::ResourceQuantities;
+
 namespace mesos {
 namespace internal {
 namespace tests {
@@ -1974,6 +1977,99 @@ TEST(ResourcesTest, isScalarQuantity)
 }
 
 
+TEST(ResourcesTest, ContainsResourceQuantities)
+{
+  auto resources = [](const string& s) {
+    return CHECK_NOTERROR(Resources::parse(s));
+  };
+
+  auto quantities = [](const string& s) {
+      return CHECK_NOTERROR(ResourceQuantities::fromString(s));
+  };
+
+  // Empty case tests.
+
+  Resources emptyResources;
+  ResourceQuantities emptyQuantities;
+
+  EXPECT_TRUE(emptyResources.contains(emptyQuantities));
+  EXPECT_FALSE(emptyResources.contains(quantities("cpus:1")));
+  EXPECT_TRUE(resources("cpus:1").contains(emptyQuantities));
+
+  // Single scalar resource tests.
+
+  EXPECT_TRUE(resources("cpus:2").contains(quantities("cpus:1")));
+
+  EXPECT_TRUE(resources("cpus:1").contains(quantities("cpus:1")));
+
+  EXPECT_FALSE(resources("cpus:0.5").contains(quantities("cpus:1")));
+
+  // Single range resource tests.
+
+  EXPECT_TRUE(resources("ports:[1-3]").contains(quantities("ports:2")));
+
+  EXPECT_TRUE(resources("ports:[1-2]").contains(quantities("ports:2")));
+
+  EXPECT_FALSE(resources("ports:[1-1]").contains(quantities("ports:2")));
+
+  // Single set resources tests.
+
+  EXPECT_TRUE(resources("features:{a,b,c}").contains(quantities("features:2")));
+
+  EXPECT_TRUE(resources("features:{a,b}").contains(quantities("features:2")));
+
+  EXPECT_FALSE(resources("features:{a}").contains(quantities("features:2")));
+
+  // Multiple resources tests.
+
+  EXPECT_TRUE(resources("cpus:3;ports:[1-3];features:{a,b,c};mem:10")
+                .contains(quantities("cpus:3;ports:3;features:3")));
+
+  EXPECT_TRUE(resources("cpus:3;ports:[1-3];features:{a,b,c}")
+                .contains(quantities("cpus:3;ports:3;features:3")));
+
+  EXPECT_FALSE(resources("cpus:1;ports:[1-3];features:{a,b,c}")
+                 .contains(quantities("cpus:3;ports:3;features:3")));
+
+  EXPECT_FALSE(resources("cpus:3;ports:[1-3]")
+                 .contains(quantities("cpus:3;ports:3;features:3")));
+
+  // Duplicate names.
+
+  EXPECT_FALSE(resources("cpus(role1):2").contains(quantities("cpus:3")));
+
+  EXPECT_TRUE(resources("cpus(role1):2;cpus:1").contains(quantities("cpus:3")));
+
+  Resource::ReservationInfo reservation =
+    createDynamicReservationInfo("role", "principal");
+  Resources resources_ = createReservedResource("ports", "[1-10]", reservation);
+
+  EXPECT_FALSE(resources_.contains(quantities("ports:12")));
+
+  resources_ +=
+    CHECK_NOTERROR(Resources::parse("ports:[20-25]")); // 15 ports in total.
+
+  EXPECT_TRUE(resources_.contains(quantities("ports:12")));
+
+  resources_ = createPersistentVolume(
+      Megabytes(64),
+      "role1",
+      "id1",
+      "path1",
+      None(),
+      None(),
+      "principal1",
+      true); // Shared.
+
+  EXPECT_FALSE(resources_.contains(quantities("disk:128")));
+
+  resources_ +=
+    CHECK_NOTERROR(Resources::parse("disk:64")); // 128M disk in total.
+
+  EXPECT_TRUE(resources_.contains(quantities("disk:128")));
+}
+
+
 TEST(ReservedResourcesTest, Validation)
 {
   // Unreserved.
diff --git a/src/v1/resources.cpp b/src/v1/resources.cpp
index ab8fc3e..95e6259 100644
--- a/src/v1/resources.cpp
+++ b/src/v1/resources.cpp
@@ -39,16 +39,20 @@
 #include <stout/strings.hpp>
 #include <stout/unreachable.hpp>
 
+#include "common/resource_quantities.hpp"
 #include "common/resources_utils.hpp"
 
 using std::map;
 using std::ostream;
+using std::pair;
 using std::set;
 using std::string;
 using std::vector;
 
 using google::protobuf::RepeatedPtrField;
 
+using mesos::internal::ResourceQuantities;
+
 namespace mesos {
 namespace v1 {
 
@@ -1485,6 +1489,46 @@ bool Resources::contains(const Resource& that) const
 }
 
 
+// This function assumes all quantities with the same name are merged
+// in the input `quantities` which is a guaranteed property of
+// `ResourceQuantities`.
+bool Resources::contains(const ResourceQuantities& quantities) const
+{
+  foreach (auto& quantity, quantities){
+    double remaining = quantity.second.value();
+
+    foreach (const Resource& r, get(quantity.first)) {
+      switch (r.type()) {
+        case Value::SCALAR: remaining -= r.scalar().value(); break;
+        case Value::SET:    remaining -= r.set().item_size(); break;
+        case Value::RANGES:
+          foreach (const Value::Range& range, r.ranges().range()) {
+            remaining -= range.end() - range.begin() + 1;
+            if (remaining <= 0) {
+              break;
+            }
+          }
+          break;
+        case Value::TEXT:
+          LOG(FATAL) << "Unexpected TEXT type resource " << r << " in "
+                     << *this;
+          break;
+      }
+
+      if (remaining <= 0) {
+        break;
+      }
+    }
+
+    if (remaining > 0) {
+      return false;
+    }
+  }
+
+  return true;
+}
+
+
 size_t Resources::count(const Resource& that) const
 {
   foreach (const Resource_& resource_, resources) {


[mesos] 02/05: Pulled up the resource quantities class for more general use.

Posted by mz...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

mzhu pushed a commit to branch 1.6.x
in repository https://gitbox.apache.org/repos/asf/mesos.git

commit 2762d74313fe3f8f8a539e09c6a60e66681f2de2
Author: Meng Zhu <mz...@mesosphere.io>
AuthorDate: Fri Jan 4 11:07:43 2019 -0800

    Pulled up the resource quantities class for more general use.
    
    There are many places that we need to express the concept of
    resource quantities such as enforcing allocation quantities
    in the allocator and set guaranteed resource quantities with quota.
    However, the current code usually uses the complex Resources
    class which is unnecessary and inefficient.
    
    This patch pulls class ScalarResourceQuantities in sorter.hpp
    up, aiming to use it for all resource quantity related usages.
    We mostly preserve the map interface and added other utilities such
    as parsing.
    
    Review: https://reviews.apache.org/r/69599
---
 src/CMakeLists.txt                            |   1 +
 src/Makefile.am                               |   2 +
 src/common/resource_quantities.cpp            | 138 ++++++++++++++++++++++++++
 src/common/resource_quantities.hpp            | 127 ++++++++++++++++++++++++
 src/master/allocator/sorter/drf/sorter.cpp    |  14 +--
 src/master/allocator/sorter/drf/sorter.hpp    |   2 +
 src/master/allocator/sorter/random/sorter.hpp |   2 +
 7 files changed, 280 insertions(+), 6 deletions(-)

diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index 70d5126..de9b5b8 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -230,6 +230,7 @@ set(COMMON_SRC
   common/http.cpp
   common/protobuf_utils.cpp
   common/resources.cpp
+  common/resource_quantities.cpp
   common/resources_utils.cpp
   common/roles.cpp
   common/type_utils.cpp
diff --git a/src/Makefile.am b/src/Makefile.am
index 7f80e55..9499a20 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -978,6 +978,8 @@ libmesos_no_3rdparty_la_SOURCES +=					\
   common/http.cpp							\
   common/protobuf_utils.cpp						\
   common/resources.cpp							\
+  common/resource_quantities.cpp					\
+  common/resource_quantities.hpp					\
   common/resources_utils.cpp						\
   common/roles.cpp							\
   common/type_utils.cpp							\
diff --git a/src/common/resource_quantities.cpp b/src/common/resource_quantities.cpp
new file mode 100644
index 0000000..3209839
--- /dev/null
+++ b/src/common/resource_quantities.cpp
@@ -0,0 +1,138 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include <string>
+#include <vector>
+
+#include <mesos/values.hpp>
+
+#include <stout/check.hpp>
+#include <stout/error.hpp>
+#include <stout/foreach.hpp>
+#include <stout/numify.hpp>
+
+#include "common/resource_quantities.hpp"
+
+using std::pair;
+using std::string;
+using std::vector;
+
+namespace mesos {
+namespace internal {
+
+  // This function tries to be consistent with `Resources::fromSimpleString()`.
+  // We trim the whitespace around the pair and in the number but whitespace in
+  // "c p us:10" are preserved and will be parsed to {"c p us", 10}.
+Try<ResourceQuantities> ResourceQuantities::fromString(const string& text)
+{
+  ResourceQuantities result;
+
+  foreach (const string& token, strings::tokenize(text, ";")) {
+    vector<string> pair = strings::tokenize(token, ":");
+    if (pair.size() != 2) {
+      return Error("Failed to parse '" + token + "': missing or extra ':'");
+    }
+
+    Try<Value> value = values::parse(pair[1]);
+    if (value.isError()) {
+      return Error(
+          "Failed to parse '" + pair[1] + "' to quantity: " + value.error());
+    }
+
+    if (value->type() != Value::SCALAR) {
+      return Error(
+          "Failed to parse '" + pair[1] + "' to quantity:"
+          " only scalar values are allowed");
+    }
+
+    if (value->scalar().value() < 0) {
+      return Error(
+          "Failed to parse '" + pair[1] + "' to quantity:"
+          " negative values are not allowed");
+    }
+
+    result[strings::trim(pair[0])] += value->scalar();
+  }
+
+  return result;
+}
+
+
+ResourceQuantities::ResourceQuantities()
+{
+  // Pre-reserve space for first-class resources.
+  // [cpus, disk, gpus, mem, ports]
+  quantities.reserve(5u);
+}
+
+
+ResourceQuantities::const_iterator ResourceQuantities::begin()
+{
+  return static_cast<const std::vector<std::pair<std::string, Value::Scalar>>&>(
+             quantities)
+    .begin();
+}
+
+
+ResourceQuantities::const_iterator ResourceQuantities::end()
+{
+  return static_cast<const std::vector<std::pair<std::string, Value::Scalar>>&>(
+             quantities)
+    .end();
+}
+
+
+Option<Value::Scalar> ResourceQuantities::get(const string& name) const
+{
+  // Don't bother binary searching since
+  // we don't expect a large number of elements.
+  foreach (auto& quantity, quantities) {
+    if (quantity.first == name) {
+      return quantity.second;
+    } else if (quantity.first > name) {
+      // We can return early since we keep names in alphabetical order.
+      break;
+    }
+  }
+
+  return None();
+}
+
+
+Value::Scalar& ResourceQuantities::operator[](const string& name)
+{
+  // Find the location to insert while maintaining
+  // alphabetical ordering. Don't bother binary searching
+  // since we don't expect a large number of quantities.
+  auto it = quantities.begin();
+  for (; it != quantities.end(); ++it) {
+    if (it->first == name) {
+      return it->second;
+    }
+
+    if (it->first > name) {
+      break;
+    }
+  }
+
+  it = quantities.insert(it, std::make_pair(name, Value::Scalar()));
+
+  return it->second;
+}
+
+
+} // namespace internal {
+} // namespace mesos {
diff --git a/src/common/resource_quantities.hpp b/src/common/resource_quantities.hpp
new file mode 100644
index 0000000..11eb426
--- /dev/null
+++ b/src/common/resource_quantities.hpp
@@ -0,0 +1,127 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#ifndef __COMMON_RESOURCE_QUANTITIES_HPP__
+#define __COMMON_RESOURCE_QUANTITIES_HPP__
+
+#include <string>
+#include <utility>
+#include <vector>
+
+#include <mesos/mesos.hpp>
+
+#include <stout/try.hpp>
+
+namespace mesos {
+namespace internal {
+
+
+// An efficient collection of resource quantities.
+//
+// E.g. [("cpus", 4.5), ("gpus", 0), ("ports", 1000)]
+//
+// We provide map-like semantics: [] operator for inserting/retrieving values,
+// keys are unique, and iteration is sorted.
+//
+// Notes on handling negativity and arithmetic operations: during construction,
+// `Value::Scalar` arguments must be non-negative and finite. Invalid arguments
+// will result in an error where `Try` is returned. Once constructed, users can
+// use `[]` operator to mutate values directly. No member methods are provided
+// for arithmetic operations. Users should be mindful of producing negatives
+// when mutating the `Value::Scalar` directly.
+//
+// An alternative design for this class was to provide addition and
+// subtraction functions as opposed to a map interface. However, this
+// leads to some un-obvious semantics around presence of zero values
+// (will zero entries be automatically removed?) and handling of negatives
+// (will negatives trigger a crash? will they be converted to zero? Note
+// that Value::Scalar should be non-negative but there is currently no
+// enforcement of this by Value::Scalar arithmetic operations; we probably
+// want all Value::Scalar operations to go through the same negative
+// handling). To avoid the confusion, we provided a map like interface
+// which produces clear semantics: insertion works like maps, and the
+// user is responsible for performing arithmetic operations on the values
+// (using the already provided Value::Scalar arithmetic overloads).
+//
+// Note for posterity, the status quo prior to this class
+// was to use stripped-down `Resources` objects for storing
+// quantities, however this approach:
+//
+//   (1) did not support quantities of non-scalar resources;
+//   (2) was error prone, the caller must ensure that no
+//       `Resource` metatdata (e.g. `DiskInfo`) is present;
+//   (3) had poor performance, given the `Resources` storage
+//       model and arithmetic implementation have to operate
+//       on broader invariants.
+class ResourceQuantities
+{
+public:
+  // Parse an input string of semicolon separated "name:number" pairs.
+  // Duplicate names are allowed in the input and will be merged into one entry.
+  //
+  // Example: "cpus:10;mem:1024;ports:3"
+  //          "cpus:10; mem:1024; ports:3"
+  // NOTE: we will trim the whitespace around the pair and in the number.
+  // However, whitespace in "c p us:10" are preserved and will be parsed to
+  // {"c p us", 10}. This is consistent with `Resources::fromSimpleString()`.
+  //
+  // Numbers must be non-negative and finite, otherwise an `Error`
+  // will be returned.
+  static Try<ResourceQuantities> fromString(const std::string& text);
+
+  ResourceQuantities();
+
+  ResourceQuantities(const ResourceQuantities& that) = default;
+  ResourceQuantities(ResourceQuantities&& that) = default;
+
+  ResourceQuantities& operator=(const ResourceQuantities& that) = default;
+  ResourceQuantities& operator=(ResourceQuantities&& that) = default;
+
+  typedef std::vector<std::pair<std::string, Value::Scalar>>::const_iterator
+    iterator;
+  typedef std::vector<std::pair<std::string, Value::Scalar>>::const_iterator
+    const_iterator;
+
+  // NOTE: Non-`const` `iterator`, `begin()` and `end()` are __intentionally__
+  // defined with `const` semantics in order to prevent mutation during
+  // iteration. Mutation is allowed with the `[]` operator.
+  const_iterator begin();
+  const_iterator end();
+
+  const_iterator begin() const { return quantities.begin(); }
+  const_iterator end() const { return quantities.end(); }
+
+  size_t size() const { return quantities.size(); };
+
+  // Returns the quantity scalar value if a quantity with the given name
+  // exists (even if the quantity is zero), otherwise return `None()`.
+  Option<Value::Scalar> get(const std::string& name) const;
+
+  // Like std::map, returns a reference to the quantity with the given name.
+  // If no quantity exists with the given name, a new entry will be created.
+  Value::Scalar& operator[](const std::string& name);
+
+private:
+  // List of name quantity pairs sorted by name.
+  // Arithmetic and comparison operations benefit from this sorting.
+  std::vector<std::pair<std::string, Value::Scalar>> quantities;
+};
+
+
+} // namespace internal {
+} // namespace mesos {
+
+#endif //  __COMMON_RESOURCE_QUANTITIES_HPP__
diff --git a/src/master/allocator/sorter/drf/sorter.cpp b/src/master/allocator/sorter/drf/sorter.cpp
index 755a36e..76dae59 100644
--- a/src/master/allocator/sorter/drf/sorter.cpp
+++ b/src/master/allocator/sorter/drf/sorter.cpp
@@ -607,13 +607,15 @@ double DRFSorter::calculateShare(const Node* node) const
       continue;
     }
 
-    if (scalar.value() > 0.0 &&
-        node->allocation.totals.contains(resourceName)) {
-      const double allocation =
-        node->allocation.totals.at(resourceName).value();
-
-      share = std::max(share, allocation / scalar.value());
+    if (scalar.value() <= 0.0) {
+      continue;
     }
+
+    Value::Scalar allocation =
+      node->allocation.totals.get(resourceName)
+        .getOrElse(Value::Scalar()); // Absent means zero.
+
+    share = std::max(share, allocation.value() / scalar.value());
   }
 
   return share / findWeight(node);
diff --git a/src/master/allocator/sorter/drf/sorter.hpp b/src/master/allocator/sorter/drf/sorter.hpp
index 8a26854..9b96ed7 100644
--- a/src/master/allocator/sorter/drf/sorter.hpp
+++ b/src/master/allocator/sorter/drf/sorter.hpp
@@ -30,6 +30,8 @@
 #include <stout/hashmap.hpp>
 #include <stout/option.hpp>
 
+#include "common/resource_quantities.hpp"
+
 #include "master/allocator/sorter/drf/metrics.hpp"
 
 #include "master/allocator/sorter/sorter.hpp"
diff --git a/src/master/allocator/sorter/random/sorter.hpp b/src/master/allocator/sorter/random/sorter.hpp
index 6e22cf6..464a4d9 100644
--- a/src/master/allocator/sorter/random/sorter.hpp
+++ b/src/master/allocator/sorter/random/sorter.hpp
@@ -31,6 +31,8 @@
 #include <stout/hashmap.hpp>
 #include <stout/option.hpp>
 
+#include "common/resource_quantities.hpp"
+
 #include "master/allocator/sorter/sorter.hpp"