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/07 17:20:44 UTC

[mesos] branch master updated (7a37859 -> 386b1fe)

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

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


    from 7a37859  Improved wording in comments for `AgentAdded` event.
     new 071bd9e  Disallowed nan, inf and so on when parsing Value::Scalar.
     new 1601d83  Pulled up the resource quantities class for more general use.
     new cc0a231  Added tests for `ResourceQuantities`.
     new 983cd42  Added a `Resources` method `contains(ResourceQuantities)`.
     new 386b1fe  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           |   5 +-
 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/hierarchical.cpp     |   6 +-
 src/master/allocator/sorter/drf/sorter.cpp      |  14 +-
 src/master/allocator/sorter/drf/sorter.hpp      |   6 +-
 src/master/allocator/sorter/random/sorter.hpp   |   6 +-
 src/master/allocator/sorter/sorter.hpp          |  82 ------------
 src/master/flags.cpp                            |  22 +--
 src/master/master.cpp                           |  58 ++++----
 src/tests/CMakeLists.txt                        |   1 +
 src/tests/hierarchical_allocator_benchmarks.cpp |  13 +-
 src/tests/hierarchical_allocator_tests.cpp      |  13 +-
 src/tests/master_allocator_tests.cpp            |  19 +--
 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 +++
 26 files changed, 790 insertions(+), 162 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] 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 master
in repository https://gitbox.apache.org/repos/asf/mesos.git

commit 1601d83f2d2796cccbc77cc3a9a288dbb90e1655
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    |   6 +-
 src/master/allocator/sorter/random/sorter.hpp |   6 +-
 src/master/allocator/sorter/sorter.hpp        |  82 ---------------
 8 files changed, 284 insertions(+), 92 deletions(-)

diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index bde0704..a574d44 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -228,6 +228,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 7a4904a..b0393e8 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -1019,6 +1019,8 @@ libmesos_no_3rdparty_la_SOURCES +=					\
   common/protobuf_utils.hpp						\
   common/recordio.hpp							\
   common/resources.cpp							\
+  common/resource_quantities.cpp					\
+  common/resource_quantities.hpp					\
   common/resources_utils.cpp						\
   common/resources_utils.hpp						\
   common/roles.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 a648fac..b128df0 100644
--- a/src/master/allocator/sorter/drf/sorter.cpp
+++ b/src/master/allocator/sorter/drf/sorter.cpp
@@ -667,13 +667,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 / getWeight(node);
diff --git a/src/master/allocator/sorter/drf/sorter.hpp b/src/master/allocator/sorter/drf/sorter.hpp
index 084df82..e64c9ad 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"
@@ -175,7 +177,7 @@ private:
     //
     // TODO(bmahler): Can we remove `scalarQuantities` in favor of
     // using this type whenever scalar quantities are needed?
-    ScalarResourceQuantities totals;
+    ResourceQuantities totals;
   } total_;
 
   // Metrics are optionally exposed by the sorter.
@@ -441,7 +443,7 @@ struct DRFSorter::Node
     //
     // TODO(bmahler): Can we remove `scalarQuantities` in favor of
     // using this type whenever scalar quantities are needed?
-    ScalarResourceQuantities totals;
+    ResourceQuantities totals;
   } allocation;
 
   // Compares two nodes according to DRF share.
diff --git a/src/master/allocator/sorter/random/sorter.hpp b/src/master/allocator/sorter/random/sorter.hpp
index 800b22c..4f230ec 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"
 
 
@@ -173,7 +175,7 @@ private:
     //
     // TODO(bmahler): Can we remove `scalarQuantities` in favor of
     // using this type whenever scalar quantities are needed?
-    ScalarResourceQuantities totals;
+    ResourceQuantities totals;
   } total_;
 };
 
@@ -417,7 +419,7 @@ struct RandomSorter::Node
     //
     // TODO(bmahler): Can we remove `scalarQuantities` in favor of
     // using this type whenever scalar quantities are needed?
-    ScalarResourceQuantities totals;
+    ResourceQuantities totals;
   } allocation;
 };
 
diff --git a/src/master/allocator/sorter/sorter.hpp b/src/master/allocator/sorter/sorter.hpp
index 68cf698..25ad48d 100644
--- a/src/master/allocator/sorter/sorter.hpp
+++ b/src/master/allocator/sorter/sorter.hpp
@@ -149,88 +149,6 @@ public:
   virtual size_t count() const = 0;
 };
 
