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 <[email protected]> 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 {
