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 0357864dc8f5c9d5aaf06a11e2a7f37aa5778a09 Author: Meng Zhu <[email protected]> AuthorDate: Fri Mar 29 11:35:42 2019 -0700 Fixed a bug where disk quota may be under allocated. The resources chopping logic in the allocator currently overly shrinks resources with the same name (e.g. vanilla disk and mount disk) if there are multiple such resources. Currently only different disk resources might share the same name (we only chop unreserved/non-revocable/non-shared resources). This would lead to disk quota being under allocated. This patch corrects the shrinking logic. See MESOS-9692. Review: https://reviews.apache.org/r/70344 --- src/master/allocator/mesos/hierarchical.cpp | 2 +- src/tests/hierarchical_allocator_tests.cpp | 49 +++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+), 1 deletion(-) diff --git a/src/master/allocator/mesos/hierarchical.cpp b/src/master/allocator/mesos/hierarchical.cpp index 1cda917..514887f 100644 --- a/src/master/allocator/mesos/hierarchical.cpp +++ b/src/master/allocator/mesos/hierarchical.cpp @@ -1654,7 +1654,7 @@ void HierarchicalAllocatorProcess::__allocate() targetScalarQuantites.get(resource.name()); if (Resources::shrink(&resource, limitScalar.get())) { - targetScalarQuantites[resource.name()] -= limitScalar.get(); + targetScalarQuantites[resource.name()] -= resource.scalar(); result += std::move(resource); } } diff --git a/src/tests/hierarchical_allocator_tests.cpp b/src/tests/hierarchical_allocator_tests.cpp index 5f011c5..d851866 100644 --- a/src/tests/hierarchical_allocator_tests.cpp +++ b/src/tests/hierarchical_allocator_tests.cpp @@ -3705,6 +3705,55 @@ TEST_F(HierarchicalAllocatorTest, QuotaAllocationGranularityUnchoppableResource) } +// This test ensures that quota is satisfied in the presence of multiple +// unreserved disk resources. This is a regression test for MESOS-9692. +TEST_F(HierarchicalAllocatorTest, QuotaAllocationMultipleDisk) +{ + // The test sets a quota that contains 100 disk resources and + // an agent with two kinds of disk resources: 50 vanilla disks + // and 50 mount disks. Both disks (100 in total) should be offered + // to the framework in a single offer to meet its role's quota. + + Clock::pause(); + initialize(); + + const string QUOTA_ROLE{"quota-role"}; + + const Quota quota = createQuota(QUOTA_ROLE, "cpus:1;mem:512;disk:100"); + allocator->setQuota(QUOTA_ROLE, quota); + + // Create 50 disk resource of type `MOUNT`. + Resources mountDiskResource = createDiskResource( + "50", "*", None(), None(), createDiskSourceMount(), false); + + Resources agentResources = + Resources::parse("cpus:1;mem:512;disk:50").get() + mountDiskResource; + + SlaveInfo agent1 = createSlaveInfo(agentResources); + allocator->addSlave( + agent1.id(), + agent1, + AGENT_CAPABILITIES(), + None(), + agent1.resources(), + {}); + + // Create `framework` under `QUOTA_ROLE`. + // This will tigger an event-driven allocation loop. + FrameworkInfo framework = createFrameworkInfo({QUOTA_ROLE}); + allocator->addFramework(framework.id(), framework, {}, true, {}); + + Clock::settle(); + + // `framework` will get all resources (including both disks) of `agent1` to + // satisfy its role's quota. + Allocation expected = Allocation( + framework.id(), {{QUOTA_ROLE, {{agent1.id(), agent1.resources()}}}}); + + AWAIT_EXPECT_EQ(expected, allocations.get()); +} + + // This test verifies the behavior of allocating a resource to quota roles // that has no quota set for that particular resource (e.g. allocating // memory to a role with only quota set for CPU). If a role has no quota
