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/04/01 19:42:54 UTC
[mesos] branch master updated (9a6b3cb -> 1acb38c)
This is an automated email from the ASF dual-hosted git repository.
mzhu pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/mesos.git.
from 9a6b3cb Updated test `AgentRegisteredWithNewId` for better code coverage.
new fbfe193 Simplified quota chopping logic in the allocator.
new a5af016 Used `ResourceQuantities` in `__allocate()` when appropriate.
new ef26c53 Used `ResourceQuantities` in the sorter when appropriate.
new 1acb38c Removed unused function `Resources::isScalarQuantity`.
The 4 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:
include/mesos/resources.hpp | 5 -
include/mesos/v1/resources.hpp | 5 -
src/common/resources.cpp | 11 --
src/master/allocator/mesos/hierarchical.cpp | 215 +++++++++++---------------
src/master/allocator/mesos/hierarchical.hpp | 12 +-
src/master/allocator/sorter/drf/sorter.cpp | 27 ++--
src/master/allocator/sorter/drf/sorter.hpp | 84 ++++------
src/master/allocator/sorter/random/sorter.cpp | 27 ++--
src/master/allocator/sorter/random/sorter.hpp | 84 ++++------
src/master/allocator/sorter/sorter.hpp | 11 +-
src/tests/resources_tests.cpp | 31 ----
src/tests/sorter_tests.cpp | 10 +-
src/v1/resources.cpp | 11 --
13 files changed, 184 insertions(+), 349 deletions(-)
[mesos] 02/04: Used `ResourceQuantities` in `__allocate()` when
appropriate.
Posted by mz...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
mzhu pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mesos.git
commit a5af0162dbe77ba3d76d94b2a2b4beb92d63f83a
Author: Meng Zhu <mz...@mesosphere.io>
AuthorDate: Fri Mar 29 15:39:17 2019 -0700
Used `ResourceQuantities` in `__allocate()` when appropriate.
Replaced `Resources` with `ResourceQuantities` when
appropriate in `__allocate()`. This simplifies the
code and improves performance.
Review: https://reviews.apache.org/r/70320
---
src/master/allocator/mesos/hierarchical.cpp | 164 ++++++++++++++--------------
src/master/allocator/mesos/hierarchical.hpp | 12 +-
2 files changed, 89 insertions(+), 87 deletions(-)
diff --git a/src/master/allocator/mesos/hierarchical.cpp b/src/master/allocator/mesos/hierarchical.cpp
index f2393ed..047e799 100644
--- a/src/master/allocator/mesos/hierarchical.cpp
+++ b/src/master/allocator/mesos/hierarchical.cpp
@@ -925,7 +925,7 @@ void HierarchicalAllocatorProcess::updateAllocation(
// Update the allocated resources in the quota sorter. We only update
// the allocated resources if this role has quota set.
- if (quotas.contains(role)) {
+ if (quotaGuarantees.contains(role)) {
// See comment at `quotaRoleSorter` declaration regarding non-revocable.
quotaRoleSorter->update(
role,
@@ -1428,11 +1428,12 @@ void HierarchicalAllocatorProcess::setQuota(
// the role is not set. Setting quota differs from updating it because
// the former moves the role to a different allocation group with a
// dedicated sorter, while the later just updates the actual quota.
- CHECK(!quotas.contains(role));
+ CHECK(!quotaGuarantees.contains(role));
// Persist quota in memory and add the role into the corresponding
// allocation group.
- quotas[role] = quota;
+ quotaGuarantees[role] =
+ ResourceQuantities::fromScalarResources(quota.info.guarantee());
quotaRoleSorter->add(role);
quotaRoleSorter->activate(role);
@@ -1468,15 +1469,15 @@ void HierarchicalAllocatorProcess::removeQuota(
CHECK(initialized);
// Do not allow removing quota if it is not set.
- CHECK(quotas.contains(role));
+ CHECK(quotaGuarantees.contains(role));
CHECK(quotaRoleSorter->contains(role));
// TODO(alexr): Print all quota info for the role.
- LOG(INFO) << "Removed quota " << quotas[role].info.guarantee()
- << " for role '" << role << "'";
+ LOG(INFO) << "Removed quota " << quotaGuarantees[role] << " for role '"
+ << role << "'";
// Remove the role from the quota'ed allocation group.
- quotas.erase(role);
+ quotaGuarantees.erase(role);
quotaRoleSorter->remove(role);
metrics.removeQuota(role);
@@ -1690,9 +1691,7 @@ void HierarchicalAllocatorProcess::__allocate()
//
// (2) Simplify the quota enforcement logic -- the allocator
// would no longer need to track reservations separately.
- //
- // Note these are __quantities__ with no meta-data.
- hashmap<string, Resources> rolesConsumedQuotaScalarQuantites;
+ hashmap<string, ResourceQuantities> rolesConsumedQuota;
// We charge a role against its quota by considering its allocation as well
// as any unallocated reservations since reservations are bound to the role.
@@ -1702,22 +1701,24 @@ void HierarchicalAllocatorProcess::__allocate()
//
// Consumed Quota = reservations + unreserved allocation
// = reservations + allocation - allocated reservations
- foreachkey (const string& role, quotas) {
+ foreachkey (const string& role, quotaGuarantees) {
// First add reservations.
- rolesConsumedQuotaScalarQuantites[role] +=
- reservationScalarQuantities.get(role).getOrElse(Resources());
+ rolesConsumedQuota[role] +=
+ reservationScalarQuantities.get(role).getOrElse(ResourceQuantities());
// Then add allocated resource _quantities_ .
// NOTE: Revocable resources are excluded in `quotaRoleSorter`.
- CHECK(quotas.contains(role));
- rolesConsumedQuotaScalarQuantites[role] +=
- quotaRoleSorter->allocationScalarQuantities(role);
+ //
+ // TODO(mzhu): Update sorter to return `ResourceQuantities` directly.
+ CHECK(quotaGuarantees.contains(role));
+ rolesConsumedQuota[role] += ResourceQuantities::fromScalarResources(
+ quotaRoleSorter->allocationScalarQuantities(role));
// Lastly subtract allocated reservations on each agent.
foreachvalue (
const Resources& resources, quotaRoleSorter->allocation(role)) {
- rolesConsumedQuotaScalarQuantites[role] -=
- resources.reserved().createStrippedScalarQuantity();
+ rolesConsumedQuota[role] -=
+ ResourceQuantities::fromScalarResources(resources.reserved().scalars());
}
}
@@ -1731,13 +1732,13 @@ void HierarchicalAllocatorProcess::__allocate()
// Given the above, if a role has more reservations (which count towards
// consumed quota) than quota guarantee, we don't need to hold back any
// unreserved headroom for it.
- Resources requiredHeadroom;
- foreachpair (const string& role, const Quota& quota, quotas) {
- // We can safely subtract resources without checking inclusion. If the
- // minuend resource is less than the subtrahend resource, the result is an
- // empty resource.
- requiredHeadroom += Resources(quota.info.guarantee()) -
- rolesConsumedQuotaScalarQuantites.get(role).getOrElse(Resources());
+ ResourceQuantities requiredHeadroom;
+ foreachpair (
+ const string& role,
+ const ResourceQuantities& guarantee,
+ quotaGuarantees) {
+ requiredHeadroom +=
+ guarantee - rolesConsumedQuota.get(role).getOrElse(ResourceQuantities());
}
// We will allocate resources while ensuring that the required
@@ -1752,17 +1753,24 @@ void HierarchicalAllocatorProcess::__allocate()
// allocated resources -
// unallocated reservations -
// unallocated revocable resources
- Resources availableHeadroom = roleSorter->totalScalarQuantities();
+ //
+ // TODO(mzhu): Update sorter to return `ResourceQuantities` directly.
+ ResourceQuantities availableHeadroom =
+ ResourceQuantities::fromScalarResources(
+ roleSorter->totalScalarQuantities());
// Subtract allocated resources from the total.
+ //
+ // TODO(mzhu): Update sorter to return `ResourceQuantities` directly.
foreachkey (const string& role, roles) {
- availableHeadroom -= roleSorter->allocationScalarQuantities(role);
+ availableHeadroom -= ResourceQuantities::fromScalarResources(
+ roleSorter->allocationScalarQuantities(role));
}
// Calculate total allocated reservations. Note that we need to ensure
// we count a reservation for "a" being allocated to "a/b", therefore
// we cannot simply loop over the reservations' roles.
- Resources totalAllocatedReservationScalarQuantities;
+ ResourceQuantities totalAllocatedReservation;
foreachkey (const string& role, roles) {
const hashmap<SlaveID, Resources>* allocations;
if (quotaRoleSorter->contains(role)) {
@@ -1774,20 +1782,19 @@ void HierarchicalAllocatorProcess::__allocate()
}
foreachvalue (const Resources& resources, *CHECK_NOTNULL(allocations)) {
- totalAllocatedReservationScalarQuantities +=
- resources.reserved().createStrippedScalarQuantity();
+ totalAllocatedReservation +=
+ ResourceQuantities::fromScalarResources(resources.reserved().scalars());
}
}
// Subtract total unallocated reservations.
- availableHeadroom -=
- Resources::sum(reservationScalarQuantities) -
- totalAllocatedReservationScalarQuantities;
+ availableHeadroom -= ResourceQuantities::sum(reservationScalarQuantities) -
+ totalAllocatedReservation;
// Subtract revocable resources.
foreachvalue (const Slave& slave, slaves) {
- availableHeadroom -=
- slave.getAvailable().revocable().createStrippedScalarQuantity();
+ availableHeadroom -= ResourceQuantities::fromScalarResources(
+ slave.getAvailable().revocable().scalars());
}
// Due to the two stages in the allocation algorithm and the nature of
@@ -1819,9 +1826,9 @@ void HierarchicalAllocatorProcess::__allocate()
Slave& slave = slaves.at(slaveId);
foreach (const string& role, quotaRoleSorter->sort()) {
- CHECK(quotas.contains(role));
+ CHECK(quotaGuarantees.contains(role));
- const Quota& quota = quotas.at(role);
+ const ResourceQuantities& quotaGuarantee = quotaGuarantees.at(role);
// If there are no active frameworks in this role, we do not
// need to do any allocations for this role.
@@ -1901,17 +1908,15 @@ void HierarchicalAllocatorProcess::__allocate()
// have quota, there are no ancestor reservations involved here.
Resources toAllocate = available.reserved(role).nonRevocable();
- // This is a scalar quantity with no meta-data.
- Resources unsatisfiedQuotaGuarantee =
- Resources(quota.info.guarantee()) -
- rolesConsumedQuotaScalarQuantites.get(role).getOrElse(Resources());
+ ResourceQuantities unsatisfiedQuotaGuarantee =
+ quotaGuarantee -
+ rolesConsumedQuota.get(role).getOrElse(ResourceQuantities());
Resources unreserved = available.nonRevocable().unreserved();
// First, allocate resources up to a role's quota guarantee.
- Resources newQuotaAllocation = shrinkResources(
- unreserved,
- ResourceQuantities::fromScalarResources(unsatisfiedQuotaGuarantee));
+ Resources newQuotaAllocation =
+ shrinkResources(unreserved, unsatisfiedQuotaGuarantee);
toAllocate += newQuotaAllocation;
@@ -1927,21 +1932,16 @@ void HierarchicalAllocatorProcess::__allocate()
// Second, allocate scalar resources with unset quota while maintaining
// the quota headroom.
- set<string> quotaGuaranteeResourceNames =
- Resources(quota.info.guarantee()).names();
-
Resources nonQuotaGuaranteeResources =
- unreserved.filter(
- ["aGuaranteeResourceNames] (const Resource& resource) {
- return quotaGuaranteeResourceNames.count(resource.name()) == 0;
- }
- );
+ unreserved.filter([&](const Resource& resource) {
+ return quotaGuarantee.get(resource.name()) == Value::Scalar();
+ });
- Resources surplusHeadroom = availableHeadroom - requiredHeadroom;
+ ResourceQuantities surplusHeadroom =
+ availableHeadroom - requiredHeadroom;
- nonQuotaGuaranteeResources = shrinkResources(
- nonQuotaGuaranteeResources,
- ResourceQuantities::fromScalarResources(surplusHeadroom.scalars()));
+ nonQuotaGuaranteeResources =
+ shrinkResources(nonQuotaGuaranteeResources, surplusHeadroom);
toAllocate += nonQuotaGuaranteeResources;
@@ -1968,11 +1968,12 @@ void HierarchicalAllocatorProcess::__allocate()
offerable[frameworkId][role][slaveId] += toAllocate;
offeredSharedResources[slaveId] += toAllocate.shared();
- Resources allocatedUnreserved =
- toAllocate.unreserved().createStrippedScalarQuantity();
+ ResourceQuantities allocatedUnreserved =
+ ResourceQuantities::fromScalarResources(
+ toAllocate.unreserved().scalars());
// Update role consumed quota.
- rolesConsumedQuotaScalarQuantites[role] += allocatedUnreserved;
+ rolesConsumedQuota[role] += allocatedUnreserved;
// Track quota guarantee headroom change.
@@ -1981,7 +1982,8 @@ void HierarchicalAllocatorProcess::__allocate()
// role's guarantee should be subtracted. Allocation of reserved
// resources or resources that this role has unset guarantee do not
// affect `requiredHeadroom`.
- requiredHeadroom -= newQuotaAllocation.createStrippedScalarQuantity();
+ requiredHeadroom -=
+ ResourceQuantities::fromScalarResources(newQuotaAllocation.scalars());
// `availableHeadroom` counts total unreserved non-revocable resources
// in the cluster.
@@ -2009,7 +2011,7 @@ void HierarchicalAllocatorProcess::__allocate()
foreach (const string& role, roleSorter->sort()) {
// In the second allocation stage, we only allocate
// for non-quota roles.
- if (quotas.contains(role)) {
+ if (quotaGuarantees.contains(role)) {
continue;
}
@@ -2059,16 +2061,17 @@ void HierarchicalAllocatorProcess::__allocate()
// If allocating these resources would reduce the headroom
// below what is required, we will hold them back.
- const Resources headroomToAllocate = toAllocate
- .scalars().unreserved().nonRevocable();
+ const Resources headroomResources =
+ toAllocate.scalars().unreserved().nonRevocable();
+ const ResourceQuantities headroomToAllocate =
+ ResourceQuantities::fromScalarResources(headroomResources);
bool sufficientHeadroom =
- (availableHeadroom -
- headroomToAllocate.createStrippedScalarQuantity())
+ (availableHeadroom - headroomToAllocate)
.contains(requiredHeadroom);
if (!sufficientHeadroom) {
- toAllocate -= headroomToAllocate;
+ toAllocate -= headroomResources;
}
// If the framework filters these resources, ignore.
@@ -2088,8 +2091,7 @@ void HierarchicalAllocatorProcess::__allocate()
offeredSharedResources[slaveId] += toAllocate.shared();
if (sufficientHeadroom) {
- availableHeadroom -=
- headroomToAllocate.createStrippedScalarQuantity();
+ availableHeadroom -= headroomToAllocate;
}
slave.allocate(toAllocate);
@@ -2583,14 +2585,14 @@ void HierarchicalAllocatorProcess::trackReservations(
{
foreachpair (const string& role,
const Resources& resources, reservations) {
- const Resources scalarQuantitesToTrack =
- resources.createStrippedScalarQuantity();
+ const ResourceQuantities quantities =
+ ResourceQuantities::fromScalarResources(resources.scalars());
- if (scalarQuantitesToTrack.empty()) {
+ if (quantities.empty()) {
continue; // Do not insert an empty entry.
}
- reservationScalarQuantities[role] += scalarQuantitesToTrack;
+ reservationScalarQuantities[role] += quantities;
}
}
@@ -2600,21 +2602,21 @@ void HierarchicalAllocatorProcess::untrackReservations(
{
foreachpair (const string& role,
const Resources& resources, reservations) {
- const Resources scalarQuantitesToUntrack =
- resources.createStrippedScalarQuantity();
+ const ResourceQuantities quantities =
+ ResourceQuantities::fromScalarResources(resources.scalars());
- if (scalarQuantitesToUntrack.empty()) {
+ if (quantities.empty()) {
continue; // Do not CHECK for the role if there's nothing to untrack.
}
CHECK(reservationScalarQuantities.contains(role));
- Resources& currentReservationQuantity =
+ ResourceQuantities& currentReservationQuantities =
reservationScalarQuantities.at(role);
- CHECK(currentReservationQuantity.contains(scalarQuantitesToUntrack));
- currentReservationQuantity -= scalarQuantitesToUntrack;
+ CHECK(currentReservationQuantities.contains(quantities));
+ currentReservationQuantities -= quantities;
- if (currentReservationQuantity.empty()) {
+ if (currentReservationQuantities.empty()) {
reservationScalarQuantities.erase(role);
}
}
@@ -2780,7 +2782,7 @@ void HierarchicalAllocatorProcess::trackAllocatedResources(
frameworkSorters.at(role)->allocated(
frameworkId.value(), slaveId, allocation);
- if (quotas.contains(role)) {
+ if (quotaGuarantees.contains(role)) {
// See comment at `quotaRoleSorter` declaration regarding non-revocable.
quotaRoleSorter->allocated(role, slaveId, allocation.nonRevocable());
}
@@ -2815,7 +2817,7 @@ void HierarchicalAllocatorProcess::untrackAllocatedResources(
roleSorter->unallocated(role, slaveId, allocation);
- if (quotas.contains(role)) {
+ if (quotaGuarantees.contains(role)) {
// See comment at `quotaRoleSorter` declaration regarding non-revocable.
quotaRoleSorter->unallocated(role, slaveId, allocation.nonRevocable());
}
diff --git a/src/master/allocator/mesos/hierarchical.hpp b/src/master/allocator/mesos/hierarchical.hpp
index 36bf90b..9d51aeb 100644
--- a/src/master/allocator/mesos/hierarchical.hpp
+++ b/src/master/allocator/mesos/hierarchical.hpp
@@ -541,17 +541,17 @@ protected:
// (e.g. some tasks and/or executors are consuming resources under the role).
hashmap<std::string, hashset<FrameworkID>> roles;
- // Configured quota for each role, if any. If a role does not have
- // an entry here it has the default quota of (no guarantee, no limit).
- hashmap<std::string, Quota> quotas;
+ // Configured guaranteed resource quantities for each role, if any.
+ // If a role does not have an entry here it has (the default)
+ // no guarantee.
+ hashmap<std::string, ResourceQuantities> quotaGuarantees;
// Aggregated resource reservations on all agents tied to a
- // particular role, if any. These are stripped scalar quantities
- // that contain no meta-data.
+ // particular role, if any.
//
// Only roles with non-empty scalar reservation quantities will
// be stored in the map.
- hashmap<std::string, Resources> reservationScalarQuantities;
+ hashmap<std::string, ResourceQuantities> reservationScalarQuantities;
// Slaves to send offers for.
Option<hashset<std::string>> whitelist;
[mesos] 04/04: Removed unused function
`Resources::isScalarQuantity`.
Posted by mz...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
mzhu pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mesos.git
commit 1acb38c27306326a53f866b5386b5e28a6dc9314
Author: Meng Zhu <mz...@mesosphere.io>
AuthorDate: Fri Mar 29 15:53:26 2019 -0700
Removed unused function `Resources::isScalarQuantity`.
The function was introduced when `class Resources` is
used for resource quantities. In particular, it was
used in API validation where users are asked to specify
resource quantities using `Resources`. Since `ResourceQuantities`
is introduced, the function is no longer needed.
Review: https://reviews.apache.org/r/70346
---
include/mesos/resources.hpp | 5 -----
include/mesos/v1/resources.hpp | 5 -----
src/common/resources.cpp | 11 -----------
src/tests/resources_tests.cpp | 31 -------------------------------
src/v1/resources.cpp | 11 -----------
5 files changed, 63 deletions(-)
diff --git a/include/mesos/resources.hpp b/include/mesos/resources.hpp
index 62b6b0c..609ca61 100644
--- a/include/mesos/resources.hpp
+++ b/include/mesos/resources.hpp
@@ -349,11 +349,6 @@ public:
// Tests if the given Resource object is shared.
static bool isShared(const Resource& resource);
- // Tests if the given Resources object is a "pure" scalar quantity which
- // consists of resource objects with ONLY name, type (set to "Scalar")
- // and scalar fields set.
- static bool isScalarQuantity(const Resources& resources);
-
// Tests if the given Resource object has refined reservations.
static bool hasRefinedReservations(const Resource& resource);
diff --git a/include/mesos/v1/resources.hpp b/include/mesos/v1/resources.hpp
index eb23359..8afccbe 100644
--- a/include/mesos/v1/resources.hpp
+++ b/include/mesos/v1/resources.hpp
@@ -349,11 +349,6 @@ public:
// Tests if the given Resource object is shared.
static bool isShared(const Resource& resource);
- // Tests if the given Resources object is a "pure" scalar quantity which
- // only consists of resource object with ONLY name, type (set to "Scalar")
- // and scalar fields set.
- static bool isScalarQuantity(const Resources& resources);
-
// Tests if the given Resource object has refined reservations.
static bool hasRefinedReservations(const Resource& resource);
diff --git a/src/common/resources.cpp b/src/common/resources.cpp
index f50fe56..543820d 100644
--- a/src/common/resources.cpp
+++ b/src/common/resources.cpp
@@ -1262,17 +1262,6 @@ bool Resources::isShared(const Resource& resource)
}
-bool Resources::isScalarQuantity(const Resources& resources)
-{
- // Instead of checking the absence of non-scalar-quantity fields,
- // we do an equality check between the original resources object and
- // its stripped counterpart.
- //
- // We remove the static reservation metadata here via `toUnreserved()`.
- return resources == resources.createStrippedScalarQuantity().toUnreserved();
-}
-
-
bool Resources::hasRefinedReservations(const Resource& resource)
{
CHECK(!resource.has_role()) << resource;
diff --git a/src/tests/resources_tests.cpp b/src/tests/resources_tests.cpp
index e67057c..dbae0f6 100644
--- a/src/tests/resources_tests.cpp
+++ b/src/tests/resources_tests.cpp
@@ -2007,37 +2007,6 @@ TEST(ResourcesTest, AbsentResources)
}
-TEST(ResourcesTest, isScalarQuantity)
-{
- Resources scalarQuantity1 = Resources::parse("cpus:1").get();
- EXPECT_TRUE(Resources::isScalarQuantity(scalarQuantity1));
-
- Resources scalarQuantity2 = Resources::parse("cpus:1;mem:1").get();
- EXPECT_TRUE(Resources::isScalarQuantity(scalarQuantity2));
-
- Resources range = Resources::parse("ports", "[1-16000]", "*").get();
- EXPECT_FALSE(Resources::isScalarQuantity(range));
-
- Resources set = Resources::parse("names:{foo,bar}").get();
- EXPECT_FALSE(Resources::isScalarQuantity(set));
-
- Resources reserved = createReservedResource(
- "cpus", "1", createDynamicReservationInfo("role", "principal"));
- EXPECT_FALSE(Resources::isScalarQuantity(reserved));
-
- Resources disk = createDiskResource("10", "role1", "1", "path");
- EXPECT_FALSE(Resources::isScalarQuantity(disk));
-
- Resources allocated = Resources::parse("cpus:1;mem:512").get();
- allocated.allocate("role");
- EXPECT_FALSE(Resources::isScalarQuantity(allocated));
-
- Resource revocable = Resources::parse("cpus", "1", "*").get();
- revocable.mutable_revocable();
- EXPECT_FALSE(Resources::isScalarQuantity(revocable));
-}
-
-
TEST(ResourcesTest, ContainsResourceQuantities)
{
auto resources = [](const string& s) {
diff --git a/src/v1/resources.cpp b/src/v1/resources.cpp
index d3a6194..7d6113f 100644
--- a/src/v1/resources.cpp
+++ b/src/v1/resources.cpp
@@ -1282,17 +1282,6 @@ bool Resources::isShared(const Resource& resource)
}
-bool Resources::isScalarQuantity(const Resources& resources)
-{
- // Instead of checking the absence of non-scalar-quantity fields,
- // we do an equality check between the original `Resources` object and
- // its stripped counterpart.
- //
- // We remove the static reservation metadata here via `toUnreserved()`.
- return resources == resources.createStrippedScalarQuantity().toUnreserved();
-}
-
-
bool Resources::hasRefinedReservations(const Resource& resource)
{
CHECK(!resource.has_role()) << resource;
[mesos] 03/04: Used `ResourceQuantities` in the sorter when
appropriate.
Posted by mz...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
mzhu pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mesos.git
commit ef26c536f89c76ec2752299096bc5f5538753ed2
Author: Meng Zhu <mz...@mesosphere.io>
AuthorDate: Wed Mar 27 17:17:32 2019 -0700
Used `ResourceQuantities` in the sorter when appropriate.
Replaced `Resources` with `ResourceQuantities` when
appropriate in the sorter. This simplifies the code
and improves performance.
Review: https://reviews.apache.org/r/70330
---
src/master/allocator/mesos/hierarchical.cpp | 29 ++-------
src/master/allocator/sorter/drf/sorter.cpp | 27 +++++----
src/master/allocator/sorter/drf/sorter.hpp | 84 +++++++++------------------
src/master/allocator/sorter/random/sorter.cpp | 27 +++++----
src/master/allocator/sorter/random/sorter.hpp | 84 +++++++++------------------
src/master/allocator/sorter/sorter.hpp | 11 ++--
src/tests/sorter_tests.cpp | 10 ++--
7 files changed, 98 insertions(+), 174 deletions(-)
diff --git a/src/master/allocator/mesos/hierarchical.cpp b/src/master/allocator/mesos/hierarchical.cpp
index 047e799..b9bddd3 100644
--- a/src/master/allocator/mesos/hierarchical.cpp
+++ b/src/master/allocator/mesos/hierarchical.cpp
@@ -1708,11 +1708,9 @@ void HierarchicalAllocatorProcess::__allocate()
// Then add allocated resource _quantities_ .
// NOTE: Revocable resources are excluded in `quotaRoleSorter`.
- //
- // TODO(mzhu): Update sorter to return `ResourceQuantities` directly.
CHECK(quotaGuarantees.contains(role));
- rolesConsumedQuota[role] += ResourceQuantities::fromScalarResources(
- quotaRoleSorter->allocationScalarQuantities(role));
+ rolesConsumedQuota[role] +=
+ quotaRoleSorter->allocationScalarQuantities(role);
// Lastly subtract allocated reservations on each agent.
foreachvalue (
@@ -1753,18 +1751,11 @@ void HierarchicalAllocatorProcess::__allocate()
// allocated resources -
// unallocated reservations -
// unallocated revocable resources
- //
- // TODO(mzhu): Update sorter to return `ResourceQuantities` directly.
- ResourceQuantities availableHeadroom =
- ResourceQuantities::fromScalarResources(
- roleSorter->totalScalarQuantities());
+ ResourceQuantities availableHeadroom = roleSorter->totalScalarQuantities();
// Subtract allocated resources from the total.
- //
- // TODO(mzhu): Update sorter to return `ResourceQuantities` directly.
foreachkey (const string& role, roles) {
- availableHeadroom -= ResourceQuantities::fromScalarResources(
- roleSorter->allocationScalarQuantities(role));
+ availableHeadroom -= roleSorter->allocationScalarQuantities(role);
}
// Calculate total allocated reservations. Note that we need to ensure
@@ -2457,11 +2448,7 @@ double HierarchicalAllocatorProcess::_resources_offered_or_allocated(
double HierarchicalAllocatorProcess::_resources_total(
const string& resource)
{
- Option<Value::Scalar> total =
- roleSorter->totalScalarQuantities()
- .get<Value::Scalar>(resource);
-
- return total.isSome() ? total->value() : 0;
+ return roleSorter->totalScalarQuantities().get(resource).value();
}
@@ -2475,11 +2462,7 @@ double HierarchicalAllocatorProcess::_quota_allocated(
return 0.;
}
- Option<Value::Scalar> used =
- roleSorter->allocationScalarQuantities(role)
- .get<Value::Scalar>(resource);
-
- return used.isSome() ? used->value() : 0;
+ return roleSorter->allocationScalarQuantities(role).get(resource).value();
}
diff --git a/src/master/allocator/sorter/drf/sorter.cpp b/src/master/allocator/sorter/drf/sorter.cpp
index 43c9767..a76888d 100644
--- a/src/master/allocator/sorter/drf/sorter.cpp
+++ b/src/master/allocator/sorter/drf/sorter.cpp
@@ -445,11 +445,11 @@ const hashmap<SlaveID, Resources>& DRFSorter::allocation(
}
-const Resources& DRFSorter::allocationScalarQuantities(
+const ResourceQuantities& DRFSorter::allocationScalarQuantities(
const string& clientPath) const
{
const Node* client = CHECK_NOTNULL(find(clientPath));
- return client->allocation.scalarQuantities;
+ return client->allocation.totals;
}
@@ -493,9 +493,9 @@ Resources DRFSorter::allocation(
}
-const Resources& DRFSorter::totalScalarQuantities() const
+const ResourceQuantities& DRFSorter::totalScalarQuantities() const
{
- return total_.scalarQuantities;
+ return total_.totals;
}
@@ -511,11 +511,11 @@ void DRFSorter::add(const SlaveID& slaveId, const Resources& resources)
total_.resources[slaveId] += resources;
- const Resources scalarQuantities =
- (resources.nonShared() + newShared).createStrippedScalarQuantity();
+ const ResourceQuantities scalarQuantities =
+ ResourceQuantities::fromScalarResources(
+ (resources.nonShared() + newShared).scalars());
- total_.scalarQuantities += scalarQuantities;
- total_.totals += ResourceQuantities::fromScalarResources(scalarQuantities);
+ total_.totals += scalarQuantities;
// We have to recalculate all shares when the total resources
// change, but we put it off until `sort` is called so that if
@@ -542,13 +542,12 @@ void DRFSorter::remove(const SlaveID& slaveId, const Resources& resources)
return !total_.resources[slaveId].contains(resource);
});
- const Resources scalarQuantities =
- (resources.nonShared() + absentShared).createStrippedScalarQuantity();
+ const ResourceQuantities scalarQuantities =
+ ResourceQuantities::fromScalarResources(
+ (resources.nonShared() + absentShared).scalars());
- CHECK(total_.scalarQuantities.contains(scalarQuantities));
- total_.scalarQuantities -= scalarQuantities;
-
- total_.totals -= ResourceQuantities::fromScalarResources(scalarQuantities);
+ CHECK(total_.totals.contains(scalarQuantities));
+ total_.totals -= scalarQuantities;
if (total_.resources[slaveId].empty()) {
total_.resources.erase(slaveId);
diff --git a/src/master/allocator/sorter/drf/sorter.hpp b/src/master/allocator/sorter/drf/sorter.hpp
index 75f90f3..9e4d036 100644
--- a/src/master/allocator/sorter/drf/sorter.hpp
+++ b/src/master/allocator/sorter/drf/sorter.hpp
@@ -86,7 +86,7 @@ public:
const hashmap<SlaveID, Resources>& allocation(
const std::string& clientPath) const override;
- const Resources& allocationScalarQuantities(
+ const ResourceQuantities& allocationScalarQuantities(
const std::string& clientPath) const override;
hashmap<std::string, Resources> allocation(
@@ -96,7 +96,7 @@ public:
const std::string& clientPath,
const SlaveID& slaveId) const override;
- const Resources& totalScalarQuantities() const override;
+ const ResourceQuantities& totalScalarQuantities() const override;
void add(const SlaveID& slaveId, const Resources& resources) override;
@@ -157,26 +157,10 @@ private:
// number of copies in the sorter.
hashmap<SlaveID, Resources> resources;
- // NOTE: Scalars can be safely aggregated across slaves. We keep
- // that to speed up the calculation of shares. See MESOS-2891 for
- // the reasons why we want to do that.
- //
- // NOTE: We omit information about dynamic reservations and
- // persistent volumes here to enable resources to be aggregated
- // across slaves more effectively. See MESOS-4833 for more
- // information.
- //
- // Sharedness info is also stripped out when resource identities
- // are omitted because sharedness inherently refers to the
- // identities of resources and not quantities.
- Resources scalarQuantities;
-
- // To improve the performance of calculating shares, we store
- // a redundant but more efficient version of `scalarQuantities`.
- // See MESOS-4694.
- //
- // TODO(bmahler): Can we remove `scalarQuantities` in favor of
- // using this type whenever scalar quantities are needed?
+ // We keep the aggregated scalar resource quantities to speed
+ // up share calculation. Note, resources shared count are ignored.
+ // Because sharedness inherently refers to the identities of resources
+ // and not quantities.
ResourceQuantities totals;
} total_;
@@ -338,13 +322,12 @@ struct DRFSorter::Node
return !resources[slaveId].contains(resource);
});
- const Resources quantitiesToAdd =
- (toAdd.nonShared() + sharedToAdd).createStrippedScalarQuantity();
+ const ResourceQuantities quantitiesToAdd =
+ ResourceQuantities::fromScalarResources(
+ (toAdd.nonShared() + sharedToAdd).scalars());
resources[slaveId] += toAdd;
- scalarQuantities += quantitiesToAdd;
-
- totals += ResourceQuantities::fromScalarResources(quantitiesToAdd);
+ totals += quantitiesToAdd;
count++;
}
@@ -365,15 +348,14 @@ struct DRFSorter::Node
return !resources[slaveId].contains(resource);
});
- const Resources quantitiesToRemove =
- (toRemove.nonShared() + sharedToRemove).createStrippedScalarQuantity();
+ const ResourceQuantities quantitiesToRemove =
+ ResourceQuantities::fromScalarResources(
+ (toRemove.nonShared() + sharedToRemove).scalars());
- totals -= ResourceQuantities::fromScalarResources(quantitiesToRemove);
+ CHECK(totals.contains(quantitiesToRemove))
+ << totals << " does not contain " << quantitiesToRemove;
- CHECK(scalarQuantities.contains(quantitiesToRemove))
- << scalarQuantities << " does not contain " << quantitiesToRemove;
-
- scalarQuantities -= quantitiesToRemove;
+ totals -= quantitiesToRemove;
if (resources[slaveId].empty()) {
resources.erase(slaveId);
@@ -385,27 +367,24 @@ struct DRFSorter::Node
const Resources& oldAllocation,
const Resources& newAllocation)
{
- const Resources oldAllocationQuantity =
- oldAllocation.createStrippedScalarQuantity();
- const Resources newAllocationQuantity =
- newAllocation.createStrippedScalarQuantity();
+ const ResourceQuantities oldAllocationQuantities =
+ ResourceQuantities::fromScalarResources(oldAllocation.scalars());
+ const ResourceQuantities newAllocationQuantities =
+ ResourceQuantities::fromScalarResources(newAllocation.scalars());
CHECK(resources.contains(slaveId));
CHECK(resources[slaveId].contains(oldAllocation))
<< "Resources " << resources[slaveId] << " at agent " << slaveId
<< " does not contain " << oldAllocation;
- CHECK(scalarQuantities.contains(oldAllocationQuantity))
- << scalarQuantities << " does not contain " << oldAllocationQuantity;
+ CHECK(totals.contains(oldAllocationQuantities))
+ << totals << " does not contain " << oldAllocationQuantities;
resources[slaveId] -= oldAllocation;
resources[slaveId] += newAllocation;
- scalarQuantities -= oldAllocationQuantity;
- scalarQuantities += newAllocationQuantity;
-
- totals -= ResourceQuantities::fromScalarResources(oldAllocationQuantity);
- totals += ResourceQuantities::fromScalarResources(newAllocationQuantity);
+ totals -= oldAllocationQuantities;
+ totals += newAllocationQuantities;
}
// We store the number of times this client has been chosen for
@@ -423,17 +402,10 @@ struct DRFSorter::Node
// not been recovered from) a specific client.
hashmap<SlaveID, Resources> resources;
- // Similarly, we aggregate scalars across slaves and omit information
- // about dynamic reservations, persistent volumes and sharedness of
- // the corresponding resource. See notes above.
- Resources scalarQuantities;
-
- // To improve the performance of calculating shares, we store
- // a redundant but more efficient version of `scalarQuantities`.
- // See MESOS-4694.
- //
- // TODO(bmahler): Can we remove `scalarQuantities` in favor of
- // using this type whenever scalar quantities are needed?
+ // We keep the aggregated scalar resource quantities to speed
+ // up share calculation. Note, resources shared count are ignored.
+ // Because sharedness inherently refers to the identities of resources
+ // and not quantities.
ResourceQuantities totals;
} allocation;
diff --git a/src/master/allocator/sorter/random/sorter.cpp b/src/master/allocator/sorter/random/sorter.cpp
index 6fcfc41..8499c69 100644
--- a/src/master/allocator/sorter/random/sorter.cpp
+++ b/src/master/allocator/sorter/random/sorter.cpp
@@ -369,11 +369,11 @@ const hashmap<SlaveID, Resources>& RandomSorter::allocation(
}
-const Resources& RandomSorter::allocationScalarQuantities(
+const ResourceQuantities& RandomSorter::allocationScalarQuantities(
const string& clientPath) const
{
const Node* client = CHECK_NOTNULL(find(clientPath));
- return client->allocation.scalarQuantities;
+ return client->allocation.totals;
}
@@ -418,9 +418,9 @@ Resources RandomSorter::allocation(
}
-const Resources& RandomSorter::totalScalarQuantities() const
+const ResourceQuantities& RandomSorter::totalScalarQuantities() const
{
- return total_.scalarQuantities;
+ return total_.totals;
}
@@ -436,11 +436,11 @@ void RandomSorter::add(const SlaveID& slaveId, const Resources& resources)
total_.resources[slaveId] += resources;
- const Resources scalarQuantities =
- (resources.nonShared() + newShared).createStrippedScalarQuantity();
+ const ResourceQuantities scalarQuantities =
+ ResourceQuantities::fromScalarResources(
+ (resources.nonShared() + newShared).scalars());
- total_.scalarQuantities += scalarQuantities;
- total_.totals += ResourceQuantities::fromScalarResources(scalarQuantities);
+ total_.totals += scalarQuantities;
}
}
@@ -461,13 +461,12 @@ void RandomSorter::remove(const SlaveID& slaveId, const Resources& resources)
return !total_.resources[slaveId].contains(resource);
});
- const Resources scalarQuantities =
- (resources.nonShared() + absentShared).createStrippedScalarQuantity();
+ const ResourceQuantities scalarQuantities =
+ ResourceQuantities::fromScalarResources(
+ (resources.nonShared() + absentShared).scalars());
- CHECK(total_.scalarQuantities.contains(scalarQuantities));
- total_.scalarQuantities -= scalarQuantities;
-
- total_.totals -= ResourceQuantities::fromScalarResources(scalarQuantities);
+ CHECK(total_.totals.contains(scalarQuantities));
+ total_.totals -= scalarQuantities;
if (total_.resources[slaveId].empty()) {
total_.resources.erase(slaveId);
diff --git a/src/master/allocator/sorter/random/sorter.hpp b/src/master/allocator/sorter/random/sorter.hpp
index 2031cb2..513abf3 100644
--- a/src/master/allocator/sorter/random/sorter.hpp
+++ b/src/master/allocator/sorter/random/sorter.hpp
@@ -85,7 +85,7 @@ public:
const hashmap<SlaveID, Resources>& allocation(
const std::string& clientPath) const override;
- const Resources& allocationScalarQuantities(
+ const ResourceQuantities& allocationScalarQuantities(
const std::string& clientPath) const override;
hashmap<std::string, Resources> allocation(
@@ -95,7 +95,7 @@ public:
const std::string& clientPath,
const SlaveID& slaveId) const override;
- const Resources& totalScalarQuantities() const override;
+ const ResourceQuantities& totalScalarQuantities() const override;
void add(const SlaveID& slaveId, const Resources& resources) override;
@@ -155,26 +155,10 @@ private:
// number of copies in the sorter.
hashmap<SlaveID, Resources> resources;
- // NOTE: Scalars can be safely aggregated across slaves. We keep
- // that to speed up the calculation of shares. See MESOS-2891 for
- // the reasons why we want to do that.
- //
- // NOTE: We omit information about dynamic reservations and
- // persistent volumes here to enable resources to be aggregated
- // across slaves more effectively. See MESOS-4833 for more
- // information.
- //
- // Sharedness info is also stripped out when resource identities
- // are omitted because sharedness inherently refers to the
- // identities of resources and not quantities.
- Resources scalarQuantities;
-
- // To improve the performance of calculating shares, we store
- // a redundant but more efficient version of `scalarQuantities`.
- // See MESOS-4694.
- //
- // TODO(bmahler): Can we remove `scalarQuantities` in favor of
- // using this type whenever scalar quantities are needed?
+ // We keep the aggregated scalar resource quantities to speed
+ // up share calculation. Note, resources shared count are ignored.
+ // Because sharedness inherently refers to the identities of resources
+ // and not quantities.
ResourceQuantities totals;
} total_;
};
@@ -325,13 +309,12 @@ struct RandomSorter::Node
return !resources[slaveId].contains(resource);
});
- const Resources quantitiesToAdd =
- (toAdd.nonShared() + sharedToAdd).createStrippedScalarQuantity();
+ const ResourceQuantities quantitiesToAdd =
+ ResourceQuantities::fromScalarResources(
+ (toAdd.nonShared() + sharedToAdd).scalars());
resources[slaveId] += toAdd;
- scalarQuantities += quantitiesToAdd;
-
- totals += ResourceQuantities::fromScalarResources(quantitiesToAdd);
+ totals += quantitiesToAdd;
}
void subtract(const SlaveID& slaveId, const Resources& toRemove)
@@ -350,15 +333,14 @@ struct RandomSorter::Node
return !resources[slaveId].contains(resource);
});
- const Resources quantitiesToRemove =
- (toRemove.nonShared() + sharedToRemove).createStrippedScalarQuantity();
+ const ResourceQuantities quantitiesToRemove =
+ ResourceQuantities::fromScalarResources(
+ (toRemove.nonShared() + sharedToRemove).scalars());
- totals -= ResourceQuantities::fromScalarResources(quantitiesToRemove);
+ CHECK(totals.contains(quantitiesToRemove))
+ << totals << " does not contain " << quantitiesToRemove;
- CHECK(scalarQuantities.contains(quantitiesToRemove))
- << scalarQuantities << " does not contain " << quantitiesToRemove;
-
- scalarQuantities -= quantitiesToRemove;
+ totals -= quantitiesToRemove;
if (resources[slaveId].empty()) {
resources.erase(slaveId);
@@ -370,27 +352,24 @@ struct RandomSorter::Node
const Resources& oldAllocation,
const Resources& newAllocation)
{
- const Resources oldAllocationQuantity =
- oldAllocation.createStrippedScalarQuantity();
- const Resources newAllocationQuantity =
- newAllocation.createStrippedScalarQuantity();
+ const ResourceQuantities oldAllocationQuantities =
+ ResourceQuantities::fromScalarResources(oldAllocation.scalars());
+ const ResourceQuantities newAllocationQuantities =
+ ResourceQuantities::fromScalarResources(newAllocation.scalars());
CHECK(resources.contains(slaveId));
CHECK(resources[slaveId].contains(oldAllocation))
<< "Resources " << resources[slaveId] << " at agent " << slaveId
<< " does not contain " << oldAllocation;
- CHECK(scalarQuantities.contains(oldAllocationQuantity))
- << scalarQuantities << " does not contain " << oldAllocationQuantity;
+ CHECK(totals.contains(oldAllocationQuantities))
+ << totals << " does not contain " << oldAllocationQuantities;
resources[slaveId] -= oldAllocation;
resources[slaveId] += newAllocation;
- scalarQuantities -= oldAllocationQuantity;
- scalarQuantities += newAllocationQuantity;
-
- totals -= ResourceQuantities::fromScalarResources(oldAllocationQuantity);
- totals += ResourceQuantities::fromScalarResources(newAllocationQuantity);
+ totals -= oldAllocationQuantities;
+ totals += newAllocationQuantities;
}
// We maintain multiple copies of each shared resource allocated
@@ -399,17 +378,10 @@ struct RandomSorter::Node
// not been recovered from) a specific client.
hashmap<SlaveID, Resources> resources;
- // Similarly, we aggregate scalars across slaves and omit information
- // about dynamic reservations, persistent volumes and sharedness of
- // the corresponding resource. See notes above.
- Resources scalarQuantities;
-
- // To improve the performance of calculating shares, we store
- // a redundant but more efficient version of `scalarQuantities`.
- // See MESOS-4694.
- //
- // TODO(bmahler): Can we remove `scalarQuantities` in favor of
- // using this type whenever scalar quantities are needed?
+ // We keep the aggregated scalar resource quantities to speed
+ // up share calculation. Note, resources shared count are ignored.
+ // Because sharedness inherently refers to the identities of resources
+ // and not quantities.
ResourceQuantities totals;
} allocation;
};
diff --git a/src/master/allocator/sorter/sorter.hpp b/src/master/allocator/sorter/sorter.hpp
index 25ad48d..d4d039f 100644
--- a/src/master/allocator/sorter/sorter.hpp
+++ b/src/master/allocator/sorter/sorter.hpp
@@ -109,9 +109,8 @@ public:
const std::string& client) const = 0;
// Returns the total scalar resource quantities that are allocated to
- // this client. This omits metadata about dynamic reservations and
- // persistent volumes; see `Resources::createStrippedScalarQuantity`.
- virtual const Resources& allocationScalarQuantities(
+ // this client.
+ virtual const ResourceQuantities& allocationScalarQuantities(
const std::string& client) const = 0;
// Returns the clients that have allocations on this slave.
@@ -124,10 +123,8 @@ public:
const std::string& client,
const SlaveID& slaveId) const = 0;
- // Returns the total scalar resource quantities in this sorter. This
- // omits metadata about dynamic reservations and persistent volumes; see
- // `Resources::createStrippedScalarQuantity`.
- virtual const Resources& totalScalarQuantities() const = 0;
+ // Returns the total scalar resource quantities in this sorter.
+ virtual const ResourceQuantities& totalScalarQuantities() const = 0;
// Add resources to the total pool of resources this
// Sorter should consider.
diff --git a/src/tests/sorter_tests.cpp b/src/tests/sorter_tests.cpp
index 1e2791f..76e3256 100644
--- a/src/tests/sorter_tests.cpp
+++ b/src/tests/sorter_tests.cpp
@@ -1628,15 +1628,17 @@ TYPED_TEST(CommonSorterTest, RemoveSharedResources)
sorter.add(
slaveId, Resources::parse("cpus:100;mem:100;disk(role1):900").get());
- Resources quantity1 = sorter.totalScalarQuantities();
+ ResourceQuantities quantity1 = sorter.totalScalarQuantities();
sorter.add(slaveId, sharedDisk);
- Resources quantity2 = sorter.totalScalarQuantities();
+ ResourceQuantities quantity2 = sorter.totalScalarQuantities();
- EXPECT_EQ(Resources::parse("disk:100").get(), quantity2 - quantity1);
+ EXPECT_EQ(
+ CHECK_NOTERROR(ResourceQuantities::fromString("disk:100")),
+ quantity2 - quantity1);
sorter.add(slaveId, sharedDisk);
- Resources quantity3 = sorter.totalScalarQuantities();
+ ResourceQuantities quantity3 = sorter.totalScalarQuantities();
EXPECT_NE(quantity1, quantity3);
EXPECT_EQ(quantity2, quantity3);
[mesos] 01/04: Simplified quota chopping logic in the allocator.
Posted by mz...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
mzhu pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mesos.git
commit fbfe1938846ef02a90dcae500a2c7843f902d1bf
Author: Meng Zhu <mz...@mesosphere.io>
AuthorDate: Fri Mar 29 15:02:02 2019 -0700
Simplified quota chopping logic in the allocator.
This patch refactors the `shrinkResources()` helper
to take in a target `ResourceQuantities`.
Due to the absent-means-zero semantic of
`class ResourceQuantities`, we also change the function
to drop (shrink to zero) resources that are absent in
the input `ResourceQuantities`.
Review: https://reviews.apache.org/r/70345
---
src/master/allocator/mesos/hierarchical.cpp | 70 +++++++++--------------------
1 file changed, 21 insertions(+), 49 deletions(-)
diff --git a/src/master/allocator/mesos/hierarchical.cpp b/src/master/allocator/mesos/hierarchical.cpp
index ef4f968..f2393ed 100644
--- a/src/master/allocator/mesos/hierarchical.cpp
+++ b/src/master/allocator/mesos/hierarchical.cpp
@@ -1633,8 +1633,7 @@ void HierarchicalAllocatorProcess::__allocate()
std::random_shuffle(slaveIds.begin(), slaveIds.end());
// Returns the result of shrinking the provided resources down to the
- // target scalar quantities. If a resource does not have a target
- // quantity provided, it will not be shrunk.
+ // target resource quantities.
//
// Note that some resources are indivisible (e.g. MOUNT volume) and
// may be excluded in entirety in order to achieve the target size
@@ -1643,27 +1642,27 @@ void HierarchicalAllocatorProcess::__allocate()
// Note also that there may be more than one result that satisfies
// the target sizes (e.g. need to exclude 1 of 2 disks); this function
// will make a random choice in these cases.
- auto shrinkResources =
- [](const Resources& resources,
- hashmap<string, Value::Scalar> targetScalarQuantites) {
- google::protobuf::RepeatedPtrField<Resource>
- resourceVector = resources;
+ auto shrinkResources = [](const Resources& resources,
+ ResourceQuantities target) {
+ if (target.empty()) {
+ return Resources();
+ }
+
+ google::protobuf::RepeatedPtrField<Resource> resourceVector = resources;
random_shuffle(resourceVector.begin(), resourceVector.end());
Resources result;
foreach (Resource& resource, resourceVector) {
- if (!targetScalarQuantites.contains(resource.name())) {
- // Resource that has no target quantity is left as is.
- result += std::move(resource);
+ Value::Scalar scalar = target.get(resource.name());
+
+ if (scalar == Value::Scalar()) {
+ // Resource that has zero quantity is dropped (shrunk to zero).
continue;
}
- Option<Value::Scalar> limitScalar =
- targetScalarQuantites.get(resource.name());
-
- if (Resources::shrink(&resource, limitScalar.get())) {
- targetScalarQuantites[resource.name()] -= resource.scalar();
+ if (Resources::shrink(&resource, scalar)) {
+ target -= ResourceQuantities::fromScalarResources(resource);
result += std::move(resource);
}
}
@@ -1910,21 +1909,9 @@ void HierarchicalAllocatorProcess::__allocate()
Resources unreserved = available.nonRevocable().unreserved();
// First, allocate resources up to a role's quota guarantee.
-
- hashmap<string, Value::Scalar> unsatisfiedQuotaGuaranteeScalarLimit;
- foreach (const string& name, unsatisfiedQuotaGuarantee.names()) {
- unsatisfiedQuotaGuaranteeScalarLimit[name] +=
- CHECK_NOTNONE(unsatisfiedQuotaGuarantee.get<Value::Scalar>(name));
- }
-
- Resources newQuotaAllocation =
- unreserved.filter([&](const Resource& resource) {
- return
- unsatisfiedQuotaGuaranteeScalarLimit.contains(resource.name());
- });
-
- newQuotaAllocation = shrinkResources(newQuotaAllocation,
- unsatisfiedQuotaGuaranteeScalarLimit);
+ Resources newQuotaAllocation = shrinkResources(
+ unreserved,
+ ResourceQuantities::fromScalarResources(unsatisfiedQuotaGuarantee));
toAllocate += newQuotaAllocation;
@@ -1950,26 +1937,11 @@ void HierarchicalAllocatorProcess::__allocate()
}
);
- // Allocation Limit = Available Headroom - Required Headroom
- Resources headroomResourcesLimit = availableHeadroom - requiredHeadroom;
-
- hashmap<string, Value::Scalar> headroomScalarLimit;
- foreach (const string& name, headroomResourcesLimit.names()) {
- headroomScalarLimit[name] =
- CHECK_NOTNONE(headroomResourcesLimit.get<Value::Scalar>(name));
- }
-
- // If a resource type is absent in `headroomScalarLimit`, it means this
- // type of resource is already in quota headroom deficit and we make
- // no more allocations. They are filtered out.
- nonQuotaGuaranteeResources = nonQuotaGuaranteeResources.filter(
- [&] (const Resource& resource) {
- return headroomScalarLimit.contains(resource.name());
- }
- );
+ Resources surplusHeadroom = availableHeadroom - requiredHeadroom;
- nonQuotaGuaranteeResources =
- shrinkResources(nonQuotaGuaranteeResources, headroomScalarLimit);
+ nonQuotaGuaranteeResources = shrinkResources(
+ nonQuotaGuaranteeResources,
+ ResourceQuantities::fromScalarResources(surplusHeadroom.scalars()));
toAllocate += nonQuotaGuaranteeResources;