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:24:59 UTC

[mesos] branch 1.5.x updated (a213535 -> 9445c94)

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

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


    from a213535  Sent SIGKILL to I/O switchboard server as a safeguard.
     new 0f9d46c  Added rvalue reference Try::get overloads.
     new bfcf849  Disallowed nan, inf and so on when parsing Value::Scalar.
     new c306531  Pulled up the resource quantities class for more general use.
     new 5d7f2cd  Added tests for `ResourceQuantities`.
     new da6857a  Added a `Resources` method `contains(ResourceQuantities)`.
     new 9445c94  Extended `min_allocatable_resources` flag to cover non-scalar resources.

The 6 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:
 3rdparty/stout/include/stout/try.hpp          |  27 ++--
 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                         |  59 ++++-----
 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 +++
 28 files changed, 907 insertions(+), 93 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/06: 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.5.x
in repository https://gitbox.apache.org/repos/asf/mesos.git

commit c306531c4a9a58c40d5180947b4e2879567dac6d
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 2a36867..6532308 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -206,6 +206,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 f851be3..dfc0127 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -981,6 +981,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 ecc5586..16af9d3 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 1a7681c..fea4b77 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 2c4f9b1..da4a2ea 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"
 
 


[mesos] 02/06: 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.5.x
in repository https://gitbox.apache.org/repos/asf/mesos.git

commit bfcf849ca7fd0815fe16228944cef8b164f67c66
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 f7e6fa7..e91ed69 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] 06/06: 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.5.x
in repository https://gitbox.apache.org/repos/asf/mesos.git

commit 9445c94b89dd5c9ec6c4964aa260cf90533bebec
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                       |  59 +++++++---------
 src/tests/allocator.hpp                     |   2 +-
 src/tests/hierarchical_allocator_tests.cpp  |  13 ++--
 src/tests/master_allocator_tests.cpp        | 106 ++++++++++++++++++++++++++++
 10 files changed, 195 insertions(+), 74 deletions(-)

diff --git a/docs/configuration/master.md b/docs/configuration/master.md
index 145c5c6..ddc3d7e 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 58155ad..87e7d53 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 ec45e16..4da1097 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(
@@ -532,7 +532,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. During the first stage resources
   // are allocated only to frameworks under roles with quota set. During
diff --git a/src/master/flags.cpp b/src/master/flags.cpp
index 090ab3e..bd67bb7 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 bcc41b3..36b167f 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"
@@ -144,6 +145,8 @@ using mesos::master::contender::MasterContender;
 
 using mesos::master::detector::MasterDetector;
 
+using mesos::internal::ResourceQuantities;
+
 static bool isValidFailoverTimeout(const FrameworkInfo& frameworkInfo);
 
 
@@ -760,39 +763,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.
@@ -803,7 +798,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 5b83061..3a18529 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(
-        Resources::parse("cpus:" + stringify(MIN_CPUS)).get());
-    minAllocatableResources.push_back(Resources::parse(
-        "mem:" + stringify((double)MIN_MEM.bytes() / Bytes::MEGABYTES)).get());
+        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 9b73277..fec69b2 100644
--- a/src/tests/master_allocator_tests.cpp
+++ b/src/tests/master_allocator_tests.cpp
@@ -1216,6 +1216,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/06: 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.5.x
in repository https://gitbox.apache.org/repos/asf/mesos.git

commit 5d7f2cdc9c2a93a45f5e60897442c5ae66853621
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 dfc0127..679a25b 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -2512,6 +2512,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 6f1c67a..2585b6c 100644
--- a/src/tests/CMakeLists.txt
+++ b/src/tests/CMakeLists.txt
@@ -110,6 +110,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/06: Added rvalue reference Try::get overloads.

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

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

commit 0f9d46c07d712b41ab90a6347b5901ef62b8e88d
Author: Benjamin Mahler <bm...@apache.org>
AuthorDate: Mon Feb 5 13:35:51 2018 -0800

    Added rvalue reference Try::get overloads.
    
    Review: https://reviews.apache.org/r/65515
---
 3rdparty/stout/include/stout/try.hpp | 27 ++++++++++++++-------------
 1 file changed, 14 insertions(+), 13 deletions(-)

diff --git a/3rdparty/stout/include/stout/try.hpp b/3rdparty/stout/include/stout/try.hpp
index 90f8aed..30cce7e 100644
--- a/3rdparty/stout/include/stout/try.hpp
+++ b/3rdparty/stout/include/stout/try.hpp
@@ -70,19 +70,10 @@ public:
   bool isSome() const { return data.isSome(); }
   bool isError() const { return data.isNone(); }
 
-  const T& get() const
-  {
-    if (!data.isSome()) {
-      assert(error_.isSome());
-      ABORT("Try::get() but state == ERROR: " + error_.get().message);
-    }
-    return data.get();
-  }
-
-  T& get()
-  {
-    return const_cast<T&>(static_cast<const Try&>(*this).get());
-  }
+  T& get() & { return get(*this); }
+  const T& get() const & { return get(*this); }
+  T&& get() && { return get(std::move(*this)); }
+  const T&& get() const && { return get(std::move(*this)); }
 
   const T* operator->() const { return &get(); }
   T* operator->() { return &get(); }
@@ -101,6 +92,16 @@ public:
 private:
   static const std::string& error_impl(const Error& err) { return err.message; }
 
+  template <typename Self>
+  static auto get(Self&& self) -> decltype(std::forward<Self>(self).data.get())
+  {
+    if (!self.data.isSome()) {
+      assert(self.error_.isSome());
+      ABORT("Try::get() but state == ERROR: " + self.error_->message);
+    }
+    return std::forward<Self>(self).data.get();
+  }
+
   template <typename Err>
   static const Err& error_impl(const Err& err) { return err; }
 


[mesos] 05/06: 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.5.x
in repository https://gitbox.apache.org/repos/asf/mesos.git

commit da6857adfc2711ca9787809387b501debf1fb8d2
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 da3e05f..d6d55e7 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 {
 
 /////////////////////////////////////////////////
@@ -1454,6 +1458,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 c16c601..88d510f 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 08444d9..99547b9 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 {
 
@@ -1473,6 +1477,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) {