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:13:14 UTC
[mesos] branch 1.7.x updated (9d54011 -> 8acfdc6)
This is an automated email from the ASF dual-hosted git repository.
mzhu pushed a change to branch 1.7.x
in repository https://gitbox.apache.org/repos/asf/mesos.git.
from 9d54011 Sent SIGKILL to I/O switchboard server as a safeguard.
new cb6ef79 Disallowed nan, inf and so on when parsing Value::Scalar.
new f0a7941 Pulled up the resource quantities class for more general use.
new 54aa25b Added tests for `ResourceQuantities`.
new f0df5b6 Added a `Resources` method `contains(ResourceQuantities)`.
new 8acfdc6 Extended `min_allocatable_resources` flag to cover non-scalar resources.
The 5 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
docs/configuration/master.md | 22 ++--
include/mesos/allocator/allocator.hpp | 4 +-
include/mesos/resources.hpp | 11 ++
include/mesos/v1/resources.hpp | 11 ++
src/CMakeLists.txt | 1 +
src/Makefile.am | 3 +
src/common/resource_quantities.cpp | 138 +++++++++++++++++++++
src/common/resource_quantities.hpp | 127 +++++++++++++++++++
src/common/resources.cpp | 44 +++++++
src/common/values.cpp | 15 +++
src/master/allocator/mesos/allocator.hpp | 25 ++--
src/master/allocator/mesos/hierarchical.cpp | 10 +-
src/master/allocator/mesos/hierarchical.hpp | 5 +-
src/master/allocator/sorter/drf/sorter.cpp | 14 ++-
src/master/allocator/sorter/drf/sorter.hpp | 6 +-
src/master/allocator/sorter/random/sorter.hpp | 6 +-
src/master/allocator/sorter/sorter.hpp | 82 ------------
src/master/flags.cpp | 22 ++--
src/master/master.cpp | 58 ++++-----
src/tests/CMakeLists.txt | 1 +
src/tests/allocator.hpp | 2 +-
src/tests/hierarchical_allocator_tests.cpp | 13 +-
src/tests/master_allocator_tests.cpp | 19 +--
src/tests/resource_quantities_tests.cpp | 171 ++++++++++++++++++++++++++
src/tests/resources_tests.cpp | 96 +++++++++++++++
src/tests/values_tests.cpp | 9 ++
src/v1/resources.cpp | 44 +++++++
src/v1/values.cpp | 15 +++
28 files changed, 800 insertions(+), 174 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] 04/05: Added a `Resources` method
`contains(ResourceQuantities)`.
Posted by mz...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
mzhu pushed a commit to branch 1.7.x
in repository https://gitbox.apache.org/repos/asf/mesos.git
commit f0df5b64acc5520f81acfbce2f195a157f97dcb7
Author: Meng Zhu <mz...@mesosphere.io>
AuthorDate: Wed Dec 19 14:49:11 2018 -0800
Added a `Resources` method `contains(ResourceQuantities)`.
This method checks if the quantities of this `Resources` is a
superset of the given `ResourceQuantities`. If a `Resource` object
is `SCALAR` type, its quantity is its scalar value. For `RANGES`
and `SET` type, their quantities are the number of different
instances in the range or set. For example, "range:[1-5]" has a
quantity of 5 and "set:{a,b}" has a quantity of 2.
Also added a dedicated test.
Review: https://reviews.apache.org/r/69601
---
include/mesos/resources.hpp | 11 +++++
include/mesos/v1/resources.hpp | 11 +++++
src/common/resources.cpp | 44 +++++++++++++++++++
src/tests/resources_tests.cpp | 96 ++++++++++++++++++++++++++++++++++++++++++
src/v1/resources.cpp | 44 +++++++++++++++++++
5 files changed, 206 insertions(+)
diff --git a/include/mesos/resources.hpp b/include/mesos/resources.hpp
index 36ccf0e..62b6b0c 100644
--- a/include/mesos/resources.hpp
+++ b/include/mesos/resources.hpp
@@ -59,6 +59,8 @@ namespace mesos {
// Forward declaration.
class ResourceConversion;
+namespace internal { class ResourceQuantities; }
+
// Helper functions.
bool operator==(
@@ -443,6 +445,15 @@ public:
// Checks if this Resources contains the given Resource.
bool contains(const Resource& that) const;
+ // Checks if the quantities of this `Resources` is a superset of the
+ // given `ResourceQuantities`. If a `Resource` object is `SCALAR` type,
+ // its quantity is its scalar value. For `RANGES` and `SET` type, their
+ // quantities are the number of different instances in the range or set.
+ // For example, "range:[1-5]" has a quantity of 5 and "set:{a,b}" has a
+ // quantity of 2.
+ bool contains(
+ const mesos::internal::ResourceQuantities& quantities) const;
+
// Count the Resource objects that match the specified value.
//
// NOTE:
diff --git a/include/mesos/v1/resources.hpp b/include/mesos/v1/resources.hpp
index 1a9ea44..eb23359 100644
--- a/include/mesos/v1/resources.hpp
+++ b/include/mesos/v1/resources.hpp
@@ -54,6 +54,9 @@
// but instead just written for correct semantics.
namespace mesos {
+
+namespace internal { class ResourceQuantities; }
+
namespace v1 {
// Forward declaration.
@@ -442,6 +445,14 @@ public:
// Checks if this Resources contains the given Resource.
bool contains(const Resource& that) const;
+ // Checks if the quantities of this `Resources` is a superset of the
+ // given `ResourceQuantities`. If a `Resource` object is `SCALAR` type,
+ // its quantity is its scalar value. For `RANGES` and `SET` type, their
+ // quantities are the number of different instances in the range or set.
+ // For example, "range:[1-5]" has a quantity of 5 and "set:{a,b}" has a
+ // quantity of 2.
+ bool contains(
+ const mesos::internal::ResourceQuantities& quantities) const;
// Count the Resource objects that match the specified value.
//
// NOTE:
diff --git a/src/common/resources.cpp b/src/common/resources.cpp
index 7a77eca..eca7fa9 100644
--- a/src/common/resources.cpp
+++ b/src/common/resources.cpp
@@ -38,11 +38,13 @@
#include <stout/strings.hpp>
#include <stout/unreachable.hpp>
+#include "common/resource_quantities.hpp"
#include "common/resources_utils.hpp"
using std::make_shared;
using std::map;
using std::ostream;
+using std::pair;
using std::set;
using std::shared_ptr;
using std::string;
@@ -50,6 +52,8 @@ using std::vector;
using google::protobuf::RepeatedPtrField;
+using mesos::internal::ResourceQuantities;
+
namespace mesos {
/////////////////////////////////////////////////
@@ -1506,6 +1510,46 @@ bool Resources::contains(const Resource& that) const
}
+// This function assumes all quantities with the same name are merged
+// in the input `quantities` which is a guaranteed property of
+// `ResourceQuantities`.
+bool Resources::contains(const ResourceQuantities& quantities) const
+{
+ foreach (auto& quantity, quantities){
+ double remaining = quantity.second.value();
+
+ foreach (const Resource& r, get(quantity.first)) {
+ switch (r.type()) {
+ case Value::SCALAR: remaining -= r.scalar().value(); break;
+ case Value::SET: remaining -= r.set().item_size(); break;
+ case Value::RANGES:
+ foreach (const Value::Range& range, r.ranges().range()) {
+ remaining -= range.end() - range.begin() + 1;
+ if (remaining <= 0) {
+ break;
+ }
+ }
+ break;
+ case Value::TEXT:
+ LOG(FATAL) << "Unexpected TEXT type resource " << r << " in "
+ << *this;
+ break;
+ }
+
+ if (remaining <= 0) {
+ break;
+ }
+ }
+
+ if (remaining > 0) {
+ return false;
+ }
+ }
+
+ return true;
+}
+
+
size_t Resources::count(const Resource& that) const
{
foreach (
diff --git a/src/tests/resources_tests.cpp b/src/tests/resources_tests.cpp
index 002338a..7907b8f 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 {
@@ -2034,6 +2037,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 d953c63..07cdb86 100644
--- a/src/v1/resources.cpp
+++ b/src/v1/resources.cpp
@@ -39,11 +39,13 @@
#include <stout/strings.hpp>
#include <stout/unreachable.hpp>
+#include "common/resource_quantities.hpp"
#include "common/resources_utils.hpp"
using std::make_shared;
using std::map;
using std::ostream;
+using std::pair;
using std::set;
using std::shared_ptr;
using std::string;
@@ -51,6 +53,8 @@ using std::vector;
using google::protobuf::RepeatedPtrField;
+using mesos::internal::ResourceQuantities;
+
namespace mesos {
namespace v1 {
@@ -1524,6 +1528,46 @@ bool Resources::contains(const Resource& that) const
}
+// This function assumes all quantities with the same name are merged
+// in the input `quantities` which is a guaranteed property of
+// `ResourceQuantities`.
+bool Resources::contains(const ResourceQuantities& quantities) const
+{
+ foreach (auto& quantity, quantities){
+ double remaining = quantity.second.value();
+
+ foreach (const Resource& r, get(quantity.first)) {
+ switch (r.type()) {
+ case Value::SCALAR: remaining -= r.scalar().value(); break;
+ case Value::SET: remaining -= r.set().item_size(); break;
+ case Value::RANGES:
+ foreach (const Value::Range& range, r.ranges().range()) {
+ remaining -= range.end() - range.begin() + 1;
+ if (remaining <= 0) {
+ break;
+ }
+ }
+ break;
+ case Value::TEXT:
+ LOG(FATAL) << "Unexpected TEXT type resource " << r << " in "
+ << *this;
+ break;
+ }
+
+ if (remaining <= 0) {
+ break;
+ }
+ }
+
+ if (remaining > 0) {
+ return false;
+ }
+ }
+
+ return true;
+}
+
+
size_t Resources::count(const Resource& that) const
{
foreach (
[mesos] 05/05: Extended `min_allocatable_resources` flag to cover
non-scalar resources.
Posted by mz...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
mzhu pushed a commit to branch 1.7.x
in repository https://gitbox.apache.org/repos/asf/mesos.git
commit 8acfdc64d3a417bf54bb3d9405dc2875d81859ac
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 | 10 +++--
src/master/allocator/mesos/hierarchical.hpp | 5 ++-
src/master/flags.cpp | 22 +++++++----
src/master/master.cpp | 58 +++++++++++++----------------
src/tests/allocator.hpp | 2 +-
src/tests/hierarchical_allocator_tests.cpp | 13 +++++--
src/tests/master_allocator_tests.cpp | 19 +++++-----
10 files changed, 98 insertions(+), 82 deletions(-)
diff --git a/docs/configuration/master.md b/docs/configuration/master.md
index f290e37..883bc61 100644
--- a/docs/configuration/master.md
+++ b/docs/configuration/master.md
@@ -207,15 +207,19 @@ load an alternate allocator module using <code>--modules</code>.
--min_allocatable_resources=VALUE
</td>
<td>
-One or more sets of resources that define the minimum allocatable
-resources for the allocator. The allocator will only offer resources
-that contain at least one of the specified sets. The resources in each
-set should be delimited by semicolons, and the sets should be delimited
-by the pipe character.
-(Example: <code>disk:1|cpu:1;mem:32</code>
-configures the allocator to only offer resources if they contain a disk
-resource of at least 1 megabyte, or if they contain both 1 cpu and
-32 megabytes of memory.) (default: cpus:0.01|mem:32).
+One or more sets of resource quantities that define the minimum allocatable
+resource for the allocator. The allocator will only offer resources that
+meets the quantity requirement of at least one of the specified sets. For
+`SCALAR` type resources, its quantity is its scalar value. For `RANGES` and
+`SET` type, their quantities are the number of different instances in the
+range or set. For example, `range:[1-5]` has a quantity of 5 and `set:{a,b}`
+has a quantity of 2. The resources in each set should be delimited by
+semicolons (acting as logical AND), and each set should be delimited by the
+pipe character (acting as logical OR).
+(Example: `disk:1|cpu:1;mem:32;port:1` configures the allocator to only offer
+resources if they contain a disk resource of at least 1 megabyte, or if they
+at least contain 1 cpu, 32 megabytes of memory and 1 port.)
+(default: cpus:0.01|mem:32).
</td>
</tr>
diff --git a/include/mesos/allocator/allocator.hpp b/include/mesos/allocator/allocator.hpp
index fadd4e8..853ed09 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(),
const size_t maxCompletedFrameworks = 0) = 0;
diff --git a/src/master/allocator/mesos/allocator.hpp b/src/master/allocator/mesos/allocator.hpp
index e65a969..2027c7e 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(),
const size_t maxCompletedFrameworks = 0) override;
void recover(
@@ -209,7 +209,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(),
const size_t maxCompletedFrameworks = 0) = 0;
@@ -367,7 +367,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,
const size_t maxCompletedFrameworks)
{
process::dispatch(
diff --git a/src/master/allocator/mesos/hierarchical.cpp b/src/master/allocator/mesos/hierarchical.cpp
index fd9e819..584197b 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,
const size_t maxCompletedFrameworks)
{
allocationInterval = _allocationInterval;
@@ -2432,10 +2434,10 @@ bool HierarchicalAllocatorProcess::allocatable(const Resources& resources)
return true;
}
- Resources quantity = resources.createStrippedScalarQuantity();
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 e3adb5d..461b192 100644
--- a/src/master/allocator/mesos/hierarchical.hpp
+++ b/src/master/allocator/mesos/hierarchical.hpp
@@ -114,7 +114,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(),
const size_t maxCompletedFrameworks = 0) override;
@@ -558,7 +558,8 @@ protected:
Option<DomainInfo> domain;
// The minimum allocatable resources, if any.
- Option<std::vector<Resources>> minAllocatableResources;
+ Option<std::vector<mesos::internal::ResourceQuantities>>
+ minAllocatableResources;
// There are two stages of allocation:
//
diff --git a/src/master/flags.cpp b/src/master/flags.cpp
index 6ad53ed..38ba888 100644
--- a/src/master/flags.cpp
+++ b/src/master/flags.cpp
@@ -488,14 +488,20 @@ mesos::internal::master::Flags::Flags()
add(&Flags::min_allocatable_resources,
"min_allocatable_resources",
- "One or more sets of resources that define the minimum allocatable\n"
- "resources for the allocator. The allocator will only offer resources\n"
- "that contain at least one of the specified sets. The resources in\n"
- "each set should be delimited by semicolons, and the sets should be\n"
- "delimited by the pipe character.\n"
- "(Example: `disk:1|cpu:1;mem:32` configures the allocator to only offer\n"
- "resources if they contain a disk resource of at least 1 megabyte, or\n"
- "if they contain both 1 cpu and 32 megabytes of memory.)\n",
+ "One or more sets of resource quantities that define the minimum\n"
+ "allocatable resources for the allocator. The allocator will only offer\n"
+ "resources that meets the quantity requirement of at least one of the\n"
+ "specified sets. For `SCALAR` type resources, its quantity is its\n"
+ "scalar value. For `RANGES` and `SET` type, their quantities are the\n"
+ "number of different instances in the range or set. For example,\n"
+ "`range:[1-5]` has a quantity of 5 and `set:{a,b}` has a quantity of 2.\n"
+ "The resources in each set should be delimited by semicolons (acting as\n"
+ "logical AND), and each set should be delimited by the pipe character\n"
+ "(acting as logical OR).\n"
+ "(Example: `disk:1|cpu:1;mem:32;port:1` configures the allocator to\n"
+ "only offer resources if they contain a disk resource of at least\n"
+ "1 megabyte, or if they at least contain 1 cpu, 32 megabytes of memory\n"
+ "and 1 port.)\n",
"cpus:" + stringify(MIN_CPUS) +
"|mem:" + stringify((double)MIN_MEM.bytes() / Bytes::MEGABYTES));
diff --git a/src/master/master.cpp b/src/master/master.cpp
index f33a5aa..873d685 100644
--- a/src/master/master.cpp
+++ b/src/master/master.cpp
@@ -78,6 +78,7 @@
#include "common/build.hpp"
#include "common/http.hpp"
#include "common/protobuf_utils.hpp"
+#include "common/resource_quantities.hpp"
#include "common/status_utils.hpp"
#include "credentials/credentials.hpp"
@@ -148,6 +149,7 @@ using mesos::master::contender::MasterContender;
using mesos::master::detector::MasterDetector;
+using mesos::internal::ResourceQuantities;
static bool isValidFailoverTimeout(const FrameworkInfo& frameworkInfo);
@@ -719,39 +721,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.
@@ -762,7 +756,7 @@ void Master::initialize()
flags.fair_sharing_excluded_resource_names,
flags.filter_gpu_resources,
flags.domain,
- CHECK_NOTERROR(minAllocatableResources),
+ minAllocatableResources,
flags.max_completed_frameworks);
// Parse the whitelist. Passing Allocator::updateWhitelist()
diff --git a/src/tests/allocator.hpp b/src/tests/allocator.hpp
index 1d84446..b251115 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>>&,
const size_t maxCompletedFrameworks));
MOCK_METHOD2(recover, void(
diff --git a/src/tests/hierarchical_allocator_tests.cpp b/src/tests/hierarchical_allocator_tests.cpp
index 27fbd9c..16b7399 100644
--- a/src/tests/hierarchical_allocator_tests.cpp
+++ b/src/tests/hierarchical_allocator_tests.cpp
@@ -40,6 +40,8 @@
#include <stout/stopwatch.hpp>
#include <stout/utils.hpp>
+#include "common/resource_quantities.hpp"
+
#include "master/constants.hpp"
#include "master/flags.hpp"
@@ -61,6 +63,8 @@ using mesos::internal::protobuf::createLabel;
using mesos::internal::slave::AGENT_CAPABILITIES;
+using mesos::internal::ResourceQuantities;
+
using mesos::allocator::Allocator;
using process::Clock;
@@ -176,11 +180,12 @@ protected:
};
}
- vector<Resources> minAllocatableResources;
+ vector<ResourceQuantities> minAllocatableResources;
+ minAllocatableResources.push_back(CHECK_NOTERROR(
+ ResourceQuantities::fromString("cpus:" + stringify(MIN_CPUS))));
minAllocatableResources.push_back(
- CHECK_NOTERROR(Resources::parse("cpus:" + stringify(MIN_CPUS))));
- minAllocatableResources.push_back(CHECK_NOTERROR(Resources::parse(
- "mem:" + stringify((double)MIN_MEM.bytes() / Bytes::MEGABYTES))));
+ CHECK_NOTERROR(ResourceQuantities::fromString(
+ "mem:" + stringify((double)MIN_MEM.bytes() / Bytes::MEGABYTES))));
allocator->initialize(
flags.allocation_interval,
diff --git a/src/tests/master_allocator_tests.cpp b/src/tests/master_allocator_tests.cpp
index 88288ae..c1cb004 100644
--- a/src/tests/master_allocator_tests.cpp
+++ b/src/tests/master_allocator_tests.cpp
@@ -1243,7 +1243,8 @@ TYPED_TEST(MasterAllocatorTest, MinAllocatableResources)
Clock::pause();
master::Flags masterFlags = this->CreateMasterFlags();
- masterFlags.min_allocatable_resources = "cpus:1;mem:100|disk:100";
+ masterFlags.min_allocatable_resources =
+ "cpus:1;mem:100|disk:100|cpus:1;ports:10";
TestAllocator<TypeParam> allocator;
@@ -1258,10 +1259,10 @@ TYPED_TEST(MasterAllocatorTest, MinAllocatableResources)
// Add agents with non-allocatable resources.
vector<string> nonAllocatableAgentResources = {
- "cpus:1;mem:0;disk:0",
- "cpus:0;mem:100;disk:0",
- "cpus:0.5;mem:200;disk:0",
- "cpus:0;mem:0;disk:50"};
+ "cpus:1;mem:10;disk:10;ports:[1-5]",
+ "cpus:0.5;mem:100;disk:10;ports:[6-10]",
+ "cpus:0.5;mem:200;disk:10;ports:[11-15]",
+ "cpus:0.5;mem:10;disk:50;ports:[16-20]"};
foreach (const string& resourcesString, nonAllocatableAgentResources) {
Future<SlaveRegisteredMessage> slaveRegisteredMessage =
@@ -1284,10 +1285,10 @@ TYPED_TEST(MasterAllocatorTest, MinAllocatableResources)
// Add agents with allocatable resources.
vector<string> allocatableAgentResources = {
- "cpus:1;mem:100;disk:0",
- "cpus:0;mem:0;disk:100",
- "cpus:0.5;mem:0;disk:100",
- "cpus:1;mem:100;disk:50",
+ "cpus:1;mem:100;disk:0;ports:[1-5]", // contains `cpus:1;mem:100`
+ "cpus:0.5;mem:10;disk:100;ports:[1-5]", // contains `disk:100`
+ "cpus:1;mem:100;disk:50", // contains `cpus:1;mem:100`
+ "cpus:1;mem:10;disk:10;ports:[1-10]", // contains `cpus:1;ports:10`
};
hashset<SlaveID> allocatableAgents;
[mesos] 01/05: Disallowed nan,
inf and so on when parsing Value::Scalar.
Posted by mz...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
mzhu pushed a commit to branch 1.7.x
in repository https://gitbox.apache.org/repos/asf/mesos.git
commit cb6ef795e32462f021bb26b683d917fbb331d547
Author: Meng Zhu <mz...@mesosphere.io>
AuthorDate: Sat Jan 5 13:28:27 2019 -0800
Disallowed nan, inf and so on when parsing Value::Scalar.
Scalar values are intended to be finite numbers. This
patch checks `nan`, `inf` and so on when parsing
`Value::Scalar`. Only normal or zero numbers (as defined
in `std::fpclassify()`) are allowed.
Also added related tests.
Review: https://reviews.apache.org/r/69673
---
src/common/values.cpp | 15 +++++++++++++++
src/tests/values_tests.cpp | 9 +++++++++
src/v1/values.cpp | 15 +++++++++++++++
3 files changed, 39 insertions(+)
diff --git a/src/common/values.cpp b/src/common/values.cpp
index f04d115..c788302 100644
--- a/src/common/values.cpp
+++ b/src/common/values.cpp
@@ -707,6 +707,21 @@ Try<Value> parse(const string& text)
} else if (index == string::npos) {
Try<double> value_ = numify<double>(temp);
if (!value_.isError()) {
+ Option<Error> error = [value_]() -> Option<Error> {
+ switch (std::fpclassify(value_.get())) {
+ case FP_NORMAL: return None();
+ case FP_ZERO: return None();
+ case FP_INFINITE: return Error("Infinite values not supported");
+ case FP_NAN: return Error("NaN not supported");
+ case FP_SUBNORMAL: return Error("Subnormal values not supported");
+ default: return Error("Unknown error");
+ }
+ }();
+
+ if (error.isSome()) {
+ return Error("Invalid scalar value '" + temp + "':" + error->message);
+ }
+
// This is a scalar.
Value::Scalar* scalar = value.mutable_scalar();
value.set_type(Value::SCALAR);
diff --git a/src/tests/values_tests.cpp b/src/tests/values_tests.cpp
index e4fcf98..d67e4eb 100644
--- a/src/tests/values_tests.cpp
+++ b/src/tests/values_tests.cpp
@@ -89,6 +89,15 @@ TEST(ValuesTest, InvalidInput)
// Test when giving empty string.
EXPECT_ERROR(parse(" "));
+
+ EXPECT_ERROR(parse("nan"));
+ EXPECT_ERROR(parse("-nan"));
+
+ EXPECT_ERROR(parse("inf"));
+ EXPECT_ERROR(parse("-inf"));
+
+ EXPECT_ERROR(parse("infinity"));
+ EXPECT_ERROR(parse("-infinity"));
}
diff --git a/src/v1/values.cpp b/src/v1/values.cpp
index 1be9945..5fd9dc5 100644
--- a/src/v1/values.cpp
+++ b/src/v1/values.cpp
@@ -735,6 +735,21 @@ Try<Value> parse(const string& text)
} else if (index == string::npos) {
Try<double> value_ = numify<double>(temp);
if (!value_.isError()) {
+ Option<Error> error = [value_]() -> Option<Error> {
+ switch (std::fpclassify(value_.get())) {
+ case FP_NORMAL: return None();
+ case FP_ZERO: return None();
+ case FP_INFINITE: return Error("Infinite values not supported");
+ case FP_NAN: return Error("NaN not supported");
+ case FP_SUBNORMAL: return Error("Subnormal values not supported");
+ default: return Error("Unknown error");
+ }
+ }();
+
+ if (error.isSome()) {
+ return Error("Invalid scalar value '" + temp + "':" + error->message);
+ }
+
// This is a scalar.
Value::Scalar* scalar = value.mutable_scalar();
value.set_type(Value::SCALAR);
[mesos] 02/05: Pulled up the resource quantities class for more
general use.
Posted by mz...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
mzhu pushed a commit to branch 1.7.x
in repository https://gitbox.apache.org/repos/asf/mesos.git
commit f0a7941a60044b5e728e214ead4c6c613bafd841
Author: Meng Zhu <mz...@mesosphere.io>
AuthorDate: Fri Jan 4 11:07:43 2019 -0800
Pulled up the resource quantities class for more general use.
There are many places that we need to express the concept of
resource quantities such as enforcing allocation quantities
in the allocator and set guaranteed resource quantities with quota.
However, the current code usually uses the complex Resources
class which is unnecessary and inefficient.
This patch pulls class ScalarResourceQuantities in sorter.hpp
up, aiming to use it for all resource quantity related usages.
We mostly preserve the map interface and added other utilities such
as parsing.
Review: https://reviews.apache.org/r/69599
---
src/CMakeLists.txt | 1 +
src/Makefile.am | 2 +
src/common/resource_quantities.cpp | 138 ++++++++++++++++++++++++++
src/common/resource_quantities.hpp | 127 ++++++++++++++++++++++++
src/master/allocator/sorter/drf/sorter.cpp | 14 +--
src/master/allocator/sorter/drf/sorter.hpp | 6 +-
src/master/allocator/sorter/random/sorter.hpp | 6 +-
src/master/allocator/sorter/sorter.hpp | 82 ---------------
8 files changed, 284 insertions(+), 92 deletions(-)
diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index a80b011..a46ff72 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -230,6 +230,7 @@ set(COMMON_SRC
common/http.cpp
common/protobuf_utils.cpp
common/resources.cpp
+ common/resource_quantities.cpp
common/resources_utils.cpp
common/roles.cpp
common/type_utils.cpp
diff --git a/src/Makefile.am b/src/Makefile.am
index 766bb92..8dfaa3f 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -992,6 +992,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 a648fac..b128df0 100644
--- a/src/master/allocator/sorter/drf/sorter.cpp
+++ b/src/master/allocator/sorter/drf/sorter.cpp
@@ -667,13 +667,15 @@ double DRFSorter::calculateShare(const Node* node) const
continue;
}
- if (scalar.value() > 0.0 &&
- node->allocation.totals.contains(resourceName)) {
- const double allocation =
- node->allocation.totals.at(resourceName).value();
-
- share = std::max(share, allocation / scalar.value());
+ if (scalar.value() <= 0.0) {
+ continue;
}
+
+ Value::Scalar allocation =
+ node->allocation.totals.get(resourceName)
+ .getOrElse(Value::Scalar()); // Absent means zero.
+
+ share = std::max(share, allocation.value() / scalar.value());
}
return share / getWeight(node);
diff --git a/src/master/allocator/sorter/drf/sorter.hpp b/src/master/allocator/sorter/drf/sorter.hpp
index 084df82..e64c9ad 100644
--- a/src/master/allocator/sorter/drf/sorter.hpp
+++ b/src/master/allocator/sorter/drf/sorter.hpp
@@ -30,6 +30,8 @@
#include <stout/hashmap.hpp>
#include <stout/option.hpp>
+#include "common/resource_quantities.hpp"
+
#include "master/allocator/sorter/drf/metrics.hpp"
#include "master/allocator/sorter/sorter.hpp"
@@ -175,7 +177,7 @@ private:
//
// TODO(bmahler): Can we remove `scalarQuantities` in favor of
// using this type whenever scalar quantities are needed?
- ScalarResourceQuantities totals;
+ ResourceQuantities totals;
} total_;
// Metrics are optionally exposed by the sorter.
@@ -441,7 +443,7 @@ struct DRFSorter::Node
//
// TODO(bmahler): Can we remove `scalarQuantities` in favor of
// using this type whenever scalar quantities are needed?
- ScalarResourceQuantities totals;
+ ResourceQuantities totals;
} allocation;
// Compares two nodes according to DRF share.
diff --git a/src/master/allocator/sorter/random/sorter.hpp b/src/master/allocator/sorter/random/sorter.hpp
index 800b22c..4f230ec 100644
--- a/src/master/allocator/sorter/random/sorter.hpp
+++ b/src/master/allocator/sorter/random/sorter.hpp
@@ -31,6 +31,8 @@
#include <stout/hashmap.hpp>
#include <stout/option.hpp>
+#include "common/resource_quantities.hpp"
+
#include "master/allocator/sorter/sorter.hpp"
@@ -173,7 +175,7 @@ private:
//
// TODO(bmahler): Can we remove `scalarQuantities` in favor of
// using this type whenever scalar quantities are needed?
- ScalarResourceQuantities totals;
+ ResourceQuantities totals;
} total_;
};
@@ -417,7 +419,7 @@ struct RandomSorter::Node
//
// TODO(bmahler): Can we remove `scalarQuantities` in favor of
// using this type whenever scalar quantities are needed?
- ScalarResourceQuantities totals;
+ ResourceQuantities totals;
} allocation;
};
diff --git a/src/master/allocator/sorter/sorter.hpp b/src/master/allocator/sorter/sorter.hpp
index 68cf698..25ad48d 100644
--- a/src/master/allocator/sorter/sorter.hpp
+++ b/src/master/allocator/sorter/sorter.hpp
@@ -149,88 +149,6 @@ public:
virtual size_t count() const = 0;
};
-// Efficient type for scalar resource quantities that avoids
-// the overhead of using `Resources`.
-//
-// TODO(bmahler): This was originally added to replace a
-// `hashmap<string, Scalar>` and hence the interface was
-// tailored to the particular usage of the map. In order
-// to move this up as a replacement of all quantities
-// (e.g. `Resources::createStrippedScalarQuantity()`),
-// this will need more functionality to do so (e.g.
-// arithmetic operators, containment check, etc).
-class ScalarResourceQuantities
-{
-public:
- ScalarResourceQuantities()
- {
- // Pre-reserve space for first-class scalars.
- quantities.reserve(4u); // [cpus, disk, gpus, mem]
- }
-
- // Returns true if there is a non-zero amount of
- // the specified resource.
- bool contains(const std::string& name) const
- {
- // Don't bother binary searching since we don't expect
- // a large number of quantities.
- foreach (auto& quantity, quantities) {
- if (quantity.first == name) {
- return quantity.second.value() > 0.0;
- }
- }
-
- return false;
- }
-
- const Value::Scalar& at(const std::string& name) const
- {
- // Don't bother binary searching since we don't expect
- // a large number of quantities.
- foreach (auto& quantity, quantities) {
- if (quantity.first == name) {
- return quantity.second;
- }
- }
-
- // TODO(bmahler): Print out the vector, need to add
- // a `stringify(const pair<T1, T2>& p)` overload.
- LOG(FATAL) << "Failed to find '" << name << "'";
- }
-
- Value::Scalar& operator[](const std::string& name)
- {
- // Find the location to insert while maintaining
- // alphabetical ordering. Don't bother binary searching
- // since we don't expect a large number of quantities.
- auto it = quantities.begin();
- for (; it != quantities.end(); ++it) {
- if (it->first == name) {
- return it->second;
- }
-
- if (it->first > name) {
- break;
- }
- }
-
- it = quantities.insert(it, std::make_pair(name, Value::Scalar()));
-
- return it->second;
- }
-
- typedef std::vector<std::pair<std::string, Value::Scalar>>::const_iterator
- const_iterator;
-
- const_iterator begin() const { return quantities.begin(); }
- const_iterator end() const { return quantities.end(); }
-
-private:
- // List of scalar resources sorted by resource name.
- // Arithmetic operations benefit from this sorting.
- std::vector<std::pair<std::string, Value::Scalar>> quantities;
-};
-
} // namespace allocator {
} // namespace master {
[mesos] 03/05: Added tests for `ResourceQuantities`.
Posted by mz...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
mzhu pushed a commit to branch 1.7.x
in repository https://gitbox.apache.org/repos/asf/mesos.git
commit 54aa25b421d2621a59ce5e4117337c6dec18de71
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 8dfaa3f..eb50fc7 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -2541,6 +2541,7 @@ mesos_tests_SOURCES = \
tests/resource_offers_tests.cpp \
tests/resource_provider_manager_tests.cpp \
tests/resource_provider_validation_tests.cpp \
+ tests/resource_quantities_tests.cpp \
tests/resources_tests.cpp \
tests/resources_utils.cpp \
tests/resources_utils.hpp \
diff --git a/src/tests/CMakeLists.txt b/src/tests/CMakeLists.txt
index fed072a..08473b3 100644
--- a/src/tests/CMakeLists.txt
+++ b/src/tests/CMakeLists.txt
@@ -113,6 +113,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 {