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/09/12 22:11:39 UTC
[mesos] branch master updated (4f7ba41 -> 783fd45)
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 4f7ba41 Activated tests for `src/python/lib`.
new 2ec34ca Added tracking of framework allocations in the allocator Slave class.
new fdaabac Removed `Sorter::allocation(const SlaveID& slaveId)`.
new 0900329 Improved allocator inverse offer test.
new 783fd45 Tracked offered and allocated resources in the role tree.
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/mesos.proto | 4 +
include/mesos/v1/mesos.proto | 4 +
src/master/allocator/mesos/hierarchical.cpp | 277 +++++++++++----------
src/master/allocator/mesos/hierarchical.hpp | 96 +++++--
src/master/allocator/mesos/sorter/drf/sorter.cpp | 26 --
src/master/allocator/mesos/sorter/drf/sorter.hpp | 3 -
.../allocator/mesos/sorter/random/sorter.cpp | 27 --
.../allocator/mesos/sorter/random/sorter.hpp | 3 -
src/master/allocator/mesos/sorter/sorter.hpp | 4 -
src/tests/hierarchical_allocator_tests.cpp | 66 +++--
src/tests/sorter_tests.cpp | 13 -
11 files changed, 275 insertions(+), 248 deletions(-)
[mesos] 03/04: Improved allocator inverse offer test.
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 09003294faab3cd3114f24e747c252a4c7433de0
Author: Meng Zhu <mz...@mesosphere.io>
AuthorDate: Thu Sep 5 14:00:28 2019 -0700
Improved allocator inverse offer test.
The test is augmented to also check that a framework that
declined offers from an agent will not get inverse offers
for that agent.
Review: https://reviews.apache.org/r/71440
---
src/tests/hierarchical_allocator_tests.cpp | 66 +++++++++++++++++++++++-------
1 file changed, 51 insertions(+), 15 deletions(-)
diff --git a/src/tests/hierarchical_allocator_tests.cpp b/src/tests/hierarchical_allocator_tests.cpp
index 2c1d0fe..5f2e2b2 100644
--- a/src/tests/hierarchical_allocator_tests.cpp
+++ b/src/tests/hierarchical_allocator_tests.cpp
@@ -1055,6 +1055,8 @@ TEST_F(HierarchicalAllocatorTest, SmallOfferFilterTimeout)
// This test ensures that agents which are scheduled for maintenance are
// properly sent inverse offers after they have accepted or reserved resources.
+// It also verifies that the frameworks declined the offer should get no
+// inverse offers.
TEST_F(HierarchicalAllocatorTest, MaintenanceInverseOffers)
{
// Pausing the clock is not necessary, but ensures that the test
@@ -1064,40 +1066,71 @@ TEST_F(HierarchicalAllocatorTest, MaintenanceInverseOffers)
initialize();
- // Create an agent.
- SlaveInfo agent = createSlaveInfo("cpus:2;mem:1024;disk:0");
+ // Create an agent which is about to enter maintenance.
+ SlaveInfo agent1 = createSlaveInfo("cpus:2;mem:1024;disk:0");
allocator->addSlave(
- agent.id(),
- agent,
+ agent1.id(),
+ agent1,
AGENT_CAPABILITIES(),
None(),
- agent.resources(),
+ agent1.resources(),
{});
// This framework will be offered all of the resources.
- FrameworkInfo framework = createFrameworkInfo({"*"});
- allocator->addFramework(framework.id(), framework, {}, true, {});
+ FrameworkInfo framework1 = createFrameworkInfo({"*"});
+ allocator->addFramework(framework1.id(), framework1, {}, true, {});
// Check that the resources go to the framework.
Allocation expected = Allocation(
- framework.id(),
- {{"*", {{agent.id(), agent.resources()}}}});
+ framework1.id(),
+ {{"*", {{agent1.id(), agent1.resources()}}}});
+
+ AWAIT_EXPECT_EQ(expected, allocations.get());
+
+ // Create another agent and framework.
+ //
+ // This framework will be offered all of 2nd agent resources.
+ FrameworkInfo framework2 = createFrameworkInfo({"*"});
+ allocator->addFramework(framework2.id(), framework2, {}, true, {});
+
+ SlaveInfo agent2 = createSlaveInfo("cpus:2;mem:1024;disk:0");
+ allocator->addSlave(
+ agent2.id(),
+ agent2,
+ AGENT_CAPABILITIES(),
+ None(),
+ agent2.resources(),
+ {});
+
+ // Check that the resources go to the framework.
+ expected =
+ Allocation(framework2.id(), {{"*", {{agent2.id(), agent2.resources()}}}});
AWAIT_EXPECT_EQ(expected, allocations.get());
+ // Recover the offer allocated to framework2.
+ Filters filter1day;
+ filter1day.set_refuse_seconds(Days(1).secs());
+
+ allocator->recoverResources(
+ framework2.id(),
+ agent2.id(),
+ allocatedResources(agent2.resources(), "*"),
+ filter1day);
+
const process::Time start = Clock::now() + Seconds(60);
- // Give the agent some unavailability.
+ // Give both agents some unavailability.
allocator->updateUnavailability(
- agent.id(),
- protobuf::maintenance::createUnavailability(
- start));
+ agent1.id(), protobuf::maintenance::createUnavailability(start));
+ allocator->updateUnavailability(
+ agent2.id(), protobuf::maintenance::createUnavailability(start));
// Check the resources get inverse offered.
Future<Deallocation> deallocation = deallocations.get();
AWAIT_READY(deallocation);
- EXPECT_EQ(framework.id(), deallocation->frameworkId);
- EXPECT_TRUE(deallocation->resources.contains(agent.id()));
+ EXPECT_EQ(framework1.id(), deallocation->frameworkId);
+ EXPECT_TRUE(deallocation->resources.contains(agent1.id()));
foreachvalue (
const UnavailableResources& unavailableResources,
@@ -1110,6 +1143,9 @@ TEST_F(HierarchicalAllocatorTest, MaintenanceInverseOffers)
start.duration(),
Nanoseconds(unavailableResources.unavailability.start().nanoseconds()));
}
+
+ // Ensure only one offer should be deallocated.
+ EXPECT_TRUE(deallocations.get().isPending());
}
[mesos] 04/04: Tracked offered and allocated resources in the role
tree.
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 783fd45c548fdff0c5c4812bc8e92c3aed202e06
Author: Meng Zhu <mz...@mesosphere.io>
AuthorDate: Sat Sep 7 16:01:51 2019 -0700
Tracked offered and allocated resources in the role tree.
This helpers simplify the quota tracking logic and also paves
the way to reduce duplicated states in the sorter.
Also documented that shared resources must be uniquely
identifiable.
Small performance degradation when making allocations due to
duplicated map construction in `(un)trackAllocatedResources`.
This will be removed once embeded the sorter in the role tree.
Benchmark `LargeAndSmallQuota/2`:
Master:
Added 3000 agents in 80.648188ms
Added 3000 frameworks in 19.7006984secs
Benchmark setup: 3000 agents, 3000 roles, 3000 frameworks,
with drf sorter
Made 3500 allocations in 16.044274434secs
Made 0 allocation in 14.476429451secs
Master + this patch:
Added 3000 agents in 80.110817ms
Added 3000 frameworks in 17.25974094secs
Benchmark setup: 3000 agents, 3000 roles, 3000 frameworks,
with drf sorter
Made 3500 allocations in 16.91971379secs
Made 0 allocation in 13.784476154secs
Review: https://reviews.apache.org/r/71460
---
include/mesos/mesos.proto | 4 +
include/mesos/v1/mesos.proto | 4 +
src/master/allocator/mesos/hierarchical.cpp | 125 +++++++++++++++++-----------
src/master/allocator/mesos/hierarchical.hpp | 22 +++++
4 files changed, 106 insertions(+), 49 deletions(-)
diff --git a/include/mesos/mesos.proto b/include/mesos/mesos.proto
index 8fd838e..0cab154 100644
--- a/include/mesos/mesos.proto
+++ b/include/mesos/mesos.proto
@@ -1565,6 +1565,10 @@ message Resource {
// can be launched using this resource and all of them shall refer
// to the same physical resource on the cluster. Note that only
// persistent volumes can be shared currently.
+ //
+ // NOTE: Different shared resources must be uniquely identifiable.
+ // This currently holds as persistent volume should have unique `id`
+ // (this is not validated for enforced though).
optional SharedInfo shared = 10;
}
diff --git a/include/mesos/v1/mesos.proto b/include/mesos/v1/mesos.proto
index da19256..a2f6dbb 100644
--- a/include/mesos/v1/mesos.proto
+++ b/include/mesos/v1/mesos.proto
@@ -1553,6 +1553,10 @@ message Resource {
// can be launched using this resource and all of them shall refer
// to the same physical resource on the cluster. Note that only
// persistent volumes can be shared currently.
+ //
+ // NOTE: Different shared resources must be uniquely identifiable.
+ // This currently holds as persistent volume should have unique `id`
+ // (this is not validated for enforced though).
optional SharedInfo shared = 10;
}
diff --git a/src/master/allocator/mesos/hierarchical.cpp b/src/master/allocator/mesos/hierarchical.cpp
index 8432a62..256fdce 100644
--- a/src/master/allocator/mesos/hierarchical.cpp
+++ b/src/master/allocator/mesos/hierarchical.cpp
@@ -249,6 +249,18 @@ Option<const Role*> RoleTree::get(const std::string& role) const
}
+Option<Role*> RoleTree::get_(const std::string& role)
+{
+ auto found = roles_.find(role);
+
+ if (found == roles_.end()) {
+ return None();
+ } else {
+ return &(found->second);
+ }
+}
+
+
Role& RoleTree::operator[](const std::string& rolePath)
{
if (roles_.contains(rolePath)) {
@@ -311,6 +323,9 @@ bool RoleTree::tryRemove(const std::string& role)
(*metrics)->removeRole(current->role);
}
+ CHECK(current->offeredOrAllocatedScalars_.empty())
+ << " role: " << role
+ << " offeredOrAllocated: " << current->offeredOrAllocatedScalars_;
roles_.erase(current->role);
current = parent;
@@ -401,6 +416,47 @@ void RoleTree::updateWeight(const string& role, double weight)
}
+void RoleTree::trackOfferedOrAllocated(const Resources& resources_)
+{
+ // TODO(mzhu): avoid building a map by traversing `resources`
+ // and look for the allocation role of individual resource.
+ // However, due to MESOS-9242, this currently does not work
+ // as traversing resources would lose the shared count.
+ foreachpair (
+ const string& role,
+ const Resources& resources,
+ resources_.scalars().allocations()) {
+ // Track it hierarchically up to the root.
+ for (Role* current = CHECK_NOTNONE(get_(role)); current != nullptr;
+ current = current->parent) {
+ current->offeredOrAllocatedScalars_ += resources;
+ }
+ }
+}
+
+
+void RoleTree::untrackOfferedOrAllocated(const Resources& resources_)
+{
+ // TODO(mzhu): avoid building a map by traversing `resources`
+ // and look for the allocation role of individual resource.
+ // However, due to MESOS-9242, this currently does not work
+ // as traversing resources would lose the shared count.
+ foreachpair (
+ const string& role,
+ const Resources& resources,
+ resources_.scalars().allocations()) {
+ // Untrack it hierarchically up to the root.
+ for (Role* current = CHECK_NOTNONE(get_(role)); current != nullptr;
+ current = current->parent) {
+ CHECK_CONTAINS(current->offeredOrAllocatedScalars_, resources)
+ << " Role: " << current->role
+ << " offeredOrAllocated: " << current->offeredOrAllocatedScalars_;
+ current->offeredOrAllocatedScalars_ -= resources;
+ }
+ }
+}
+
+
std::string RoleTree::toJSON() const
{
std::function<void(JSON::ObjectWriter*, const Role*)> json =
@@ -412,6 +468,8 @@ std::string RoleTree::toJSON() const
writer->field("limits", role->quota_.limits);
writer->field(
"reservation_quantities", role->reservationScalarQuantities_);
+ writer->field(
+ "offered_or_allocated_scalars", role->offeredOrAllocatedScalars_);
writer->field("frameworks", [&](JSON::ArrayWriter* writer) {
foreach (const FrameworkID& id, role->frameworks_) {
@@ -1108,15 +1166,16 @@ void HierarchicalAllocatorProcess::updateAllocation(
// foreach (const ResourceConversion& conversion, conversions) {
// CHECK_NONE(validateConversionOnAllocatedResources(conversion));
// }
- Try<Resources> _updatedOfferedResources = offeredResources.apply(conversions);
- CHECK_SOME(_updatedOfferedResources);
-
- const Resources& updatedOfferedResources = _updatedOfferedResources.get();
+ Resources updatedOfferedResources =
+ CHECK_NOTERROR(offeredResources.apply(conversions));
// Update the per-slave allocation.
slave.increaseAvailable(frameworkId, offeredResources);
slave.decreaseAvailable(frameworkId, updatedOfferedResources);
+ roleTree.untrackOfferedOrAllocated(offeredResources);
+ roleTree.trackOfferedOrAllocated(updatedOfferedResources);
+
// Update the allocation in the framework sorter.
frameworkSorter->update(
frameworkId.value(),
@@ -1804,7 +1863,7 @@ void HierarchicalAllocatorProcess::__generateOffers()
//
// Consumed Quota = reservations + unreserved allocation
- // First add reservations.
+ // Add reservations and unreserved offeredOrAllocated.
//
// Currently, only top level roles can have quota set and thus
// we only track consumed quota for top level roles.
@@ -1813,34 +1872,10 @@ void HierarchicalAllocatorProcess::__generateOffers()
// these as metrics.
if (r->quota() != DEFAULT_QUOTA) {
logHeadroomInfo = true;
- // Note, `reservationScalarQuantities` in `struct role`
- // is hierarchical aware, thus it also includes subrole reservations.
- rolesConsumedQuota[r->role] += r->reservationScalarQuantities();
- }
- }
-
- // Then add the unreserved allocation.
- //
- // TODO(mzhu): make allocation tracking hierarchical, so that we only
- // need to look at the top-level node.
- foreachpair (const string& role, const Role& r, roleTree.roles()) {
- if (r.frameworks().empty()) {
- continue;
- }
-
- const string& topLevelRole =
- strings::contains(role, "/") ? role.substr(0, role.find('/')) : role;
-
- if (getQuota(topLevelRole) == DEFAULT_QUOTA) {
- continue;
- }
-
- if (roleSorter->contains(role)) {
- foreachvalue (const Resources& resources, roleSorter->allocation(role)) {
- rolesConsumedQuota[topLevelRole] +=
- ResourceQuantities::fromScalarResources(
- resources.unreserved().nonRevocable().scalars());
- }
+ rolesConsumedQuota[r->role] +=
+ r->reservationScalarQuantities() +
+ ResourceQuantities::fromScalarResources(
+ r->offeredOrAllocatedScalars().unreserved().nonRevocable());
}
}
@@ -1882,24 +1917,12 @@ void HierarchicalAllocatorProcess::__generateOffers()
// Subtract allocated resources from the total.
availableHeadroom -= roleSorter->allocationScalarQuantities();
- // TODO(mzhu): make allocation tracking hierarchical, so that we only
- // need to look at the top-level node.
- ResourceQuantities totalOfferedOrAllocatedReservation;
- foreachkey (const string& role, roleTree.roles()) {
- if (!roleSorter->contains(role)) {
- continue; // This role has no allocation.
- }
-
- foreachvalue (const Resources& resources, roleSorter->allocation(role)) {
- totalOfferedOrAllocatedReservation +=
- ResourceQuantities::fromScalarResources(resources.reserved().scalars());
- }
- }
-
// Subtract total unallocated reservations.
// unallocated reservations = total reservations - allocated reservations
- availableHeadroom -= roleTree.root()->reservationScalarQuantities() -
- totalOfferedOrAllocatedReservation;
+ availableHeadroom -=
+ roleTree.root()->reservationScalarQuantities() -
+ ResourceQuantities::fromScalarResources(
+ roleTree.root()->offeredOrAllocatedScalars().reserved());
// Subtract revocable resources.
foreachvalue (const Slave& slave, slaves) {
@@ -2948,6 +2971,8 @@ void HierarchicalAllocatorProcess::trackAllocatedResources(
CHECK_CONTAINS(*frameworkSorter, frameworkId.value())
<< " for role " << role;
+ roleTree.trackOfferedOrAllocated(allocation);
+
roleSorter->allocated(role, slaveId, allocation);
frameworkSorter->allocated(
frameworkId.value(), slaveId, allocation);
@@ -2980,6 +3005,8 @@ void HierarchicalAllocatorProcess::untrackAllocatedResources(
CHECK_CONTAINS(*frameworkSorter, frameworkId.value())
<< "for role " << role;
+ roleTree.untrackOfferedOrAllocated(allocation);
+
frameworkSorter->unallocated(
frameworkId.value(), slaveId, allocation);
diff --git a/src/master/allocator/mesos/hierarchical.hpp b/src/master/allocator/mesos/hierarchical.hpp
index 9e6570a..5ea3791 100644
--- a/src/master/allocator/mesos/hierarchical.hpp
+++ b/src/master/allocator/mesos/hierarchical.hpp
@@ -122,6 +122,11 @@ public:
return reservationScalarQuantities_;
}
+ const Resources& offeredOrAllocatedScalars() const
+ {
+ return offeredOrAllocatedScalars_;
+ }
+
const hashset<FrameworkID>& frameworks() const { return frameworks_; }
const Quota& quota() const { return quota_; }
@@ -166,6 +171,13 @@ private:
// IDs of the frameworks subscribed to the role, if any.
hashset<FrameworkID> frameworks_;
+ // Total allocated or offered scalar resources to this role, including
+ // meta data. This field dose not affect role's lifecycle. However, since
+ // any offered or allocated resources should be tied to a framework,
+ // an empty role (that has no registered framework) must have
+ // empty offeredOrAllocated resources.
+ Resources offeredOrAllocatedScalars_;
+
// Aggregated reserved scalar resource quantities on all agents tied to this
// role, if any. This includes both its own reservations as well as
// reservations of any of its subroles (i.e. it is hierarchical aware).
@@ -217,10 +229,16 @@ public:
void updateWeight(const std::string& role, double weight);
+ void trackOfferedOrAllocated(const Resources& resources);
+ void untrackOfferedOrAllocated(const Resources& resources);
+
// Dump the role tree state in JSON format for debugging.
std::string toJSON() const;
private:
+ // Private helper to get non-const pointers.
+ Option<Role*> get_(const std::string& role);
+
// Lookup or add the role struct associated with the role. Ancestor roles
// along the tree path will be created if necessary.
Role& operator[](const std::string& role);
@@ -798,6 +816,8 @@ private:
//
// TODO(asekretenko): rename `(un)trackAllocatedResources()` to reflect the
// fact that these methods do not distinguish between offered and allocated.
+ //
+ // TODO(mzhu): replace this with `RoleTree::trackOfferedOrAllocated`.
void trackAllocatedResources(
const SlaveID& slaveId,
const FrameworkID& frameworkId,
@@ -805,6 +825,8 @@ private:
// Helper to untrack resources that are no longer offered or allocated
// on an agent.
+ //
+ // TODO(mzhu): replace this with `RoleTree::untrackOfferedOrAllocated`.
void untrackAllocatedResources(
const SlaveID& slaveId,
const FrameworkID& frameworkId,
[mesos] 01/04: Added tracking of framework allocations in the
allocator Slave class.
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 2ec34ca5951a5a8da3d1ab93839cce68e815c1d5
Author: Meng Zhu <mz...@mesosphere.io>
AuthorDate: Tue Sep 3 13:31:36 2019 -0700
Added tracking of framework allocations in the allocator Slave class.
This would simplify the tracking logic regarding
resource allocations in the allocator. See MESOS-9182.
Negligible performance impact:
Master:
BENCHMARK_HierarchicalAllocator_WithQuotaParam.LargeAndSmallQuota/2
Added 3000 agents in 77.999483ms
Added 3000 frameworks in 16.736076171secs
Benchmark setup: 3000 agents, 3000 roles, 3000 frameworks,
with drf sorter
Made 3500 allocations in 15.342376944secs
Made 0 allocation in 13.96720191secs
Master + this patch:
BENCHMARK_HierarchicalAllocator_WithQuotaParam.LargeAndSmallQuota/2
Added 3000 agents in 83.597048ms
Added 3000 frameworks in 16.757011745secs
Benchmark setup: 3000 agents, 3000 roles, 3000 frameworks,
with drf sorter
Made 3500 allocations in 15.566366241secs
Made 0 allocation in 14.022591871secs
Review: https://reviews.apache.org/r/68508
---
src/master/allocator/mesos/hierarchical.cpp | 31 ++++++------
src/master/allocator/mesos/hierarchical.hpp | 74 ++++++++++++++++++-----------
2 files changed, 63 insertions(+), 42 deletions(-)
diff --git a/src/master/allocator/mesos/hierarchical.cpp b/src/master/allocator/mesos/hierarchical.cpp
index 187de17..a477e50 100644
--- a/src/master/allocator/mesos/hierarchical.cpp
+++ b/src/master/allocator/mesos/hierarchical.cpp
@@ -791,7 +791,7 @@ void HierarchicalAllocatorProcess::addSlave(
protobuf::slave::Capabilities(capabilities),
true,
total,
- Resources::sum(used))});
+ used)});
Slave& slave = *CHECK_NOTNONE(getSlave(slaveId));
@@ -854,7 +854,7 @@ void HierarchicalAllocatorProcess::addSlave(
LOG(INFO)
<< "Added agent " << slaveId << " (" << slave.info.hostname() << ")"
<< " with " << slave.getTotal()
- << " (offered or allocated: " << slave.getOfferedOrAllocated() << ")";
+ << " (offered or allocated: " << slave.getTotalOfferedOrAllocated() << ")";
generateOffers(slaveId);
}
@@ -964,6 +964,9 @@ void HierarchicalAllocatorProcess::addResourceProvider(
{
CHECK(initialized);
+ Slave& slave = *CHECK_NOTNONE(getSlave(slaveId));
+ updateSlaveTotal(slaveId, slave.getTotal() + total);
+
foreachpair (const FrameworkID& frameworkId,
const Resources& allocation,
used) {
@@ -982,13 +985,10 @@ void HierarchicalAllocatorProcess::addResourceProvider(
continue;
}
+ slave.decreaseAvailable(frameworkId, allocation);
trackAllocatedResources(slaveId, frameworkId, allocation);
}
- Slave& slave = *CHECK_NOTNONE(getSlave(slaveId));
- updateSlaveTotal(slaveId, slave.getTotal() + total);
- slave.decreaseAvailable(Resources::sum(used));
-
VLOG(1)
<< "Grew agent " << slaveId << " by "
<< total << " (total), "
@@ -1114,8 +1114,8 @@ void HierarchicalAllocatorProcess::updateAllocation(
const Resources& updatedOfferedResources = _updatedOfferedResources.get();
// Update the per-slave allocation.
- slave.increaseAvailable(offeredResources);
- slave.decreaseAvailable(updatedOfferedResources);
+ slave.increaseAvailable(frameworkId, offeredResources);
+ slave.decreaseAvailable(frameworkId, updatedOfferedResources);
// Update the allocation in the framework sorter.
frameworkSorter->update(
@@ -1442,16 +1442,17 @@ void HierarchicalAllocatorProcess::recoverResources(
Option<Slave*> slave = getSlave(slaveId);
if (slave.isSome()) {
- CHECK((*slave)->getOfferedOrAllocated().contains(resources))
+ CHECK((*slave)->getTotalOfferedOrAllocated().contains(resources))
<< "agent " << slaveId << " resources "
- << (*slave)->getOfferedOrAllocated() << " do not contain " << resources;
+ << (*slave)->getTotalOfferedOrAllocated() << " do not contain "
+ << resources;
- (*slave)->increaseAvailable(resources);
+ (*slave)->increaseAvailable(frameworkId, resources);
VLOG(1) << "Recovered " << resources
<< " (total: " << (*slave)->getTotal()
<< ", offered or allocated: "
- << (*slave)->getOfferedOrAllocated() << ")"
+ << (*slave)->getTotalOfferedOrAllocated() << ")"
<< " on agent " << slaveId
<< " from framework " << frameworkId;
}
@@ -2168,7 +2169,7 @@ void HierarchicalAllocatorProcess::__generateOffers()
ResourceQuantities::fromScalarResources(guaranteesOffering);
availableHeadroom -= increasedQuotaConsumption;
- slave.decreaseAvailable(toOffer);
+ slave.decreaseAvailable(frameworkId, toOffer);
trackAllocatedResources(slaveId, frameworkId, toOffer);
}
@@ -2315,7 +2316,7 @@ void HierarchicalAllocatorProcess::__generateOffers()
availableHeadroom -= increasedQuotaConsumption;
- slave.decreaseAvailable(toOffer);
+ slave.decreaseAvailable(frameworkId, toOffer);
trackAllocatedResources(slaveId, frameworkId, toOffer);
}
@@ -2654,7 +2655,7 @@ double HierarchicalAllocatorProcess::_resources_offered_or_allocated(
foreachvalue (const Slave& slave, slaves) {
Option<Value::Scalar> value =
- slave.getOfferedOrAllocated().get<Value::Scalar>(resource);
+ slave.getTotalOfferedOrAllocated().get<Value::Scalar>(resource);
if (value.isSome()) {
offered_or_allocated += value->value();
diff --git a/src/master/allocator/mesos/hierarchical.hpp b/src/master/allocator/mesos/hierarchical.hpp
index 48ba399..9e6570a 100644
--- a/src/master/allocator/mesos/hierarchical.hpp
+++ b/src/master/allocator/mesos/hierarchical.hpp
@@ -253,12 +253,13 @@ public:
const protobuf::slave::Capabilities& _capabilities,
bool _activated,
const Resources& _total,
- const Resources& _offeredOrAllocated)
+ const hashmap<FrameworkID, Resources>& _offeredOrAllocated)
: info(_info),
capabilities(_capabilities),
activated(_activated),
total(_total),
offeredOrAllocated(_offeredOrAllocated),
+ totalOfferedOrAllocated(Resources::sum(_offeredOrAllocated)),
shared(_total.shared()),
hasGpu_(_total.gpus().getOrElse(0) > 0)
{
@@ -267,7 +268,15 @@ public:
const Resources& getTotal() const { return total; }
- const Resources& getOfferedOrAllocated() const { return offeredOrAllocated; }
+ const hashmap<FrameworkID, Resources>& getOfferedOrAllocated() const
+ {
+ return offeredOrAllocated;
+ }
+
+ const Resources& getTotalOfferedOrAllocated() const
+ {
+ return totalOfferedOrAllocated;
+ }
const Resources& getAvailable() const { return available; }
@@ -281,16 +290,30 @@ public:
updateAvailable();
}
- void decreaseAvailable(const Resources& offeredOrAllocated_)
+ void increaseAvailable(
+ const FrameworkID& frameworkId, const Resources& offeredOrAllocated_)
{
- offeredOrAllocated += offeredOrAllocated_;
+ // Increasing available is to subtract offered or allocated.
+
+ Resources& resources = offeredOrAllocated.at(frameworkId);
+ resources -= offeredOrAllocated_;
+ if (resources.empty()) {
+ offeredOrAllocated.erase(frameworkId);
+ }
+
+ totalOfferedOrAllocated -= offeredOrAllocated_;
updateAvailable();
}
- void increaseAvailable(const Resources& offeredOrAllocated_)
+ void decreaseAvailable(
+ const FrameworkID& frameworkId, const Resources& offeredOrAllocated_)
{
- offeredOrAllocated -= offeredOrAllocated_;
+ // Decreasing available is to add offered or allocated.
+
+ offeredOrAllocated[frameworkId] += offeredOrAllocated_;
+
+ totalOfferedOrAllocated += offeredOrAllocated_;
updateAvailable();
}
@@ -342,45 +365,42 @@ public:
Option<Maintenance> maintenance;
private:
- void updateAvailable() {
+ void updateAvailable()
+ {
// In order to subtract from the total,
// we strip the allocation information.
- Resources offeredOrAllocated_ = offeredOrAllocated;
- offeredOrAllocated_.unallocate();
+ Resources totalOfferedOrAllocated_ = totalOfferedOrAllocated;
+ totalOfferedOrAllocated_.unallocate();
- // Calling `nonShared()` currently copies the underlying resources
- // and is therefore rather expensive. We avoid it in the common
- // case that there are no shared resources.
- //
- // TODO(mzhu): Ideally there would be a single logical path here.
- // One solution is to have `Resources` be copy-on-write such that
- // `nonShared()` performs no copying and instead points to a
- // subset of the original `Resource` objects.
+ // This is hot path. We avoid the unnecessary resource traversals
+ // in the common case where there are no shared resources.
if (shared.empty()) {
- available = total - offeredOrAllocated_;
+ available = total - totalOfferedOrAllocated_;
} else {
// Since shared resources are offerable even when they are in use, we
// always include them as part of available resources.
available =
- (total.nonShared() - offeredOrAllocated_.nonShared()) + shared;
+ (total.nonShared() - totalOfferedOrAllocated_.nonShared()) + shared;
}
}
// Total amount of regular *and* oversubscribed resources.
Resources total;
- // Regular *and* oversubscribed resources that are offered or allocated.
- //
- // NOTE: We maintain multiple copies of each shared resource allocated
- // to a slave, where the number of copies represents the number of times
- // this shared resource has been allocated to (and has not been recovered
- // from) a specific framework.
- //
// NOTE: We keep track of the slave's allocated resources despite
// having that information in sorters. This is because the
// information in sorters is not accurate if some framework
// hasn't reregistered. See MESOS-2919 for details.
- Resources offeredOrAllocated;
+ //
+ // This includes both regular *and* oversubscribed resources.
+ //
+ // An entry is erased if a framework no longer has any
+ // offered or allocated on the agent.
+ hashmap<FrameworkID, Resources> offeredOrAllocated;
+
+ // Sum of all offered or allocated resources on the agent. This should equal
+ // to sum of `offeredOrAllocated` (including all the meta-data).
+ Resources totalOfferedOrAllocated;
// We track the total and allocated resources on the slave to
// avoid calculating it in place every time.
[mesos] 02/04: Removed `Sorter::allocation(const SlaveID& slaveId)`.
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 fdaabac781d62f7c969064009e6dc8e109617b74
Author: Meng Zhu <mz...@mesosphere.io>
AuthorDate: Tue Sep 3 13:44:39 2019 -0700
Removed `Sorter::allocation(const SlaveID& slaveId)`.
This paves the way for removing the allocation info tracking
in the sorter (in favor of tracking them in the allocator).
Review: https://reviews.apache.org/r/71428
---
src/master/allocator/mesos/hierarchical.cpp | 121 ++++++++++-----------
src/master/allocator/mesos/sorter/drf/sorter.cpp | 26 -----
src/master/allocator/mesos/sorter/drf/sorter.hpp | 3 -
.../allocator/mesos/sorter/random/sorter.cpp | 27 -----
.../allocator/mesos/sorter/random/sorter.hpp | 3 -
src/master/allocator/mesos/sorter/sorter.hpp | 4 -
src/tests/sorter_tests.cpp | 13 ---
7 files changed, 55 insertions(+), 142 deletions(-)
diff --git a/src/master/allocator/mesos/hierarchical.cpp b/src/master/allocator/mesos/hierarchical.cpp
index a477e50..8432a62 100644
--- a/src/master/allocator/mesos/hierarchical.cpp
+++ b/src/master/allocator/mesos/hierarchical.cpp
@@ -2348,74 +2348,63 @@ void HierarchicalAllocatorProcess::generateInverseOffers()
// want the master to create `InverseOffer`s from.
hashmap<FrameworkID, hashmap<SlaveID, UnavailableResources>> offerable;
- // For maintenance, we use the framework sorters to determine which frameworks
- // have (1) reserved and / or (2) unreserved resource on the specified
- // slaveIds. This way we only send inverse offers to frameworks that have the
- // potential to lose something. We keep track of which frameworks already have
- // an outstanding inverse offer for the given slave in the
- // UnavailabilityStatus of the specific slave using the `offerOutstanding`
- // flag. This is equivalent to the accounting we do for resources when we send
- // regular offers. If we didn't keep track of outstanding offers then we would
- // keep generating new inverse offers even though the framework had not
- // responded yet.
-
- foreachvalue (const Owned<Sorter>& frameworkSorter, frameworkSorters) {
- foreach (const SlaveID& slaveId, allocationCandidates) {
- Slave& slave = *CHECK_NOTNONE(getSlave(slaveId));
-
- if (slave.maintenance.isSome()) {
- // We use a reference by alias because we intend to modify the
- // `maintenance` and to improve readability.
- Slave::Maintenance& maintenance = slave.maintenance.get();
-
- hashmap<string, Resources> allocation =
- frameworkSorter->allocation(slaveId);
-
- foreachkey (const string& frameworkId_, allocation) {
- FrameworkID frameworkId;
- frameworkId.set_value(frameworkId_);
-
- const Framework& framework =
- *CHECK_NOTNONE(getFramework(frameworkId));
-
- // No need to deallocate for an inactive framework as the master
- // will not send it inverse offers.
- if (!framework.active) {
- continue;
- }
+ // For maintenance, we only send inverse offers to frameworks that have the
+ // potential to lose something (i.e. it has resources offered or allocated on
+ // a given agent). We keep track of which frameworks already have an
+ // outstanding inverse offer for the given slave in the UnavailabilityStatus
+ // of the specific slave using the `offerOutstanding` flag. This is equivalent
+ // to the accounting we do for resources when we send regular offers. If we
+ // didn't keep track of outstanding offers then we would keep generating new
+ // inverse offers even though the framework had not responded yet.
+ //
+ // TODO(mzhu): Need to consider reservations as well.
+ foreach (const SlaveID& slaveId, allocationCandidates) {
+ Slave& slave = *CHECK_NOTNONE(getSlave(slaveId));
+
+ if (slave.maintenance.isSome()) {
+ // We use a reference by alias because we intend to modify the
+ // `maintenance` and to improve readability.
+ Slave::Maintenance& maintenance = slave.maintenance.get();
+
+ foreachkey (
+ const FrameworkID& frameworkId, slave.getOfferedOrAllocated()) {
+ const Framework& framework = *CHECK_NOTNONE(getFramework(frameworkId));
+
+ // No need to deallocate for an inactive framework as the master
+ // will not send it inverse offers.
+ if (!framework.active) {
+ continue;
+ }
- // If this framework doesn't already have inverse offers for the
- // specified slave.
- if (!offerable[frameworkId].contains(slaveId)) {
- // If there isn't already an outstanding inverse offer to this
- // framework for the specified slave.
- if (!maintenance.offersOutstanding.contains(frameworkId)) {
- // Ignore in case the framework filters inverse offers for this
- // slave.
- //
- // NOTE: Since this specific allocator implementation only sends
- // inverse offers for maintenance primitives, and those are at the
- // whole slave level, we only need to filter based on the
- // time-out.
- if (isFiltered(framework, slave)) {
- continue;
- }
-
- const UnavailableResources unavailableResources =
- UnavailableResources{
- Resources(),
- maintenance.unavailability};
-
- // For now we send inverse offers with empty resources when the
- // inverse offer represents maintenance on the machine. In the
- // future we could be more specific about the resources on the
- // host, as we have the information available.
- offerable[frameworkId][slaveId] = unavailableResources;
-
- // Mark this framework as having an offer outstanding for the
- // specified slave.
- maintenance.offersOutstanding.insert(frameworkId);
+ // If this framework doesn't already have inverse offers for the
+ // specified slave.
+ if (!offerable[frameworkId].contains(slaveId)) {
+ // If there isn't already an outstanding inverse offer to this
+ // framework for the specified slave.
+ if (!maintenance.offersOutstanding.contains(frameworkId)) {
+ // Ignore in case the framework filters inverse offers for this
+ // slave.
+ //
+ // NOTE: Since this specific allocator implementation only sends
+ // inverse offers for maintenance primitives, and those are at the
+ // whole slave level, we only need to filter based on the
+ // time-out.
+ if (isFiltered(framework, slave)) {
+ continue;
}
+
+ const UnavailableResources unavailableResources =
+ UnavailableResources{Resources(), maintenance.unavailability};
+
+ // For now we send inverse offers with empty resources when the
+ // inverse offer represents maintenance on the machine. In the
+ // future we could be more specific about the resources on the
+ // host, as we have the information available.
+ offerable[frameworkId][slaveId] = unavailableResources;
+
+ // Mark this framework as having an offer outstanding for the
+ // specified slave.
+ maintenance.offersOutstanding.insert(frameworkId);
}
}
}
diff --git a/src/master/allocator/mesos/sorter/drf/sorter.cpp b/src/master/allocator/mesos/sorter/drf/sorter.cpp
index 09889cd..ef79083 100644
--- a/src/master/allocator/mesos/sorter/drf/sorter.cpp
+++ b/src/master/allocator/mesos/sorter/drf/sorter.cpp
@@ -431,32 +431,6 @@ const ResourceQuantities& DRFSorter::allocationScalarQuantities() const
}
-hashmap<string, Resources> DRFSorter::allocation(const SlaveID& slaveId) const
-{
- hashmap<string, Resources> result;
-
- // We want to find the allocation that has been made to each client
- // on a particular `slaveId`. Rather than traversing the tree
- // looking for leaf nodes (clients), we can instead just iterate
- // over the `clients` hashmap.
- //
- // TODO(jmlvanre): We can index the allocation by slaveId to make
- // this faster. It is a tradeoff between speed vs. memory. For now
- // we use existing data structures.
- foreachvalue (const Node* client, clients) {
- if (client->allocation.resources.contains(slaveId)) {
- // It is safe to use `at()` here because we've just checked the
- // existence of the key. This avoids unnecessary copies.
- string path = client->clientPath();
- CHECK(!result.contains(path));
- result.emplace(path, client->allocation.resources.at(slaveId));
- }
- }
-
- return result;
-}
-
-
Resources DRFSorter::allocation(
const string& clientPath,
const SlaveID& slaveId) const
diff --git a/src/master/allocator/mesos/sorter/drf/sorter.hpp b/src/master/allocator/mesos/sorter/drf/sorter.hpp
index f157ec6..2a861e2 100644
--- a/src/master/allocator/mesos/sorter/drf/sorter.hpp
+++ b/src/master/allocator/mesos/sorter/drf/sorter.hpp
@@ -90,9 +90,6 @@ public:
const ResourceQuantities& allocationScalarQuantities()
const override;
- hashmap<std::string, Resources> allocation(
- const SlaveID& slaveId) const override;
-
Resources allocation(
const std::string& clientPath,
const SlaveID& slaveId) const override;
diff --git a/src/master/allocator/mesos/sorter/random/sorter.cpp b/src/master/allocator/mesos/sorter/random/sorter.cpp
index 60a5797..86aeb1b 100644
--- a/src/master/allocator/mesos/sorter/random/sorter.cpp
+++ b/src/master/allocator/mesos/sorter/random/sorter.cpp
@@ -367,33 +367,6 @@ const ResourceQuantities& RandomSorter::allocationScalarQuantities() const
}
-hashmap<string, Resources> RandomSorter::allocation(
- const SlaveID& slaveId) const
-{
- hashmap<string, Resources> result;
-
- // We want to find the allocation that has been made to each client
- // on a particular `slaveId`. Rather than traversing the tree
- // looking for leaf nodes (clients), we can instead just iterate
- // over the `clients` hashmap.
- //
- // TODO(jmlvanre): We can index the allocation by slaveId to make
- // this faster. It is a tradeoff between speed vs. memory. For now
- // we use existing data structures.
- foreachvalue (const Node* client, clients) {
- if (client->allocation.resources.contains(slaveId)) {
- // It is safe to use `at()` here because we've just checked the
- // existence of the key. This avoids unnecessary copies.
- string path = client->clientPath();
- CHECK(!result.contains(path));
- result.emplace(path, client->allocation.resources.at(slaveId));
- }
- }
-
- return result;
-}
-
-
Resources RandomSorter::allocation(
const string& clientPath,
const SlaveID& slaveId) const
diff --git a/src/master/allocator/mesos/sorter/random/sorter.hpp b/src/master/allocator/mesos/sorter/random/sorter.hpp
index 8663ccd..bc55809 100644
--- a/src/master/allocator/mesos/sorter/random/sorter.hpp
+++ b/src/master/allocator/mesos/sorter/random/sorter.hpp
@@ -90,9 +90,6 @@ public:
const ResourceQuantities& allocationScalarQuantities()
const override;
- hashmap<std::string, Resources> allocation(
- const SlaveID& slaveId) const override;
-
Resources allocation(
const std::string& clientPath,
const SlaveID& slaveId) const override;
diff --git a/src/master/allocator/mesos/sorter/sorter.hpp b/src/master/allocator/mesos/sorter/sorter.hpp
index 52b8a7b..6b6b4a1 100644
--- a/src/master/allocator/mesos/sorter/sorter.hpp
+++ b/src/master/allocator/mesos/sorter/sorter.hpp
@@ -114,10 +114,6 @@ public:
const std::string& client) const = 0;
virtual const ResourceQuantities& allocationScalarQuantities() const = 0;
- // Returns the clients that have allocations on this slave.
- virtual hashmap<std::string, Resources> allocation(
- const SlaveID& slaveId) const = 0;
-
// Returns the given slave's resources that have been allocated to
// this client.
virtual Resources allocation(
diff --git a/src/tests/sorter_tests.cpp b/src/tests/sorter_tests.cpp
index 97ab910..64627c4 100644
--- a/src/tests/sorter_tests.cpp
+++ b/src/tests/sorter_tests.cpp
@@ -737,14 +737,6 @@ TEST(DRFSorterTest, HierarchicalAllocation)
EXPECT_EQ(vector<string>({"a", "b/d", "b/c"}), sorter.sort());
{
- hashmap<string, Resources> agentAllocation =
- sorter.allocation(slaveId);
-
- EXPECT_EQ(3u, agentAllocation.size());
- EXPECT_EQ(aResources, agentAllocation.at("a"));
- EXPECT_EQ(cResources, agentAllocation.at("b/c"));
- EXPECT_EQ(dResources, agentAllocation.at("b/d"));
-
EXPECT_EQ(aResources, sorter.allocation("a", slaveId));
EXPECT_EQ(cResources, sorter.allocation("b/c", slaveId));
EXPECT_EQ(dResources, sorter.allocation("b/d", slaveId));
@@ -1298,11 +1290,6 @@ TYPED_TEST(CommonSorterTest, AllocationForInactiveClient)
sorter.allocated("a", slaveId, Resources::parse("cpus:2;mem:2").get());
sorter.allocated("b", slaveId, Resources::parse("cpus:3;mem:3").get());
- hashmap<string, Resources> clientAllocation = sorter.allocation(slaveId);
- EXPECT_EQ(2u, clientAllocation.size());
- EXPECT_EQ(Resources::parse("cpus:2;mem:2").get(), clientAllocation.at("a"));
- EXPECT_EQ(Resources::parse("cpus:3;mem:3").get(), clientAllocation.at("b"));
-
hashmap<SlaveID, Resources> agentAllocation1 = sorter.allocation("a");
EXPECT_EQ(1u, agentAllocation1.size());
EXPECT_EQ(