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:25:02 UTC

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

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"