-// Efficient type for scalar resource quantities that avoids
-// the overhead of using `Resources`.
-//
-// TODO(bmahler): This was originally added to replace a
-// `hashmap<string, Scalar>` and hence the interface was
-// tailored to the particular usage of the map. In order
-// to move this up as a replacement of all quantities
-// (e.g. `Resources::createStrippedScalarQuantity()`),
-// this will need more functionality to do so (e.g.
-// arithmetic operators, containment check, etc).
-class ScalarResourceQuantities
-{
-public:
-  ScalarResourceQuantities()
-  {
-    // Pre-reserve space for first-class scalars.
-    quantities.reserve(4u);  // [cpus, disk, gpus, mem]
-  }
-
-  // Returns true if there is a non-zero amount of
-  // the specified resource.
-  bool contains(const std::string& name) const
-  {
-    // Don't bother binary searching since we don't expect
-    // a large number of quantities.
-    foreach (auto& quantity, quantities) {
-      if (quantity.first == name) {
-        return quantity.second.value() > 0.0;
-      }
-    }
-
-    return false;
-  }
-
-  const Value::Scalar& at(const std::string& name) const
-  {
-    // Don't bother binary searching since we don't expect
-    // a large number of quantities.
-    foreach (auto& quantity, quantities) {
-      if (quantity.first == name) {
-        return quantity.second;
-      }
-    }
-
-    // TODO(bmahler): Print out the vector, need to add
-    // a `stringify(const pair<T1, T2>& p)` overload.
-    LOG(FATAL) << "Failed to find '" << name << "'";
-  }
-
-  Value::Scalar& operator[](const std::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;
-  }
-
-  typedef std::vector<std::pair<std::string, Value::Scalar>>::const_iterator
-    const_iterator;
-
-  const_iterator begin() const { return quantities.begin(); }
-  const_iterator   end() const { return quantities.end(); }
-
-private:
-  // List of scalar resources sorted by resource name.
-  // Arithmetic operations benefit from this sorting.
-  std::vector<std::pair<std::string, Value::Scalar>> quantities;
-};
-
 
 } // namespace allocator {
 } // namespace master {


[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 master
in repository https://gitbox.apache.org/repos/asf/mesos.git

commit 071bd9e1b770026381640d4f9b25e19ee040af7b
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 f04d115..c788302 100644
--- a/src/common/values.cpp
+++ b/src/common/values.cpp
@@ -707,6 +707,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 e4fcf98..d67e4eb 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 1be9945..5fd9dc5 100644
--- a/src/v1/values.cpp
+++ b/src/v1/values.cpp
@@ -735,6 +735,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 master
in repository https://gitbox.apache.org/repos/asf/mesos.git

commit 386b1fe99bb9d10af2abaca4832bf584b6181799
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           |  5 ++-
 src/master/allocator/mesos/hierarchical.cpp     |  6 +--
 src/master/flags.cpp                            | 22 ++++++----
 src/master/master.cpp                           | 58 +++++++++++--------------
 src/tests/hierarchical_allocator_benchmarks.cpp | 13 ++++--
 src/tests/hierarchical_allocator_tests.cpp      | 13 ++++--
 src/tests/master_allocator_tests.cpp            | 19 ++++----
 8 files changed, 88 insertions(+), 70 deletions(-)

diff --git a/docs/configuration/master.md b/docs/configuration/master.md
index 83b83b0..7859d80 100644
--- a/docs/configuration/master.md
+++ b/docs/configuration/master.md
@@ -207,15 +207,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 2a6849b..b58371b 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 {
 
@@ -50,7 +52,8 @@ struct Options
   Option<std::set<std::string>> fairnessExcludeResourceNames = None();
   bool filterGpuResources = true;
   Option<DomainInfo> domain = None();
-  Option<std::vector<Resources>> minAllocatableResources = None();
+  Option<std::vector<mesos::internal::ResourceQuantities>>
+    minAllocatableResources = None();
   size_t maxCompletedFrameworks = 0;
   bool publishPerFrameworkMetrics = true;
 };
diff --git a/src/master/allocator/mesos/hierarchical.cpp b/src/master/allocator/mesos/hierarchical.cpp
index cc8ab91..40621ec 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;
@@ -2430,11 +2431,10 @@ bool HierarchicalAllocatorProcess::allocatable(const Resources& resources)
     return true;
   }
 
-  Resources quantity = resources.createStrippedScalarQuantity();
   foreach (
-      const Resources& minResources,
+      const ResourceQuantities& resourceQuantities,
       CHECK_NOTNONE(options.minAllocatableResources)) {
-    if (quantity.contains(minResources)) {
+    if (resources.contains(resourceQuantities)) {
       return true;
     }
   }
diff --git a/src/master/flags.cpp b/src/master/flags.cpp
index f9b68bc..9288714 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 b4faf2b..49b6e5c 100644
--- a/src/master/master.cpp
+++ b/src/master/master.cpp
@@ -79,6 +79,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"
@@ -149,6 +150,7 @@ using mesos::master::contender::MasterContender;
 
 using mesos::master::detector::MasterDetector;
 
+using mesos::internal::ResourceQuantities;
 
 static bool isValidFailoverTimeout(const FrameworkInfo& frameworkInfo);
 
@@ -720,39 +722,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);
-
-        if (resourceVector.isError()) {
-          return Error(resourceVector.error());
-        }
-
-        result.push_back(Resources(CHECK_NOTERROR(resourceVector)));
-      }
-
-      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.
+  vector<ResourceQuantities> minAllocatableResources;
   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";
+      const string& token,
+      strings::tokenize(flags.min_allocatable_resources, "|")) {
+    Try<ResourceQuantities> resourceQuantities =
+      ResourceQuantities::fromString(token);
+
+    if (resourceQuantities.isError()) {
+      EXIT(EXIT_FAILURE) << "Error parsing min_allocatable_resources '"
+                         << flags.min_allocatable_resources
+                         << "': " << resourceQuantities.error();
+    }
+
+    // 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";
+      }
     }
+
+    minAllocatableResources.push_back(resourceQuantities.get());
   }
 
   // Initialize the allocator options.
@@ -763,7 +757,7 @@ void Master::initialize()
     flags.fair_sharing_excluded_resource_names;
   options.filterGpuResources = flags.filter_gpu_resources;
   options.domain = flags.domain;
-  options.minAllocatableResources = CHECK_NOTERROR(minAllocatableResources);
+  options.minAllocatableResources = minAllocatableResources;
   options.maxCompletedFrameworks = flags.max_completed_frameworks;
   options.publishPerFrameworkMetrics = flags.publish_per_framework_metrics;
 
diff --git a/src/tests/hierarchical_allocator_benchmarks.cpp b/src/tests/hierarchical_allocator_benchmarks.cpp
index 527a25d..a240a2b 100644
--- a/src/tests/hierarchical_allocator_benchmarks.cpp
+++ b/src/tests/hierarchical_allocator_benchmarks.cpp
@@ -32,6 +32,8 @@
 #include <stout/hashmap.hpp>
 #include <stout/stopwatch.hpp>
 
+#include "common/resource_quantities.hpp"
+
 #include "master/constants.hpp"
 
 #include "master/allocator/mesos/hierarchical.hpp"
@@ -44,6 +46,8 @@ using mesos::internal::master::MIN_MEM;
 
 using mesos::internal::master::allocator::HierarchicalDRFAllocator;
 
+using mesos::internal::ResourceQuantities;
+
 using mesos::internal::slave::AGENT_CAPABILITIES;
 
 using mesos::allocator::Allocator;
@@ -137,10 +141,11 @@ struct BenchmarkConfig
       frameworkSorter(frameworkSorter_),
       allocationInterval(allocationInterval_)
   {
+    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))));
   }
 
   string allocator;
@@ -149,7 +154,7 @@ struct BenchmarkConfig
 
   Duration allocationInterval;
 
-  vector<Resources> minAllocatableResources;
+  vector<ResourceQuantities> minAllocatableResources;
 
   vector<FrameworkProfile> frameworkProfiles;
   vector<AgentProfile> agentProfiles;
diff --git a/src/tests/hierarchical_allocator_tests.cpp b/src/tests/hierarchical_allocator_tests.cpp
index a3a6d7b..cc88afb 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))));
 
     Options options;
     options.allocationInterval = flags.allocation_interval;
diff --git a/src/tests/master_allocator_tests.cpp b/src/tests/master_allocator_tests.cpp
index 9dbab18..e960757 100644
--- a/src/tests/master_allocator_tests.cpp
+++ b/src/tests/master_allocator_tests.cpp
@@ -1243,7 +1243,8 @@ TYPED_TEST(MasterAllocatorTest, MinAllocatableResources)
   Clock::pause();
 
   master::Flags masterFlags = this->CreateMasterFlags();
-  masterFlags.min_allocatable_resources = "cpus:1;mem:100|disk:100";
+  masterFlags.min_allocatable_resources =
+    "cpus:1;mem:100|disk:100|cpus:1;ports:10";
 
   TestAllocator<TypeParam> allocator;
 
@@ -1258,10 +1259,10 @@ TYPED_TEST(MasterAllocatorTest, MinAllocatableResources)
 
   // Add agents with non-allocatable resources.
   vector<string> nonAllocatableAgentResources = {
-    "cpus:1;mem:0;disk:0",
-    "cpus:0;mem:100;disk:0",
-    "cpus:0.5;mem:200;disk:0",
-    "cpus:0;mem:0;disk:50"};
+    "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 =
@@ -1284,10 +1285,10 @@ TYPED_TEST(MasterAllocatorTest, MinAllocatableResources)
 
   // Add agents with allocatable resources.
   vector<string> allocatableAgentResources = {
-    "cpus:1;mem:100;disk:0",
-    "cpus:0;mem:0;disk:100",
-    "cpus:0.5;mem:0;disk:100",
-    "cpus:1;mem:100;disk:50",
+    "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;


[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 master
in repository https://gitbox.apache.org/repos/asf/mesos.git

commit 983cd42edd404d7cb04b96adb6c379c2e09efdca
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 36ccf0e..62b6b0c 100644
--- a/include/mesos/resources.hpp
+++ b/include/mesos/resources.hpp
@@ -59,6 +59,8 @@ namespace mesos {
 // Forward declaration.
 class ResourceConversion;
 
+namespace internal { class ResourceQuantities; }
+
 
 // Helper functions.
 bool operator==(
@@ -443,6 +445,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 1a9ea44..eb23359 100644
--- a/include/mesos/v1/resources.hpp
+++ b/include/mesos/v1/resources.hpp
@@ -54,6 +54,9 @@
 // but instead just written for correct semantics.
 
 namespace mesos {
+
+namespace internal { class ResourceQuantities; }
+
 namespace v1 {
 
 // Forward declaration.
@@ -442,6 +445,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 758b5a2..f50fe56 100644
--- a/src/common/resources.cpp
+++ b/src/common/resources.cpp
@@ -38,11 +38,13 @@
 #include <stout/strings.hpp>
 #include <stout/unreachable.hpp>
 
+#include "common/resource_quantities.hpp"
 #include "common/resources_utils.hpp"
 
 using std::make_shared;
 using std::map;
 using std::ostream;
+using std::pair;
 using std::set;
 using std::shared_ptr;
 using std::string;
@@ -50,6 +52,8 @@ using std::vector;
 
 using google::protobuf::RepeatedPtrField;
 
+using mesos::internal::ResourceQuantities;
+
 namespace mesos {
 
 /////////////////////////////////////////////////
@@ -1506,6 +1510,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 (
diff --git a/src/tests/resources_tests.cpp b/src/tests/resources_tests.cpp
index 5337a5c..f762d17 100644
--- a/src/tests/resources_tests.cpp
+++ b/src/tests/resources_tests.cpp
@@ -32,6 +32,7 @@
 
 #include <mesos/v1/resources.hpp>
 
+#include "common/resource_quantities.hpp"
 #include "common/resources_utils.hpp"
 
 #include "internal/evolve.hpp"
@@ -58,6 +59,8 @@ using mesos::internal::evolve;
 
 using mesos::internal::protobuf::createLabel;
 
+using mesos::internal::ResourceQuantities;
+
 namespace mesos {
 namespace internal {
 namespace tests {
@@ -2035,6 +2038,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 d717f53..110be10 100644
--- a/src/v1/resources.cpp
+++ b/src/v1/resources.cpp
@@ -39,11 +39,13 @@
 #include <stout/strings.hpp>
 #include <stout/unreachable.hpp>
 
+#include "common/resource_quantities.hpp"
 #include "common/resources_utils.hpp"
 
 using std::make_shared;
 using std::map;
 using std::ostream;
+using std::pair;
 using std::set;
 using std::shared_ptr;
 using std::string;
@@ -51,6 +53,8 @@ using std::vector;
 
 using google::protobuf::RepeatedPtrField;
 
+using mesos::internal::ResourceQuantities;
+
 namespace mesos {
 namespace v1 {
 
@@ -1524,6 +1528,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 (


[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 master
in repository https://gitbox.apache.org/repos/asf/mesos.git

commit cc0a231481db705a8ef45c8be6f68dd52c23ff51
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 b0393e8..188a470 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -2546,6 +2546,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/resources_utils.hpp					\
diff --git a/src/tests/CMakeLists.txt b/src/tests/CMakeLists.txt
index c588183..7a1265e 100644
--- a/src/tests/CMakeLists.txt
+++ b/src/tests/CMakeLists.txt
@@ -115,6 +115,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 {